ed/manual_slop
Private
Public Access
0
0
Fork 0
Code Activity

conductor(track-update): mcp_architecture_refactor - list_tool_schemas + security-as-contract

Browse Source 
4 surgical additions to the spec, no task changes:

1. list_tool_schemas on the SubMCP Protocol: Added the method
   to §3.1 (The SubMCP Protocol). Per nagent_review Pitfall #6
   (hard-coded tool discovery) and takeaway #5 (self-describing
   tools), each sub-MCP advertises its own capabilities via
   list_tool_schemas() rather than relying on a central registry.
   This is the equivalent of nagent's collect_bin_tool_descriptions
   per sub-MCP. The MCPController.get_tool_schemas() becomes a
   simple aggregator.

2. Security model is the contract: Added a new Important note
   to §3.3 (The 3-Layer Security Model). The 3 layers
   (Allowlist Construction -> Path Validation -> Resolution
   Gate, per docs/guide_mcp_client.md) are not just refactored
   - they are the CONTRACT between MCPController and the
   sub-MCPs. Sub-MCPs receive a pre-validated Path and trust
   it. They do NOT re-validate. The refactor is structural,
   not security-changing.

3. Docs touchpoint in Phase 7: Added the docs touchpoint to
   Phase 7 per the docs Refresh Protocol. The update to
   docs/guide_mcp_client.md should add a Sub-MCP Architecture
   section, link the list_tool_schemas pattern to 3-Layer
   Security Model, and cross-link the 3 new guides from
   the 2026-06-08 docs refresh.

4. See Also cross-references: Added 8 new entries to §12.2:
   - docs/guide_context_aggregation.md (FileItem consumer)
   - docs/guide_state_lifecycle.md (App state delegation)
   - docs/guide_discussions.md (23-operation matrix)
   - conductor/tracks/qwen_llama_grok_integration_20260606/
     (Result return type coordination)
   - conductor/tracks/nagent_review_20260608/{report,takeaways}.md
   - (2 specific data_oriented_error_handling and
     data_structure_strengthening cross-refs)

No plan.md changes.
This commit is contained in:
conductor-tier2
2026-06-08 20:59:27 -04:00
parent 1fb0d79c0d
commit 8a597d1832
1 changed files with 30 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
conductor/tracks/mcp_architecture_refactor_20260606/spec.md
+30 -5
View File
@@ -69,9 +69,21 @@ class SubMCP(Protocol):
description: str
tools: dict[str, Callable[..., str]]
def invoke(self, tool_name: str, args: dict[str, Any]) -> Result[str, Any]: ...
def list_tool_schemas(self) -> list[dict[str, Any]]:
"""Return the JSON-serializable tool schemas for this sub-MCP's tools.
Used by MCPController.get_tool_schemas() to aggregate the full list
for the AI's initial context. Per nagent_review takeaway #5 (the
self-describing tool pattern), this is the data-driven alternative
to a hard-coded dispatch chain. Implementations return OpenAI-
shaped tool definitions (the same shape that the existing
mcp_client.get_tool_schemas() returns).
"""
...
```
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.**
The `tools` dict is the public API: tool_name → function. The `invoke` method is the dispatch entry point. The `list_tool_schemas` method is the *self-describing* interface — the sub-MCP advertises its own capabilities rather than relying on a central registry. 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.**
> **Note (added 2026-06-08 per nagent_review Pitfall #6 + takeaway #5).** The current `src/mcp_client.py:dispatch` is a flat 45-branch `if/elif` chain (per `docs/guide_mcp_client.md` and the nagent_review deep-dive). The new sub-MCP structure replaces this with the `SubMCP.list_tool_schemas()` pattern. Each sub-MCP **owns its own tool list** (the dict, the schemas, the dispatch); `MCPController` is the aggregator. This is the equivalent of nagent's `collect_bin_tool_descriptions` per sub-MCP.
### 3.2 The `MCPController` Class
@@ -122,6 +134,13 @@ The controller is a module-level singleton. The `ALL_SUB_MCPS` list is implicit
### 3.3 The 3-Layer Security Model
**Important (added 2026-06-08):** the 3-layer security model (Allowlist Construction → Path Validation → Resolution Gate, per `docs/guide_mcp_client.md`) is not just refactored — it is the **contract** between `MCPController` and the sub-MCPs. Sub-MCPs receive a *pre-validated* `pathlib.Path` from `_resolve_and_check` and trust it. They do *not* re-validate. This is the security invariant that the refactor must preserve: the 3 layers run *before* the sub-MCP's `invoke()` is called, and the sub-MCP treats the path as already-allowed.
Concrete consequences:
- `_resolve_and_check` is called by `MCPController.dispatch` *before* the sub-MCP's `invoke()`. The sub-MCP sees a `Result[Path]` and the `data` field is either a real `Path` (allowed) or a `NilPath` (denied).
- Sub-MCPs that take a `path: str` parameter call `_resolve_and_check` themselves in their `invoke()` (or, if the path is already validated, they skip it). The current `src/mcp_client.py:_resolve_and_check` is moved to `src/mcp_client_security.py` unchanged.
- The 3-layer pattern is *not* weakened by the refactor. The `_is_allowed` check (Layer 1) still uses `_ALLOWED_BASE_DIRS`; the resolution (Layer 3) still uses `Path.resolve()`. The refactor is a *structural* change, not a *security* change.
`src/mcp_client_security.py` (NEW):
```python
@@ -318,7 +337,7 @@ tests/
| **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. |
| **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` with the new architecture. **Docs touchpoint (added 2026-06-08 per the docs Refresh Protocol):** `docs/guide_mcp_client.md` documents the 3-layer security model and the 45 tools; the refactor changes the *implementation* (sub-MCPs) but not the *security invariant* or the tool surface. The update should add a §"Sub-MCP Architecture" section describing the new layout, link the `SubMCP.list_tool_schemas()` pattern to `docs/guide_mcp_client.md §"3-Layer Security Model"`, and cross-link `docs/guide_context_aggregation.md` (the new pipeline guide, which `mcp_file_io.py` consumers use) and `docs/guide_state_lifecycle.md` (which documents the `App.__getattr__`/`__setattr__` state delegation that sub-MCPs must respect). 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.
@@ -448,12 +467,18 @@ Prerequisites: this track (the sub-MCP architecture is the natural unit to pair
### 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.
- `docs/guide_mcp_client.md` (if it exists; will be created/updated) — the in-context guide for the MCP layer. **Added 2026-06-08:** the docs refresh created this guide; it documents the 45 tools, the 3-layer security model, and the `dispatch()`/`async_dispatch()` entry points. The Phase 7 update for this track should add a §"Sub-MCP Architecture" section.
- `docs/guide_context_aggregation.md` — added 2026-06-08. The `aggregate.py:142 build_file_items` function consumes the `FileItem` list and is the *upstream* consumer of `mcp_file_io.py`. The sub-MCP refactor must preserve the `FileItem` schema documented in §"The FileItem Schema (Full)".
- `docs/guide_state_lifecycle.md` — added 2026-06-08. The `App.__getattr__`/`__setattr__` state delegation (per `gui_2.py:666-675`) and the `UISnapshot` capture/restore are the *correctness* the sub-MCP refactor must preserve; sub-MCP tools are called from the `App` instance and any state mutation must go through the Controller.
- `docs/guide_discussions.md` — added 2026-06-08. The 23-operation matrix (A1-A7 + B1-B11 + C1-C5) drives several sub-MCP tool calls (read_file, py_get_skeleton, etc.); the refactor must not change the tool-call surface.
- `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/data_oriented_error_handling_20260606/` — the previous track that established the `Result` pattern. Specifically: the new `ErrorKind.PROVIDER_HISTORY_DIVERGED_FROM_UI` kind (added 2026-06-08) is a *future* error category the sub-MCPs may surface.
- `conductor/tracks/data_structure_strengthening_20260606/` — the previous track that established the `Metadata` aliases. Specifically: the `FileItem` alias is the only alias in the 10 that points to a concrete dataclass (`models.FileItem`), not `Metadata`; sub-MCPs that consume `FileItem` should use the dataclass directly, not a dict round-trip.
- `conductor/tracks/qwen_llama_grok_integration_20260606/` — the parallel major track. The `send_openai_compatible()` helper is *expected* to return `Result` from day 1 (per the qwen spec §3.1 coordination note). The MCP refactor composes with this; the sub-MCP `invoke()` returns `Result[str, ErrorInfo]` and the helper returns `Result[NormalizedResponse, ErrorInfo]` — same shape, different layer.
- `conductor/tracks/public_api_migration_20260606/` (planned; from data_oriented_error_handling) — the natural track to remove the `mcp_client_legacy.py` shim.
- `conductor/tracks/nagent_review_20260608/report.md` — added 2026-06-08. §12 (Tool discovery) and §15 Pitfall #6 (hard-coded tool discovery) directly motivate this track's refactor. The 23-operation matrix in §3 (Conversations are editable state) is a use-case the sub-MCPs must continue to serve.
- `conductor/tracks/nagent_review_20260608/nagent_takeaways_20260608.md` — added 2026-06-08. §8 (self-describing tools / nagent `--description` pattern) is the conceptual model for the new `SubMCP.list_tool_schemas()` method.
### 12.3 External References