conductor(track): Initialize mcp_architecture_refactor_20260606
Track + metadata + state + tracks.md registration for the 2,205-line mcp_client.py split into a slim controller + 6 native sub-MCPs + 1 external sub-MCP. Key design decisions (per user feedback): - Naming convention: mcp_<type>.py for native MCPs (mcp_file_io.py, mcp_python.py, mcp_c.py, mcp_cpp.py, mcp_web.py, mcp_analysis.py). - ExternalMCPManager class name preserved (moves to mcp_external.py). - Sub-MCP shape: class with name / description / tools / invoke(). - MCPController: holds ALL_SUB_MCPS list, inverted-dict tool lookup, 3-layer security (extracted to mcp_client_security.py), schema aggregation. - Each invoke() returns Result[str, ErrorInfo] (from data_oriented_error_handling_20260606). - Backward compat: mcp_client_legacy.py re-exports all 45+ old symbols; the 4 existing test files + src/app_controller.py:61 direct call continue to work. DSL future (per user notes on APL/K/Cosy): NOT in this track. Documented in spec §12.1 as the mcp_dsl_20260606 follow-up. Sub-MCP architecture is the natural unit to pair with a DSL emitter. 7 phases. ~22 task slots. New tests: 9 (one per sub-MCP + controller + security + legacy). Modified tests: 4 (existing mcp_* tests must pass unchanged). Blocked by: data_oriented_error_handling_20260606, data_structure_strengthening_20260606. Blocks: mcp_dsl_20260606 (future DSL track).
This commit is contained in:
@@ -173,6 +173,10 @@ User review surfaced five outstanding UI issues, each previously attempted witho
|
||||
*Link: [./tracks/data_structure_strengthening_20260606/](./tracks/data_structure_strengthening_20260606/), Spec: [./tracks/data_structure_strengthening_20260606/spec.md](./tracks/data_structure_strengthening_20260606/spec.md), Plan: [./tracks/data_structure_strengthening_20260606/plan.md](./tracks/data_structure_strengthening_20260606/plan.md) (to be authored by writing-plans skill)*
|
||||
*Goal: Improve AI-readability by naming 430 currently-anonymous `dict[str, Any]` / `list[dict[...]]` / `Tuple[...]` types. New `src/type_aliases.py` with 10 `TypeAlias` definitions (`Metadata`, `CommsLogEntry`, `CommsLog`, `HistoryMessage`, `History`, `FileItem`, `FileItems`, `ToolDefinition`, `ToolCall`, `CommsLogCallback`) and 1 `NamedTuple` (`FileItemsDiff`). Mechanical replacement of 345 weak sites across 6 high-traffic files: `src/ai_client.py` (139), `src/app_controller.py` (86), `src/models.py` (51), `src/api_hook_client.py` (32), `src/project_manager.py` (20), `src/aggregate.py` (17). Add `--strict` mode to the existing `scripts/audit_weak_types.py` (committed in 84fd9ac9; found the 430 sites) so it becomes a permanent CI gate that fails when new weak types are introduced. Generate `scripts/audit_weak_types.baseline.json` with the post-refactor count. 2 phases: aliases + 6-file replacement + audit baseline; NamedTuples + docs + archive. **Data-grounded**: the audit script is the source of truth; the count drops from 430 to ~60 (86% reduction) in the 6 high-traffic files. **Honest about what's missing**: 23 lower-impact files remain; TypedDict/dataclass migration is deferred to a follow-up track. 2-3 days work, 1-2 phases, low risk.*
|
||||
|
||||
0g. [ ] **Track: MCP Architecture Refactor (Sub-MCP Extraction)** `[track-created: <pending>]`
|
||||
*Link: [./tracks/mcp_architecture_refactor_20260606/](./tracks/mcp_architecture_refactor_20260606/), Spec: [./tracks/mcp_architecture_refactor_20260606/spec.md](./tracks/mcp_architecture_refactor_20260606/spec.md), Plan: [./tracks/mcp_architecture_refactor_20260606/plan.md](./tracks/mcp_architecture_refactor_20260606/plan.md) (to be authored by writing-plans skill)*
|
||||
*Goal: Split the 2,205-line monolithic `src/mcp_client.py` (45 module-level functions) into a slim controller + 6 native sub-MCPs + 1 external sub-MCP. Naming convention `mcp_<type>.py` for native MCPs: `mcp_file_io.py` (9 tools), `mcp_python.py` (14), `mcp_c.py` (5), `mcp_cpp.py` (5), `mcp_web.py` (2), `mcp_analysis.py` (2). The existing `ExternalMCPManager` is extracted to `mcp_external.py` (class name preserved). New `MCPController` class in `src/mcp_client.py` holds the 3-layer security model (extracted to `src/mcp_client_security.py`), the `ALL_SUB_MCPS` registration list, and the inverted-dict dispatch lookup. New `src/mcp_client_legacy.py` re-exports all 45+ old symbols for backward compat (the 4 existing test files + `src/app_controller.py:61` continue to work). Each sub-MCP's `invoke()` returns `Result[str, ErrorInfo]` (Fleury pattern). Path parameters use the `Metadata` family aliases. **Blocked by** `data_oriented_error_handling_20260606` (for `Result`/`ErrorInfo`) and `data_structure_strengthening_20260606` (for `Metadata` aliases). 7 phases: foundation (security + controller), move-to-legacy, extract File I/O, extract Python, extract C/C++/Web/Analysis, extract External, dispatch update + docs + archive. **Out of scope** (per user): a per-MCP DSL (APL/K/Cosy-inspired) for compact tool calls — deferred to `mcp_dsl_20260606` follow-up. JSON-only for now.*
|
||||
|
||||
0b. [x] **Track: rag_phase4_stress_test_flake_20260606** — fixed 16412ad5
|
||||
*Status: 2026-06-06 — Surfaced during post-v2 verification. Resolved: real bug, NOT a test flake. Root cause: ChromaDB collection dimension mismatch across test runs. The persistent on-disk collection (`tests/artifacts/live_gui_workspace/.slop_cache/chroma_test_stress/`) was created by a previous run with Gemini embeddings (3072-dim); the current run uses local SentenceTransformers (384-dim). `index_file()` upserts silently corrupt the collection, then `search()` fails with `Collection expecting embedding with dimension of 3072, got 384` and the AI request never reaches 'done' status, timing out the 50*0.5s = 25s poll loop. Fix: `RAGEngine._init_vector_store` now calls `_validate_collection_dim` which inspects the first existing vector's dim, compares to the current provider's output, and recreates the collection on mismatch (with a stderr warning). Regression tests added: `test_rag_collection_dim_mismatch_recreates_collection` and `test_rag_collection_dim_match_preserves_collection` in `tests/test_rag_engine.py`. This also fixes a real user-facing bug: switching embedding providers in the GUI previously caused silent corruption. Commit 16412ad5.*
|
||||
0a. [ ] **Track: prior_session_test_harden_20260605** [superseded by live_gui_test_hardening_v2_20260605]
|
||||
|
||||
@@ -0,0 +1,162 @@
|
||||
{
|
||||
"track_id": "mcp_architecture_refactor_20260606",
|
||||
"name": "MCP Architecture Refactor (Sub-MCP Extraction)",
|
||||
"initialized": "2026-06-06",
|
||||
"owner": "tier2-tech-lead",
|
||||
"priority": "high",
|
||||
"status": "active",
|
||||
"type": "refactor + structural + ai-readability",
|
||||
"scope": {
|
||||
"new_files": [
|
||||
"src/mcp_client_security.py",
|
||||
"src/mcp_client_legacy.py",
|
||||
"src/mcp_file_io.py",
|
||||
"src/mcp_python.py",
|
||||
"src/mcp_c.py",
|
||||
"src/mcp_cpp.py",
|
||||
"src/mcp_web.py",
|
||||
"src/mcp_analysis.py",
|
||||
"src/mcp_external.py",
|
||||
"tests/test_mcp_client.py",
|
||||
"tests/test_mcp_client_security.py",
|
||||
"tests/test_mcp_file_io.py",
|
||||
"tests/test_mcp_python.py",
|
||||
"tests/test_mcp_c.py",
|
||||
"tests/test_mcp_cpp.py",
|
||||
"tests/test_mcp_web.py",
|
||||
"tests/test_mcp_analysis.py",
|
||||
"tests/test_mcp_external.py",
|
||||
"tests/test_mcp_client_legacy.py"
|
||||
],
|
||||
"modified_files": [
|
||||
"src/mcp_client.py",
|
||||
"tests/test_mcp_client_beads.py",
|
||||
"tests/test_mcp_config.py",
|
||||
"tests/test_mcp_perf_tool.py",
|
||||
"tests/test_mcp_ts_integration.py"
|
||||
]
|
||||
},
|
||||
"blocked_by": ["data_oriented_error_handling_20260606", "data_structure_strengthening_20260606"],
|
||||
"blocks": ["mcp_dsl_20260606" /* not yet created; the future DSL track */],
|
||||
"estimated_phases": 7,
|
||||
"spec": "spec.md",
|
||||
"plan": "plan.md",
|
||||
"priority_order": "A (foundation + sub-MCPs) > B (Result pattern + security) > C (dispatch inversion + docs) > D (plan DSL follow-up)",
|
||||
"naming_convention": "mcp_<type>.py for native MCPs; ExternalMCPManager class name preserved in mcp_external.py",
|
||||
"current_state": {
|
||||
"mcp_client_py_lines": 2205,
|
||||
"function_count": 45,
|
||||
"dispatch_entry_points": ["dispatch (sync, line 1338)", "async_dispatch (line 1496)"],
|
||||
"external_callers": ["src/app_controller.py:61 (direct mcp_client.py_get_symbol_info call)"],
|
||||
"existing_test_files": [
|
||||
"tests/test_mcp_client_beads.py",
|
||||
"tests/test_mcp_config.py",
|
||||
"tests/test_mcp_perf_tool.py",
|
||||
"tests/test_mcp_ts_integration.py"
|
||||
],
|
||||
"external_mcp_existing_class": "ExternalMCPManager (in mcp_client.py; runtime-loaded MCPs)"
|
||||
},
|
||||
"sub_mcps": {
|
||||
"file_io": {
|
||||
"file": "src/mcp_file_io.py",
|
||||
"class": "FileIOMCP",
|
||||
"tool_count": 9,
|
||||
"tools": ["read_file", "list_directory", "search_files", "get_file_summary", "get_file_slice", "set_file_slice", "edit_file", "get_tree", "get_git_diff"],
|
||||
"uses_security": true
|
||||
},
|
||||
"python": {
|
||||
"file": "src/mcp_python.py",
|
||||
"class": "PythonMCP",
|
||||
"tool_count": 14,
|
||||
"tools_prefix": "py_",
|
||||
"uses_security": true
|
||||
},
|
||||
"c": {
|
||||
"file": "src/mcp_c.py",
|
||||
"class": "CMCP",
|
||||
"tool_count": 5,
|
||||
"tools_prefix": "ts_c_",
|
||||
"uses_security": true
|
||||
},
|
||||
"cpp": {
|
||||
"file": "src/mcp_cpp.py",
|
||||
"class": "CppMCP",
|
||||
"tool_count": 5,
|
||||
"tools_prefix": "ts_cpp_",
|
||||
"uses_security": true
|
||||
},
|
||||
"web": {
|
||||
"file": "src/mcp_web.py",
|
||||
"class": "WebMCP",
|
||||
"tool_count": 2,
|
||||
"tools": ["web_search", "fetch_url"],
|
||||
"uses_security": false,
|
||||
"uses_url_validation": true
|
||||
},
|
||||
"analysis": {
|
||||
"file": "src/mcp_analysis.py",
|
||||
"class": "AnalysisMCP",
|
||||
"tool_count": 2,
|
||||
"tools": ["derive_code_path", "get_ui_performance"],
|
||||
"uses_security": false
|
||||
},
|
||||
"external": {
|
||||
"file": "src/mcp_external.py",
|
||||
"class": "ExternalMCP (was ExternalMCPManager; class name preserved)",
|
||||
"registered_in_all_sub_mcps": false,
|
||||
"note": "Sub-controller for runtime-loaded MCPs; the main controller delegates to it AFTER native sub-MCPs miss."
|
||||
}
|
||||
},
|
||||
"architectural_invariant": "src/mcp_client.py is the controller; the sub-MCPs (mcp_<type>.py) are self-contained units that implement the SubMCP Protocol. The 3-layer security model lives in src/mcp_client_security.py and is invoked by the controller BEFORE delegating to sub-MCPs. The legacy shim (src/mcp_client_legacy.py) re-exports all old symbols for backward compat. Result[str, ErrorInfo] is the canonical return type from invoke().",
|
||||
"threading_constraint": "Same as existing pattern. The dispatch is synchronous; async_dispatch is for external MCPs. Sub-MCPs are stateless (no shared state between calls). The controller's _tool_index is built once at init and is read-only afterward.",
|
||||
"dsl_future": {
|
||||
"rationale": "Per user notes: 'kinda want to compress the mcp to just have a single intention based DSL per mcp, kinda like command line but more flexible'. Inspired by APL/K/Cosy. Out of scope for this track ('no time for that' per user).",
|
||||
"estimated_token_savings": "JSON: ~60-100 tokens per call. DSL: ~10-20 tokens per call. ~5x reduction.",
|
||||
"follow_up_track": "mcp_dsl_20260606 (planned; not in this spec)",
|
||||
"architectural_fit": "The sub-MCP architecture is the natural unit to pair with a DSL emitter. Each mcp_<type>.py could declare a grammar (e.g., src/mcp_python_grammar.k) that compiles to a parser; the controller dispatches to either the JSON or the DSL path based on tool_input type."
|
||||
},
|
||||
"verification_criteria": [
|
||||
"src/mcp_client_security.py exists with _is_allowed, _resolve_and_check, configure; returns Result[Path] (not tuple); 100% test coverage",
|
||||
"src/mcp_client.py is slim (< 200 lines); contains MCPController + SubMCP Protocol + module-level singleton + ALL_SUB_MCPS registration; re-exports from mcp_client_legacy for backward compat",
|
||||
"src/mcp_client_legacy.py re-exports all 45+ old function names; tests/test_mcp_client_legacy.py verifies the surface",
|
||||
"src/mcp_file_io.py exists with FileIOMCP class; read_file, list_directory, etc. are instance methods; invoke() returns Result[str, ErrorInfo]",
|
||||
"src/mcp_python.py exists with PythonMCP class; all 14 py_* tools",
|
||||
"src/mcp_c.py exists with CMCP class; all 5 ts_c_* tools",
|
||||
"src/mcp_cpp.py exists with CppMCP class; all 5 ts_cpp_* tools",
|
||||
"src/mcp_web.py exists with WebMCP class; web_search, fetch_url; URL validation",
|
||||
"src/mcp_analysis.py exists with AnalysisMCP class; derive_code_path, get_ui_performance",
|
||||
"src/mcp_external.py exists with ExternalMCP class (renamed from ExternalMCPManager); same methods as the existing class",
|
||||
"MCPController.dispatch uses the ALL_SUB_MCPS lookup (O(1)); not an if/elif chain",
|
||||
"MCPController.dispatch runs _resolve_and_check for path-taking tools BEFORE delegating to sub-MCPs",
|
||||
"MCPController.get_tool_schemas aggregates from all sub-MCPs (single source of truth)",
|
||||
"tests/test_mcp_client.py: 6+ tests pass (registration, dispatch, security integration, schema aggregation)",
|
||||
"tests/test_mcp_client_security.py: 8+ tests pass (allowed, not-allowed, configure, resolve errors)",
|
||||
"tests/test_mcp_file_io.py: 9+ tests pass (one per tool + security integration)",
|
||||
"tests/test_mcp_python.py: 14+ tests pass (one per py_* tool)",
|
||||
"tests/test_mcp_c.py: 5+ tests pass (one per ts_c_* tool)",
|
||||
"tests/test_mcp_cpp.py: 5+ tests pass (one per ts_cpp_* tool)",
|
||||
"tests/test_mcp_web.py: 4+ tests pass (web_search, fetch_url, URL validation)",
|
||||
"tests/test_mcp_analysis.py: 4+ tests pass (derive_code_path, get_ui_performance)",
|
||||
"tests/test_mcp_external.py: 4+ tests pass (register_server, async_dispatch, get_tool_schemas)",
|
||||
"tests/test_mcp_client_legacy.py: 10+ tests pass (verify all 45+ old symbols re-exported)",
|
||||
"tests/test_mcp_client_beads.py (existing): no regressions",
|
||||
"tests/test_mcp_config.py (existing): no regressions",
|
||||
"tests/test_mcp_perf_tool.py (existing): no regressions",
|
||||
"tests/test_mcp_ts_integration.py (existing): no regressions",
|
||||
"src/app_controller.py:61 (the direct mcp_client.py_get_symbol_info call) still works (verified by existing tests)",
|
||||
"Full test suite: no regressions in 273+ existing tests",
|
||||
"No new threading.Thread calls in src/",
|
||||
"No new Optional[X] in the new files (the aliases are used where dicts are needed)"
|
||||
],
|
||||
"links": {
|
||||
"backlog_entry": "conductor/tracks.md (to be added)",
|
||||
"current_mcp_client": "src/mcp_client.py",
|
||||
"external_mcp_existing": "src/mcp_client.py:ExternalMCPManager (will move to mcp_external.py:ExternalMCP)",
|
||||
"related_tracks": [
|
||||
"conductor/tracks/data_oriented_error_handling_20260606/",
|
||||
"conductor/tracks/data_structure_strengthening_20260606/",
|
||||
"conductor/tracks/test_batching_refactor_20260606/",
|
||||
"conductor/tracks/qwen_llama_grok_integration_20260606/"
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,406 @@
|
||||
# Track: MCP Architecture Refactor (Sub-MCP Extraction)
|
||||
|
||||
**Status:** Active (spec approved 2026-06-06)
|
||||
**Initialized:** 2026-06-06
|
||||
**Owner:** Tier 2 Tech Lead
|
||||
**Priority:** High (structural; 2,205-line mcp_client.py is the largest single file in the project; reduces future maintenance cost)
|
||||
|
||||
---
|
||||
|
||||
## 1. Overview
|
||||
|
||||
This track splits `src/mcp_client.py` (currently 2,205 lines with 45 module-level functions) into a **main controller** plus **6 native sub-MCPs** + **1 external sub-MCP**. The controller owns the 3-layer security model (Allowlist → Validate → Resolve), the dispatch logic, and the tool-schema export. Each sub-MCP owns a category of tools:
|
||||
|
||||
- `mcp_file_io.py` — File I/O (read_file, list_directory, search_files, get_file_summary, get_file_slice, set_file_slice, edit_file, get_tree, get_git_diff; ~9 funcs)
|
||||
- `mcp_python.py` — Python AST (py_* family; ~14 funcs)
|
||||
- `mcp_c.py` — C AST (ts_c_* family; 5 funcs)
|
||||
- `mcp_cpp.py` — C++ AST (ts_cpp_* family; 5 funcs)
|
||||
- `mcp_web.py` — Web (web_search, fetch_url; 2 funcs)
|
||||
- `mcp_analysis.py` — Analysis (derive_code_path, get_ui_performance; 2 funcs)
|
||||
- `mcp_external.py` — External MCPs (the existing `ExternalMCPManager`; runtime-loaded)
|
||||
|
||||
**Sub-MCP shape:** each `mcp_<type>.py` exports a class (e.g., `class PythonMCP`) that implements a `SubMCP` Protocol: `name: str`, `tools: dict[str, Callable]`, `invoke(tool_name, args) -> Result[str, ErrorInfo]`. The controller holds a list `ALL_SUB_MCPS` and dispatches via the `tools` dict. **Adding a new sub-MCP = create a new `mcp_<type>.py` file + add 2 lines to `mcp_client.py`'s `ALL_SUB_MCPS` list.**
|
||||
|
||||
**File naming convention:** `mcp_<type>.py` for native MCPs (per user direction). For externals, the existing `ExternalMCPManager` class name is preserved (the class moves to `mcp_external.py`; the name doesn't change to avoid breaking the existing import surface).
|
||||
|
||||
**DSL future:** the user noted a future interest in per-MCP compact DSLs (APL/K/Cosy-inspired) for tool calling instead of JSON. **This is explicitly OUT OF SCOPE for this track** (per user: "no time for that"). A future track MAY introduce a DSL layer; this track stays JSON-compatible and lays no groundwork that would prevent a future DSL.
|
||||
|
||||
## 2. Goals (Priority Order)
|
||||
|
||||
| Priority | Goal | Rationale |
|
||||
|---|---|---|
|
||||
| **A (foundational)** | New `SubMCP` Protocol + `MCPController` class in `src/mcp_client.py`. Controller dispatches via `ALL_SUB_MCPS` list; holds the 3-layer security model; holds the schema export. | The controller is the central abstraction. Per Casey Muratori's module-layer boundary: each module owns its data and exposes a clean interface; consumers adapt. |
|
||||
| **A (primary value)** | Extract 6 native sub-MCPs (File I/O, Python, C, C++, Web, Analysis) into separate `mcp_<type>.py` files. Each is a class with `name`, `tools`, `invoke()`. | The current monolithic file is the largest in the project. Extracting by category aligns with the user's mental model and makes future maintenance tractable. |
|
||||
| **A (primary value)** | Extract the existing `ExternalMCPManager` into `mcp_external.py`. The class name is preserved. | The external MCPs (Beads, etc.) are a separate concern; they were already a class. Moving them to their own file clarifies the architecture. |
|
||||
| **A (backward compat)** | New `src/mcp_client_legacy.py` re-exports all 45+ old function names. Old `mcp_client.py` becomes a thin shim that imports from `mcp_client_legacy` and re-exports. | The 4 existing test files (`test_mcp_client_beads.py`, `test_mcp_config.py`, `test_mcp_perf_tool.py`, `test_mcp_ts_integration.py`) and `src/app_controller.py:61` (the direct `mcp_client.py_get_symbol_info` call) keep working during the transition. |
|
||||
| **B (architectural)** | Sub-MCPs return `Result[str, ErrorInfo]` (from `data_oriented_error_handling_20260606`). Path parameters use the `Metadata` family aliases (from `data_structure_strengthening_20260606`). | Consistent with the project's post-Fleury conventions. The 3-layer security becomes `Result.errors` entries. |
|
||||
| **B (architectural)** | The 3-layer security model (`_is_allowed`, `_resolve_and_check`) is extracted to `src/mcp_client_security.py` (a sub-module of the controller). The controller calls it BEFORE delegating to sub-MCPs. Sub-MCPs receive already-validated paths. | Clean separation: sub-MCPs are testable in isolation without security; one place to update security policy. |
|
||||
| **C (optimization)** | `dispatch()` and `async_dispatch()` in the controller use the `ALL_SUB_MCPS` list for tool lookup (O(1) per dispatch via inverted dict), not the current if/elif chain (O(n) per dispatch). | At ~60 tools today, the if/elif is fast enough but doesn't scale. The inverted-dict lookup is the same code complexity and the right shape. |
|
||||
| **C (optimization)** | `get_tool_schemas()` aggregates the schemas from all registered sub-MCPs. Single source of truth for the AI-facing tool catalog. | The current `get_tool_schemas()` is a manual list; the new version is auto-derived from the registered sub-MCPs. |
|
||||
| **D (forward-looking)** | Plan a future "MCP DSL Track" that introduces a per-MCP compact dialect (replacing or augmenting JSON for tool calls). NOT in this track; documented in §13.1. | The user expressed interest in this idea; this track lays the groundwork (each sub-MCP is a self-contained unit that could be paired with a DSL emitter) but does not implement it. |
|
||||
|
||||
### 2.1 Non-Goals (this track)
|
||||
|
||||
- **Not** implementing a DSL for tool calls. JSON-only for now. A future track can layer a DSL on top.
|
||||
- **Not** touching the agent runtime's tool-calling format. The agent still calls `mcp_client.dispatch("py_get_skeleton", {"path": "/src/foo.py"})` — the format is unchanged.
|
||||
- **Not** merging or splitting sub-MCPs. The 6-7 categories are fixed for this track.
|
||||
- **Not** adding new tool categories. If a future tool doesn't fit any of the 7 categories, that's a separate concern (either add a new `mcp_<type>.py` or extend an existing one).
|
||||
- **Not** migrating to `TypedDict` schemas for tool arguments. The `Metadata` family aliases are used; the deeper schema is deferred to the `typed_dict_migration_20260606` follow-up.
|
||||
- **Not** changing the public API of any tool function. The tools' signatures stay the same; the return type changes from `str` to `Result[str, ErrorInfo]` but the legacy shim unwraps `.data` for backward compat.
|
||||
|
||||
## 3. Architecture
|
||||
|
||||
### 3.1 The `SubMCP` Protocol
|
||||
|
||||
`src/mcp_client.py` (slim controller) defines the Protocol:
|
||||
|
||||
```python
|
||||
from typing import Protocol, Any, Callable, TYPE_CHECKING
|
||||
from src.result_types import Result
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from src.mcp_sub_file_io import FileIOMCP
|
||||
# ... etc (avoid runtime circular imports)
|
||||
|
||||
class SubMCP(Protocol):
|
||||
"""A native MCP that owns a category of tools.
|
||||
Implementations live in src/mcp_<type>.py."""
|
||||
name: str
|
||||
description: str
|
||||
tools: dict[str, Callable[..., str]]
|
||||
def invoke(self, tool_name: str, args: dict[str, Any]) -> Result[str, Any]: ...
|
||||
```
|
||||
|
||||
The `tools` dict is the public API: tool_name → function. The `invoke` method is the dispatch entry point. Implementations are not required to be classes; they can be modules with a `register_sub_mcp()` function, or dataclasses. **The Protocol is the contract; the implementation strategy is flexible.**
|
||||
|
||||
### 3.2 The `MCPController` Class
|
||||
|
||||
```python
|
||||
class MCPController:
|
||||
def __init__(self) -> None:
|
||||
self._sub_mcps: list[SubMCP] = []
|
||||
self._tool_index: dict[str, SubMCP] = {} # tool_name -> owning SubMCP
|
||||
self._external_mcp = ExternalMCP() # the new mcp_external.py's class
|
||||
|
||||
def register(self, sub_mcp: SubMCP) -> None:
|
||||
self._sub_mcps.append(sub_mcp)
|
||||
for tool_name in sub_mcp.tools:
|
||||
if tool_name in self._tool_index:
|
||||
raise ValueError(f"Tool {tool_name!r} already registered by {self._tool_index[tool_name].name}")
|
||||
self._tool_index[tool_name] = sub_mcp
|
||||
|
||||
def dispatch(self, tool_name: str, tool_input: dict[str, Any]) -> Result[str, Any]:
|
||||
# 1. Check native sub-MCPs (O(1) lookup)
|
||||
if tool_name in self._tool_index:
|
||||
return self._tool_index[tool_name].invoke(tool_name, tool_input)
|
||||
# 2. Check external MCPs (runtime-loaded)
|
||||
ext_result = self._external_mcp.try_invoke(tool_name, tool_input)
|
||||
if ext_result is not None:
|
||||
return ext_result
|
||||
# 3. Not found
|
||||
return Result(data="", errors=[ErrorInfo(kind=ErrorKind.NOT_FOUND, message=f"Tool {tool_name!r} not found", source="mcp_client.dispatch")])
|
||||
|
||||
async def async_dispatch(self, tool_name: str, tool_input: dict[str, Any]) -> Result[str, Any]:
|
||||
# Similar; uses async tools for sub-MCPs that need them
|
||||
...
|
||||
|
||||
def get_tool_schemas(self) -> list[dict[str, Any]]:
|
||||
return [schema for sub_mcp in self._sub_mcps for schema in sub_mcp.schemas()]
|
||||
|
||||
# Module-level singleton
|
||||
_controller = MCPController()
|
||||
_controller.register(FileIOMCP())
|
||||
controller.register(PythonMCP())
|
||||
controller.register(CMCP())
|
||||
controller.register(CppMCP())
|
||||
controller.register(WebMCP())
|
||||
controller.register(AnalysisMCP())
|
||||
# ExternalMCP is NOT registered as a tool (it's a sub-controller for runtime-loaded tools)
|
||||
```
|
||||
|
||||
The controller is a module-level singleton. The `ALL_SUB_MCPS` list is implicit in the registration calls at module bottom; the registration order doesn't matter.
|
||||
|
||||
### 3.3 The 3-Layer Security Model
|
||||
|
||||
`src/mcp_client_security.py` (NEW):
|
||||
|
||||
```python
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from src.result_types import ErrorInfo, ErrorKind, Result, NilPath
|
||||
|
||||
_ALLOWED_BASE_DIRS: list[Path] = [Path(".").resolve()]
|
||||
|
||||
def configure(file_items: list[dict[str, Any]], extra_base_dirs: list[str] | None = None) -> None:
|
||||
"""Configure the allowed base directories. Called by app_controller.py at startup."""
|
||||
global _ALLOWED_BASE_DIRS
|
||||
_ALLOWED_BASE_DIRS = [Path(".").resolve()]
|
||||
for item in file_items:
|
||||
p = Path(item.get("path", ".")).resolve()
|
||||
if p not in _ALLOWED_BASE_DIRS:
|
||||
_ALLOWED_BASE_DIRS.append(p)
|
||||
if extra_base_dirs:
|
||||
for d in extra_base_dirs:
|
||||
_ALLOWED_BASE_DIRS.append(Path(d).resolve())
|
||||
|
||||
def _is_allowed(path: Path) -> bool:
|
||||
"""Layer 1: Is the path in an allowed base?"""
|
||||
for base in _ALLOWED_BASE_DIRS:
|
||||
try:
|
||||
if path.resolve().is_relative_to(base):
|
||||
return True
|
||||
except (ValueError, OSError):
|
||||
pass
|
||||
return False
|
||||
|
||||
def _resolve_and_check(raw_path: str) -> Result[Path]:
|
||||
"""Layer 2 + 3: Resolve the path AND check it against the allowlist.
|
||||
Returns Result[Path]. data is a real Path on success or NilPath() on failure.
|
||||
errors contains the layered error info."""
|
||||
try:
|
||||
p = Path(raw_path).resolve()
|
||||
except (OSError, ValueError) as e:
|
||||
return Result(data=NilPath(), errors=[ErrorInfo(kind=ErrorKind.INVALID_INPUT, message=str(e), source="mcp_client_security", original=e)])
|
||||
if not _is_allowed(p):
|
||||
return Result(data=NilPath(), errors=[ErrorInfo(kind=ErrorKind.PERMISSION, message=f"path {raw_path!r} not in allowed base", source="mcp_client_security")])
|
||||
return Result(data=p)
|
||||
```
|
||||
|
||||
The controller's `dispatch` runs `_resolve_and_check` BEFORE delegating to sub-MCPs (for path-taking tools). Sub-MCPs receive already-validated paths.
|
||||
|
||||
### 3.4 Per-Sub-MCP Shape
|
||||
|
||||
Each `mcp_<type>.py` exports a class. Example for File I/O:
|
||||
|
||||
```python
|
||||
# src/mcp_file_io.py
|
||||
from pathlib import Path
|
||||
from typing import Any, Callable
|
||||
from src.result_types import ErrorInfo, ErrorKind, Result
|
||||
from src.type_aliases import FileItem, FileItems, Metadata
|
||||
from src.mcp_client_security import _resolve_and_check
|
||||
|
||||
class FileIOMCP:
|
||||
name = "file_io"
|
||||
description = "File I/O: read, list, search, slice, edit, summary"
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.tools: dict[str, Callable[..., str]] = {
|
||||
"read_file": self.read_file,
|
||||
"list_directory": self.list_directory,
|
||||
# ... etc
|
||||
}
|
||||
|
||||
def invoke(self, tool_name: str, args: dict[str, Any]) -> Result[str, Any]:
|
||||
if tool_name not in self.tools:
|
||||
return Result(data="", errors=[ErrorInfo(kind=ErrorKind.NOT_FOUND, message=f"{tool_name!r} not in {self.name}", source=f"mcp.{self.name}")])
|
||||
try:
|
||||
result = self.tools[tool_name](**args)
|
||||
return Result(data=result)
|
||||
except Exception as e:
|
||||
return Result(data="", errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source=f"mcp.{self.name}.{tool_name}", original=e)])
|
||||
|
||||
def read_file(self, path: str) -> str:
|
||||
resolved = _resolve_and_check(path)
|
||||
if not resolved.ok:
|
||||
return ""
|
||||
p = resolved.data
|
||||
if isinstance(p, NilPath):
|
||||
return ""
|
||||
if not p.exists() or not p.is_file():
|
||||
return f"ERROR: file not found: {path}"
|
||||
try:
|
||||
return p.read_text(encoding="utf-8")
|
||||
except Exception as e:
|
||||
return f"ERROR reading {path!r}: {e}"
|
||||
|
||||
def list_directory(self, path: str) -> str:
|
||||
# ... similar pattern
|
||||
```
|
||||
|
||||
Each sub-MCP:
|
||||
- Exposes `name`, `description`, `tools` (dict), `invoke()` (Result-returning)
|
||||
- Uses `_resolve_and_check` for path-taking tools (delegated to the security module)
|
||||
- Uses the `Metadata` family aliases for dict parameters
|
||||
- Returns `Result[str, Any]` from `invoke()`; converts exceptions to `ErrorInfo` at the boundary
|
||||
|
||||
### 3.5 Module Layout
|
||||
|
||||
```
|
||||
src/
|
||||
mcp_client.py # MODIFIED: slim controller; re-exports from mcp_client_legacy for compat
|
||||
mcp_client_legacy.py # NEW: the OLD mcp_client.py code, re-exported
|
||||
mcp_client_security.py # NEW: the 3-layer security model
|
||||
mcp_file_io.py # NEW: FileIOMCP class
|
||||
mcp_python.py # NEW: PythonMCP class
|
||||
mcp_c.py # NEW: CMCP class
|
||||
mcp_cpp.py # NEW: CppMCP class
|
||||
mcp_web.py # NEW: WebMCP class
|
||||
mcp_analysis.py # NEW: AnalysisMCP class
|
||||
mcp_external.py # NEW: ExternalMCP class (refactor of ExternalMCPManager)
|
||||
|
||||
tests/
|
||||
test_mcp_client.py # NEW: controller tests (dispatch, registration, security)
|
||||
test_mcp_client_security.py # NEW: security model tests
|
||||
test_mcp_file_io.py # NEW: FileIOMCP tests
|
||||
test_mcp_python.py # NEW: PythonMCP tests
|
||||
test_mcp_c.py # NEW: CMCP tests
|
||||
test_mcp_cpp.py # NEW: CppMCP tests
|
||||
test_mcp_web.py # NEW: WebMCP tests
|
||||
test_mcp_analysis.py # NEW: AnalysisMCP tests
|
||||
test_mcp_external.py # NEW: ExternalMCP tests
|
||||
test_mcp_client_legacy.py # NEW: legacy shim tests (verify all 45+ old symbols are re-exported)
|
||||
test_mcp_client_beads.py # MODIFIED: existing; should pass unchanged
|
||||
test_mcp_config.py # MODIFIED: existing; should pass unchanged
|
||||
test_mcp_perf_tool.py # MODIFIED: existing; should pass unchanged
|
||||
test_mcp_ts_integration.py # MODIFIED: existing; should pass unchanged
|
||||
```
|
||||
|
||||
## 4. Per-Sub-MCP Design
|
||||
|
||||
### 4.1 File I/O (`mcp_file_io.py`)
|
||||
|
||||
**Tools (9):** read_file, list_directory, search_files, get_file_summary, get_file_slice, set_file_slice, edit_file, get_tree, get_git_diff
|
||||
|
||||
**Security:** all tools take `path: str` and use `_resolve_and_check` to validate.
|
||||
|
||||
**Returns:** `str` (the contents or error string). The `invoke()` method wraps in `Result[str, Any]`.
|
||||
|
||||
### 4.2 Python (`mcp_python.py`)
|
||||
|
||||
**Tools (14):** py_get_skeleton, py_get_code_outline, py_get_definition, py_get_signature, py_get_class_summary, py_get_var_declaration, py_get_hierarchy, py_get_docstring, py_get_symbol_info, py_find_usages, py_get_imports, py_check_syntax, py_update_definition, py_set_signature, py_set_var_declaration
|
||||
|
||||
**Security:** all take `path: str`; use `_resolve_and_check`.
|
||||
|
||||
**Returns:** `str` for read-only tools; `str` (the new content) for mutators.
|
||||
|
||||
### 4.3 C (`mcp_c.py`)
|
||||
|
||||
**Tools (5):** ts_c_get_skeleton, ts_c_get_code_outline, ts_c_get_definition, ts_c_get_signature, ts_c_update_definition
|
||||
|
||||
**Security:** path validation.
|
||||
|
||||
### 4.4 C++ (`mcp_cpp.py`)
|
||||
|
||||
**Tools (5):** ts_cpp_get_skeleton, ts_cpp_get_code_outline, ts_cpp_get_definition, ts_cpp_get_signature, ts_cpp_update_definition
|
||||
|
||||
**Security:** path validation.
|
||||
|
||||
### 4.5 Web (`mcp_web.py`)
|
||||
|
||||
**Tools (2):** web_search, fetch_url
|
||||
|
||||
**Security:** NO path validation. The Web sub-MCP handles URL validation internally (e.g., block internal IPs, no file:// scheme).
|
||||
|
||||
**Returns:** `str` (the search result or fetched content).
|
||||
|
||||
### 4.6 Analysis (`mcp_analysis.py`)
|
||||
|
||||
**Tools (2):** derive_code_path, get_ui_performance
|
||||
|
||||
**Security:** NO path validation (these tools don't take paths). `derive_code_path` takes a function/target name; `get_ui_performance` takes no arguments.
|
||||
|
||||
### 4.7 External (`mcp_external.py`)
|
||||
|
||||
**Class:** `ExternalMCP` (was `ExternalMCPManager`; the class name is preserved for compat).
|
||||
|
||||
**Methods:** `register_server(server)`, `unregister_server(name)`, `async_dispatch(tool_name, tool_input)`, `get_tool_schemas()`.
|
||||
|
||||
**Difference from native sub-MCPs:** the External MCP is NOT in `ALL_SUB_MCPS`; it's a sub-controller that the main controller delegates to AFTER the native sub-MCPs miss.
|
||||
|
||||
## 5. Migration / Rollout
|
||||
|
||||
| Phase | What | Risk |
|
||||
|---|---|---|
|
||||
| **Phase 1 — Foundation: security module + SubMCP Protocol + controller skeleton** | New `src/mcp_client_security.py`. New `MCPController` class in `src/mcp_client.py` (skeleton; no sub-MCPs yet). New `SubMCP` Protocol. Old `mcp_client.py` still has all 45 functions; the new controller is alongside. | Low. New files; the old code is untouched. |
|
||||
| **Phase 2 — Move old code to `mcp_client_legacy.py`; `mcp_client.py` becomes the shim** | Move the current `mcp_client.py` content to `src/mcp_client_legacy.py`. Replace `mcp_client.py` with a thin shim that re-exports all 45+ old symbols from `mcp_client_legacy`. | Low. Re-exports preserve the import surface; existing tests pass unchanged. |
|
||||
| **Phase 3 — Extract File I/O sub-MCP** | Create `src/mcp_file_io.py` with the `FileIOMCP` class. Register it in the controller. Update the existing `read_file`, `list_directory`, etc. functions in `mcp_client_legacy.py` to delegate to the File I/O sub-MCP (or remove them entirely; the legacy shim only re-exports what's not in a sub-MCP). | Medium. 9 functions moved. The dispatch function in the shim is updated to use the controller. |
|
||||
| **Phase 4 — Extract Python sub-MCP** | Create `src/mcp_python.py` with the `PythonMCP` class. Register. | Medium. 14 functions moved. |
|
||||
| **Phase 5 — Extract C, C++, Web, Analysis sub-MCPs** | One sub-MCP per phase task. Each extraction is a separate commit. | Medium each. 5 + 5 + 2 + 2 = 14 functions moved. |
|
||||
| **Phase 6 — Extract External sub-MCP** | Move the `ExternalMCPManager` class to `mcp_external.py` (class name preserved as `ExternalMCP`). | Low. The class is already self-contained. |
|
||||
| **Phase 7 — Update the dispatch + add security + use Result pattern; archive** | Update `dispatch` and `async_dispatch` to use the controller's `ALL_SUB_MCPS` lookup. Add the security check before path-taking tools. Convert the legacy shim to unwrap `Result.data` for backward compat. Update `docs/guide_mcp_client.md` (if it exists) with the new architecture. Archive the track. | Low. The dispatch is the central change; everything else flows from it. |
|
||||
|
||||
Each phase has its own checkpoint commit and git note.
|
||||
|
||||
## 6. Configuration
|
||||
|
||||
No new dependencies. The existing stdlib `ast`, `pathlib`, `dataclasses`, etc. are used. The `result_types.py` and `type_aliases.py` modules are already in place from the previous tracks.
|
||||
|
||||
## 7. Testing Strategy
|
||||
|
||||
| Test File | Purpose | Coverage Target |
|
||||
|---|---|---|
|
||||
| `tests/test_mcp_client.py` | Controller: registration, dispatch (O(1) lookup), security check before delegation, schema aggregation. | 90% |
|
||||
| `tests/test_mcp_client_security.py` | `_is_allowed`, `_resolve_and_check`, `configure` (with file_items + extra_base_dirs). | 100% |
|
||||
| `tests/test_mcp_file_io.py` | `FileIOMCP`: each tool's read/write behavior; security integration. | 90% |
|
||||
| `tests/test_mcp_python.py` | `PythonMCP`: each py_* tool. | 90% |
|
||||
| `tests/test_mcp_c.py` | `CMCP`: each ts_c_* tool. | 90% |
|
||||
| `tests/test_mcp_cpp.py` | `CppMCP`: each ts_cpp_* tool. | 90% |
|
||||
| `tests/test_mcp_web.py` | `WebMCP`: web_search, fetch_url; URL validation. | 90% |
|
||||
| `tests/test_mcp_analysis.py` | `AnalysisMCP`: derive_code_path, get_ui_performance. | 90% |
|
||||
| `tests/test_mcp_external.py` | `ExternalMCP`: register_server, async_dispatch, get_tool_schemas. | 90% |
|
||||
| `tests/test_mcp_client_legacy.py` | Verify all 45+ old symbols are re-exported from the legacy shim. | 100% |
|
||||
| `tests/test_mcp_client_beads.py` (existing) | Verify Beads tools work via the new architecture. | 100% (regression) |
|
||||
| `tests/test_mcp_config.py` (existing) | Verify config-related MCP tools work. | 100% (regression) |
|
||||
| `tests/test_mcp_perf_tool.py` (existing) | Verify the perf tool works. | 100% (regression) |
|
||||
| `tests/test_mcp_ts_integration.py` (existing) | Verify the ts_c / ts_cpp integration tests work. | 100% (regression) |
|
||||
|
||||
## 8. Risks & Mitigations
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|---|---|---|---|
|
||||
| One of the 45+ function extractions introduces a regression. | Medium | Medium | Per-MCP unit tests + the existing 4 test files serve as regression tests. The legacy shim re-exports the old symbols, so the 4 test files don't need to change. |
|
||||
| The dispatch inversion (if/elif → dict lookup) breaks some edge case (e.g., tool_name aliases). | Low | Low | The new dispatch preserves the existing alias behavior (`path` / `file_path` / `dir_path` are normalized in the current dispatch; the new dispatch does the same). |
|
||||
| The `mcp_client_legacy.py` shim becomes permanent (never removed). | Medium | Low (acceptable) | The `public_api_migration_20260606` follow-up track (from the data_oriented_error_handling track) is the natural place to remove the legacy shim. |
|
||||
| The `Result[str, Any]` return type from sub-MCPs is incompatible with the existing tests' `assert dispatch(...) == "text"` pattern. | Low | Low | The legacy shim's `dispatch` unwraps `.data` so existing tests see the same string. New tests can check `.data` and `.errors` directly. |
|
||||
| The new sub-MCP architecture is "overkill" for the project's scale. | Low | Low (subjective) | The current 2,205-line file is the largest in the project; even if only 30% of the function count grew 2x in the next year, the file would be unmanageable. The investment now is bounded; the maintenance cost avoided is unbounded. |
|
||||
| The DSL future becomes "we have to do it now" before this track is done. | Low | Low | The DSL is explicitly out of scope. This track stays JSON-compatible. A future DSL track can layer on top without breaking the architecture. |
|
||||
|
||||
## 9. Out of Scope (Explicit)
|
||||
|
||||
- **MCP DSL (APL/K/Cosy-inspired compact tool-call format).** Deferred to a future track; documented in §13.1.
|
||||
- **Migrating to `TypedDict` schemas for tool arguments.** The `Metadata` family aliases are used; the deeper schema is deferred to `typed_dict_migration_20260606`.
|
||||
- **Adding new tool categories beyond the 7.** If a future tool doesn't fit, that's a separate track.
|
||||
- **Removing the `mcp_client_legacy.py` shim.** Deferred to the `public_api_migration_20260606` follow-up.
|
||||
- **Touching the agent runtime's tool-calling format.** The format is unchanged.
|
||||
- **Performance optimizations** (e.g., caching tool schemas, lazy-loading sub-MCPs). Out of scope; can be a follow-up.
|
||||
|
||||
## 10. Open Questions
|
||||
|
||||
1. **Sub-MCP implementation style.** The spec uses a class with `name` / `description` / `tools` / `invoke()`. Alternative: a module-level function `register(controller) -> None` that does the registration. (Proposal: class is the primary; module-level is an alternative for simple cases. Both are supported by the Protocol.)
|
||||
2. **The `ExternalMCP` class name.** The spec preserves the existing `ExternalMCPManager` name (to avoid breaking the import surface). The new file is `mcp_external.py`. Should the class also be renamed to `ExternalMCP` (dropping the `Manager` suffix)? (Proposal: keep the existing name for now; the class name change is a separate concern. The file rename + class-internal refactor is enough for this track.)
|
||||
3. **Backward compat scope.** The legacy shim re-exports all 45+ old function names. Should it also re-export the old `dispatch` and `async_dispatch` signatures (the current if/elif chain), or should the old function names delegate to the new controller? (Proposal: the old function names remain as functions (they may be called directly from `app_controller.py:61`); the old `dispatch` function in the shim is REPLACED by the new controller's `dispatch`.)
|
||||
|
||||
## 11. Configuration
|
||||
|
||||
No new environment variables. The existing `config.toml` is unchanged. The `extra_base_dirs` and `file_items` security configuration is set by `app_controller.py` at startup (unchanged).
|
||||
|
||||
## 12. See Also
|
||||
|
||||
### 12.1 Follow-up Track (planned; not in this spec)
|
||||
|
||||
**"MCP DSL Track"** (`mcp_dsl_20260606` or similar) — introduces a per-MCP compact dialect for tool calls, replacing or augmenting the JSON format. Inspired by the user's notes on APL/K/Cosy DSLs. Examples:
|
||||
- JSON: `{"name": "py_get_skeleton", "arguments": "{\"path\": \"/src/foo.py\"}"}` (~80 tokens per call)
|
||||
- DSL: `py k /src/foo.py` (~10 tokens per call, ~8x reduction)
|
||||
- A per-MCP grammar definition (`py_grammar.k`, `file_io_grammar.k`, etc.) could be authored and compiled to a parser
|
||||
- A per-MCP DSL → JSON converter at the dispatch boundary
|
||||
- Backward compat: the JSON path stays; the DSL is opt-in per MCP
|
||||
|
||||
Prerequisites: this track (the sub-MCP architecture is the natural unit to pair with a DSL).
|
||||
|
||||
### 12.2 Project References
|
||||
|
||||
- `docs/guide_ai_client.md` "Data-Oriented Error Handling (Fleury Pattern)" — the `Result[T]` pattern used by sub-MCPs.
|
||||
- `docs/guide_mcp_client.md` (if it exists; will be created/updated) — the in-context guide for the MCP layer.
|
||||
- `conductor/code_styleguides/error_handling.md` (from `data_oriented_error_handling_20260606`) — the `Result` / `ErrorInfo` convention.
|
||||
- `conductor/code_styleguides/type_aliases.md` (from `data_structure_strengthening_20260606`) — the `Metadata` family aliases used by sub-MCPs.
|
||||
- `conductor/tracks/data_oriented_error_handling_20260606/` — the previous track that established the `Result` pattern.
|
||||
- `conductor/tracks/data_structure_strengthening_20260606/` — the previous track that established the `Metadata` aliases.
|
||||
- `conductor/tracks/public_api_migration_20260606/` (planned; from data_oriented_error_handling) — the natural track to remove the `mcp_client_legacy.py` shim.
|
||||
|
||||
### 12.3 External References
|
||||
|
||||
- **Ryan Fleury on module layer boundaries** — the convention that each module owns its data and exposes a clean interface; consumers adapt. The sub-MCP architecture follows this: each sub-MCP owns its tools; the controller owns dispatch; the security module owns validation.
|
||||
- **Mike Acton on data-oriented design** — the "data is the API" framing. The `Result[str, ErrorInfo]` returned by `invoke()` is the API; sub-MCPs transform inputs to this shape.
|
||||
- **Casey Muratori on Handmade Hero** — the spirit of explicit, self-contained modules with no magic. The `ALL_SUB_MCPS` registration at the bottom of `mcp_client.py` is explicit; no auto-discovery magic.
|
||||
- **The user's friend on APL/K/Cosy DSLs for tool calling** — the inspiration for the future DSL track (§13.1).
|
||||
@@ -0,0 +1,110 @@
|
||||
# Track state for mcp_architecture_refactor_20260606
|
||||
# Updated by Tier 2 Tech Lead as tasks complete
|
||||
|
||||
[meta]
|
||||
track_id = "mcp_architecture_refactor_20260606"
|
||||
name = "MCP Architecture Refactor (Sub-MCP Extraction)"
|
||||
status = "active"
|
||||
current_phase = 0
|
||||
last_updated = "2026-06-06"
|
||||
|
||||
[blocked_by]
|
||||
data_oriented_error_handling_20260606 = "merged"
|
||||
data_structure_strengthening_20260606 = "merged"
|
||||
|
||||
[blocks]
|
||||
mcp_dsl_20260606 = "planned in spec §12.1; the future DSL track"
|
||||
|
||||
[phases]
|
||||
phase_1 = { status = "pending", checkpointsha = "", name = "Foundation: security module + SubMCP Protocol + controller skeleton" }
|
||||
phase_2 = { status = "pending", checkpointsha = "", name = "Move old code to mcp_client_legacy.py; mcp_client.py becomes the shim" }
|
||||
phase_3 = { status = "pending", checkpointsha = "", name = "Extract File I/O sub-MCP" }
|
||||
phase_4 = { status = "pending", checkpointsha = "", name = "Extract Python sub-MCP" }
|
||||
phase_5 = { status = "pending", checkpointsha = "", name = "Extract C, C++, Web, Analysis sub-MCPs" }
|
||||
phase_6 = { status = "pending", checkpointsha = "", name = "Extract External sub-MCP" }
|
||||
phase_7 = { status = "pending", checkpointsha = "", name = "Update dispatch + Result integration + docs + archive" }
|
||||
|
||||
[tasks]
|
||||
# Phase 1: Foundation
|
||||
t1_1 = { status = "pending", commit_sha = "", description = "Red: tests/test_mcp_client_security.py (8+ tests: _is_allowed positive/negative, _resolve_and_check, configure, Result[Path] return)" }
|
||||
t1_2 = { status = "pending", commit_sha = "", description = "Green: create src/mcp_client_security.py with _is_allowed, _resolve_and_check, configure (all return Result[Path], use Metadata, NilPath)" }
|
||||
t1_3 = { status = "pending", commit_sha = "", description = "Red: tests/test_mcp_client.py (controller skeleton: SubMCP Protocol, MCPController class with register/dispatch/get_tool_schemas; no sub-MCPs yet)" }
|
||||
t1_4 = { status = "pending", commit_sha = "", description = "Green: add SubMCP Protocol + MCPController class skeleton to src/mcp_client.py (alongside the existing 45 functions; the controller is alongside, not replacing)" }
|
||||
t1_5 = { status = "pending", commit_sha = "", description = "Verify the 4 existing test files still pass (no regression: mcp_client.py is unchanged at this point)" }
|
||||
t1_6 = { status = "pending", commit_sha = "", description = "Phase 1 checkpoint commit + git note" }
|
||||
# Phase 2: Move to legacy
|
||||
t2_1 = { status = "pending", commit_sha = "", description = "Use git mv to move src/mcp_client.py to src/mcp_client_legacy.py" }
|
||||
t2_2 = { status = "pending", commit_sha = "", description = "Create a new src/mcp_client.py that re-exports all 45+ old symbols from mcp_client_legacy" }
|
||||
t2_3 = { status = "pending", commit_sha = "", description = "Red: tests/test_mcp_client_legacy.py (verify all 45+ old symbols are still importable from src.mcp_client)" }
|
||||
t2_4 = { status = "pending", commit_sha = "", description = "Run all 4 existing test files; confirm no regressions (they import from src.mcp_client which is now the shim)" }
|
||||
t2_5 = { status = "pending", commit_sha = "", description = "Run src/app_controller.py:61 usage; confirm mcp_client.py_get_symbol_info is accessible via the shim" }
|
||||
t2_6 = { status = "pending", commit_sha = "", description = "Phase 2 checkpoint commit + git note" }
|
||||
# Phase 3: Extract File I/O
|
||||
t3_1 = { status = "pending", commit_sha = "", description = "Red: tests/test_mcp_file_io.py (9+ tests: one per FileIOMCP tool, plus security integration)" }
|
||||
t3_2 = { status = "pending", commit_sha = "", description = "Green: create src/mcp_file_io.py with FileIOMCP class (read_file, list_directory, search_files, get_file_summary, get_file_slice, set_file_slice, edit_file, get_tree, get_git_diff)" }
|
||||
t3_3 = { status = "pending", commit_sha = "", description = "Register FileIOMCP in the controller (add 2 lines to src/mcp_client.py: import + register call)" }
|
||||
t3_4 = { status = "pending", commit_sha = "", description = "Verify: existing tests pass; the dispatch function in mcp_client_legacy.py still works (FileIOMCP is registered alongside, not replacing)" }
|
||||
t3_5 = { status = "pending", commit_sha = "", description = "Phase 3 checkpoint commit + git note" }
|
||||
# Phase 4: Extract Python
|
||||
t4_1 = { status = "pending", commit_sha = "", description = "Red: tests/test_mcp_python.py (14+ tests: one per py_* tool)" }
|
||||
t4_2 = { status = "pending", commit_sha = "", description = "Green: create src/mcp_python.py with PythonMCP class" }
|
||||
t4_3 = { status = "pending", commit_sha = "", description = "Register PythonMCP in the controller" }
|
||||
t4_4 = { status = "pending", commit_sha = "", description = "Verify: existing tests pass; especially test_mcp_ts_integration.py for any py_* related integration" }
|
||||
t4_5 = { status = "pending", commit_sha = "", description = "Phase 4 checkpoint commit + git note" }
|
||||
# Phase 5: Extract C, C++, Web, Analysis
|
||||
t5_1 = { status = "pending", commit_sha = "", description = "Red + Green: src/mcp_c.py with CMCP class; register; 5+ tests" }
|
||||
t5_2 = { status = "pending", commit_sha = "", description = "Red + Green: src/mcp_cpp.py with CppMCP class; register; 5+ tests" }
|
||||
t5_3 = { status = "pending", commit_sha = "", description = "Red + Green: src/mcp_web.py with WebMCP class; URL validation; register; 4+ tests" }
|
||||
t5_4 = { status = "pending", commit_sha = "", description = "Red + Green: src/mcp_analysis.py with AnalysisMCP class; register; 4+ tests" }
|
||||
t5_5 = { status = "pending", commit_sha = "", description = "Phase 5 checkpoint commit + git note" }
|
||||
# Phase 6: Extract External
|
||||
t6_1 = { status = "pending", commit_sha = "", description = "Red: tests/test_mcp_external.py (4+ tests: register_server, async_dispatch, get_tool_schemas, unregister_server)" }
|
||||
t6_2 = { status = "pending", commit_sha = "", description = "Green: create src/mcp_external.py with ExternalMCP class (the existing ExternalMCPManager refactored; class name preserved)" }
|
||||
t6_3 = { status = "pending", commit_sha = "", description = "Wire the controller to delegate to ExternalMCP AFTER native sub-MCPs miss (in dispatch())" }
|
||||
t6_4 = { status = "pending", commit_sha = "", description = "Verify: test_mcp_client_beads.py (existing) still passes (the Beads MCP is an external)" }
|
||||
t6_5 = { status = "pending", commit_sha = "", description = "Phase 6 checkpoint commit + git note" }
|
||||
# Phase 7: Update dispatch + Result integration + docs + archive
|
||||
t7_1 = { status = "pending", commit_sha = "", description = "Update mcp_client_legacy.py's dispatch() to use the new controller's dispatch() (delegate to MCPController)" }
|
||||
t7_2 = { status = "pending", commit_sha = "", description = "Verify the dispatch now returns Result[str, ErrorInfo]; the legacy shim unwraps .data so existing tests see strings" }
|
||||
t7_3 = { status = "pending", commit_sha = "", description = "Update docs/guide_mcp_client.md (if exists) with the new architecture diagram + per-MCP reference" }
|
||||
t7_4 = { status = "pending", commit_sha = "", description = "Manual smoke test: launch GUI; trigger one tool from each sub-MCP; verify it works" }
|
||||
t7_5 = { status = "pending", commit_sha = "", description = "Final state.toml update; mark all phases completed; git mv to archive; update tracks.md" }
|
||||
t7_6 = { status = "pending", commit_sha = "", description = "Phase 7 checkpoint commit + git note (TRACK COMPLETE)" }
|
||||
|
||||
[verification]
|
||||
# Filled as phases complete
|
||||
phase_1_foundation_complete = false
|
||||
phase_2_legacy_shim_complete = false
|
||||
phase_3_file_io_extracted = false
|
||||
phase_4_python_extracted = false
|
||||
phase_5_c_cpp_web_analysis_extracted = false
|
||||
phase_6_external_extracted = false
|
||||
phase_7_dispatch_updated_and_archived = false
|
||||
full_test_suite_passes = false
|
||||
no_new_optional_introduced = false
|
||||
existing_test_files_pass_unchanged = false
|
||||
|
||||
[line_count_progression]
|
||||
# Filled as phases complete; original mcp_client.py was 2205 lines
|
||||
phase_1_start = 2205
|
||||
phase_2_after_move = 2205 # same code, just in legacy file
|
||||
phase_3_after_file_io = 2205 - 200 # approx 200 lines for FileIOMCP extracted
|
||||
phase_4_after_python = 0 # approx 200 more lines extracted
|
||||
phase_5_after_c_cpp_web_analysis = 0 # approx 400 more lines
|
||||
phase_6_after_external = 0 # approx 200 more lines
|
||||
phase_7_final_mcp_client_py = 200 # controller + shim re-exports
|
||||
|
||||
[sub_mcp_extraction_status]
|
||||
file_io = { status = "pending", tools_extracted = 0, of_total = 9 }
|
||||
python = { status = "pending", tools_extracted = 0, of_total = 14 }
|
||||
c = { status = "pending", tools_extracted = 0, of_total = 5 }
|
||||
cpp = { status = "pending", tools_extracted = 0, of_total = 5 }
|
||||
web = { status = "pending", tools_extracted = 0, of_total = 2 }
|
||||
analysis = { status = "pending", tools_extracted = 0, of_total = 2 }
|
||||
external = { status = "pending", class_extracted = false }
|
||||
|
||||
[mcp_dsl_followup]
|
||||
track_id = "mcp_dsl_20260606"
|
||||
status = "planned_in_mcp_architecture_refactor_20260606"
|
||||
goal = "Introduce a per-MCP compact dialect for tool calls (APL/K/Cosy-inspired), replacing or augmenting JSON. Estimated 5x token reduction per call."
|
||||
note = "Per user feedback (2026-06-06): 'kinda want to compress the mcp to just have a single intention based DSL per mcp, kinda like command line but more flexible'. Out of scope for this track; this track lays the architectural groundwork (sub-MCPs are the natural unit to pair with a DSL emitter)."
|
||||
Reference in New Issue
Block a user