fix(engine,wrapper): proper path-based MCP config delivery (protocol 0.2.0)

CHANGELOG.md

@@ -5,6 +5,47 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.3.0 engine / 0.4.0 wrapper] — 2026-05-27
+
+### Fixed
+
+- **Engine** `_runtime.py` — three latent runtime-crashing bugs in MCP server config handling, all silenced by `# pyright: ignore` suppressions:
+  - `AttributeError: 'PreparedBundle' object has no attribute 'config'` — author wrote prose comments asserting `PreparedBundle.config` was the merged bundle yaml; it does not exist. The merged yaml lives on `mount_plan`.
+  - `AttributeError: 'list' object has no attribute 'get'` — `mount_plan["tools"]` is a list of `{module, source, config}` dicts, not a dict keyed by module name. The author treated it as a dict.
+  - `TypeError: PreparedBundle.create_session() got an unexpected keyword argument 'tool_overrides'` — the kwarg does not exist on the foundation API.
+  Each suppression masked a real attribute or call error pyright had flagged. The whole `--mcp-servers` flow was non-functional at 0.2.0; the file-based discovery paths documented in `amplifier-module-tool-mcp` continued to work.
+
+### Changed
+
+- **Wire (BREAKING)** `PROTOCOL_VERSION` bumped `0.1.0` → `0.2.0`. MCP server delivery refactored from inline `mcpServers: dict` to path-based `mcpConfigPath: str`. The engine forwards the path to `tool-mcp` via `AMPLIFIER_MCP_CONFIG` (one of four documented config priorities in the module). Old wrappers fail with a clean `protocol_version_mismatch` rather than a confusing runtime crash.
+- **Engine CLI** `--mcp-servers` flag renamed to `--mcp-config-path`. The engine no longer parses MCP config contents — it validates the path exists and forwards it to the module.
+- **Wrapper** `mcp-spill.ts` now always spills to a `0600` tmpfile (dropping the inline-JSON-on-argv branch — also eliminates server-config visibility in `ps aux`) and writes content in the format the module expects (`{"mcpServers": <map>}`).
+- **Wrapper** `resolveMcpServersFlag` → `resolveMcpConfigPath`; `McpSpillResult.flag` → `configPath`; `mcpServersFlag` in `AssembleArgvInput` → `mcpConfigPath`.
+- **Engine** audit field renamed `mcpServersDigest` → `mcpConfigPathDigest` (hashes the path string for stable identifier without dragging file IO into the audit path).
+
+### Architecture
+
+Each layer now owns exactly one responsibility:
+
+| Layer | Responsibility |
+|---|---|
+| Wrapper | Write the file in the format the module expects; manage tmpfile lifecycle |
+| Engine CLI | Validate that the path exists; forward to runtime |
+| Engine runtime | One line: `os.environ["AMPLIFIER_MCP_CONFIG"] = mcp_config_path` |
+| Module (`amplifier-module-tool-mcp`, unchanged) | Read the file via its existing 4-source config priority |
+
+The previous design tried to merge MCP config inside the engine via a non-existent `tool_overrides` kwarg, building parallel rails to a config-discovery mechanism the module already implemented.
+
+### Released
+
+- `amplifier-agent` (engine) 0.3.0 — consumers pin via `git+https://github.com/microsoft/amplifier-agent@engine-v0.3.0`. PyPI publishing not yet wired.
+- `amplifier-agent-ts` (wrapper) 0.4.0 — published to npm with provenance via the existing OIDC trusted-publishing workflow (`publish-wrapper.yml` on `wrapper-v*` tag push).
+
+### Migration
+
+- **Wrapper callers**: no API change. `SpawnAgentParams.mcpServers: Record<string, McpServerConfig>` is preserved. The wire-level rename is internal to the wrapper.
+- **Container/Dockerfile consumers (e.g. nanoclaw)**: bump `AMPLIFIER_AGENT_REF` to `engine-v0.3.0` and bump the wrapper dependency to `0.4.0`. The pair must move together — protocol mismatch errors are explicit and ship with remediation hints.
+
 ## [0.2.0] — 2026-05-22
 
 ### Added

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = 'amplifier-agent'
-version = '0.2.0'
+version = '0.3.0'
 requires-python = '>=3.12'
 license = 'MIT'
 dependencies = [

src/amplifier_agent_cli/modes/single_turn.py

@@ -173,7 +173,7 @@ def _write_audit(
     started_at: str,
     ended_at: str,
     argv: list[str],
-    mcp_servers: dict[str, Any] | None,
+    mcp_config_path: str | None,
     env_allowlist: list[str] | None,
     env_extra: dict[str, Any] | None,
     host_capabilities: dict[str, Any] | None,
@@ -188,7 +188,9 @@ def _write_audit(
     audits_dir.mkdir(parents=True, exist_ok=True)
     audit = {
         "argvDigest": _sha256(" ".join(argv)),
-        "mcpServersDigest": (_sha256(json.dumps(mcp_servers, sort_keys=True)) if mcp_servers else None),
+        # Path is non-secret; hashing gives a stable identifier for audit
+        # correlation without dragging file I/O into the audit path.
+        "mcpConfigPathDigest": (_sha256(mcp_config_path) if mcp_config_path else None),
         "envDigest": _sha256(json.dumps({"allow": env_allowlist or [], "extra": env_extra or {}}, sort_keys=True)),
         "hostCapabilities": host_capabilities,
         "protocolVersion": protocol_version,
@@ -323,7 +325,7 @@ class _TurnSpec:
     display: CliDisplaySystem
     provider: str  # detected provider short-name (e.g. 'anthropic')
     allow_protocol_skew: bool = False
-    mcp_servers: dict[str, Any] | None = None
+    mcp_config_path: str | None = None
 
 
 # ---------------------------------------------------------------------------
@@ -355,7 +357,7 @@ async def _execute_turn(spec: _TurnSpec) -> dict[str, Any]:
         prepared,
         cwd=spec.cwd,
         is_resumed=spec.resume and not spec.fresh,
-        mcp_servers=spec.mcp_servers,
+        mcp_config_path=spec.mcp_config_path,
     )
     engine = Engine(
         turn_handler=handler,
@@ -417,10 +419,10 @@ async def _execute_turn(spec: _TurnSpec) -> dict[str, Any]:
     help="Output mode: 'json' (default, envelope) or 'text' (reply only).",
 )
 @click.option(
-    "--mcp-servers",
-    "mcp_servers_raw",
+    "--mcp-config-path",
+    "mcp_config_path",
     default=None,
-    help="MCP servers config as inline JSON or '@<path>' to JSON file.",
+    help="Path to MCP config JSON (see amplifier-module-tool-mcp for the schema; written by the host/wrapper).",
 )
 @click.option(
     "--allow-protocol-skew",
@@ -468,7 +470,7 @@ def run(
     no_flag: bool,
     quiet: bool,
     output_mode: str,
-    mcp_servers_raw: str | None,
+    mcp_config_path: str | None,
     allow_protocol_skew: bool,
     host_capabilities_raw: str | None,
     env_allowlist_raw: str | None,
@@ -524,9 +526,15 @@ def run(
         stream=sys.stderr,
     )
 
-    # (5b) Parse --mcp-servers (inline JSON or @path).  Emits a §4.1 error
-    # envelope and exits 2 on parse / IO / type errors.
-    mcp_servers = _parse_json_or_atpath(mcp_servers_raw, flag_name="--mcp-servers")
+    # (5b) Validate --mcp-config-path is a real file if provided. The file's
+    # contents are not parsed here — the engine forwards the path to
+    # tool-mcp via AMPLIFIER_MCP_CONFIG and the module reads/validates.
+    if mcp_config_path is not None:
+        if not Path(mcp_config_path).is_file():
+            _emit_argv_envelope(
+                "mcp_config_path_invalid",
+                f"--mcp-config-path: file not found: {mcp_config_path}",
+            )
 
     # (5c) Parse host capabilities, env extras, and env allowlist (A1'/D12').
     # env_extra and env_allowlist are parsed here but threaded into the engine
@@ -560,7 +568,7 @@ def run(
         display=display,
         provider=provider_name,
         allow_protocol_skew=allow_protocol_skew or bool(os.environ.get("AMPLIFIER_AGENT_ALLOW_PROTOCOL_SKEW")),
-        mcp_servers=mcp_servers,
+        mcp_config_path=mcp_config_path,
     )
 
     # (7) Run with error handling.
@@ -605,7 +613,7 @@ def run(
             started_at=started_iso,
             ended_at=datetime.now(UTC).isoformat(),
             argv=sys.argv,
-            mcp_servers=mcp_servers,
+            mcp_config_path=mcp_config_path,
             env_allowlist=env_allowlist,
             env_extra=env_extra,
             host_capabilities=host_capabilities,
@@ -633,7 +641,7 @@ def run(
             started_at=started_iso,
             ended_at=datetime.now(UTC).isoformat(),
             argv=sys.argv,
-            mcp_servers=mcp_servers,
+            mcp_config_path=mcp_config_path,
             env_allowlist=env_allowlist,
             env_extra=env_extra,
             host_capabilities=host_capabilities,
@@ -663,7 +671,7 @@ def run(
         started_at=started_iso,
         ended_at=datetime.now(UTC).isoformat(),
         argv=sys.argv,
-        mcp_servers=mcp_servers,
+        mcp_config_path=mcp_config_path,
         env_allowlist=env_allowlist,
         env_extra=env_extra,
         host_capabilities=host_capabilities,

src/amplifier_agent_lib/_runtime.py

@@ -5,13 +5,15 @@
 replay; OpenClaw pattern).
 
 ``handle_initialize`` is the wire-side entry point that loads the prepared
-bundle, threads wire-supplied ``mcpServers`` into ``tool-mcp.mount()`` via
-``tool_overrides``, and stores ``host.capabilities`` on ``session.metadata``.
+bundle, forwards a wire-supplied ``mcpConfigPath`` to ``tool-mcp`` via
+``AMPLIFIER_MCP_CONFIG``, and stores ``host.capabilities`` on
+``session.metadata``.
 """
 
 from __future__ import annotations
 
 import logging
+import os
 from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
@@ -34,7 +36,7 @@ def make_turn_handler(
     *,
     cwd: str | None,
     is_resumed: bool,
-    mcp_servers: dict[str, Any] | None = None,
+    mcp_config_path: str | None = None,
 ) -> TurnHandler:
     """Return a TurnHandler closed over the loaded PreparedBundle.
 
@@ -54,13 +56,13 @@ def make_turn_handler(
         if provided; None otherwise.
     is_resumed:
         Whether the session should be treated as a resumed session.
-    mcp_servers:
-        Dynamic MCP server configurations supplied by the CLI invoker (e.g. via
-        the ``--mcp-servers`` flag).  Merged with the bundle's static ``tool-mcp``
-        config and passed to ``prepared.create_session(tool_overrides=...)`` so
-        ``tool-mcp.mount()`` sees the dynamic ``servers`` dict.  Mirrors the wire
-        path's behavior in ``handle_initialize`` (this function's CLI-side twin).
-        Defaults to ``None`` (treated as ``{}``) for backward compatibility.
+    mcp_config_path:
+        Path to a JSON file containing MCP server configuration in the format
+        documented by amplifier-module-tool-mcp. When provided, the engine sets
+        ``AMPLIFIER_MCP_CONFIG`` so the module loads it via its standard config
+        discovery (config.py priority chain). The wrapper owns the file format;
+        the engine just routes the path.
+        Defaults to ``None`` (no MCP config override).
 
     Returns
     -------
@@ -72,15 +74,12 @@ def make_turn_handler(
 
     resolved_cwd: Path | None = Path(cwd).resolve() if cwd else None
 
-    # Merge dynamic mcp_servers (CLI-supplied) with the bundle's static tool-mcp
-    # config and close over the resulting dict.  Mirrors the wire path in
-    # handle_initialize so that --mcp-servers reaches tool-mcp.mount() the same
-    # way params["mcpServers"] does.  Per amplifier_module_tool_mcp/config.py,
-    # the tool_overrides config dict has highest priority at mount-time.
-    _tool_mcp_static = (
-        prepared.config.get("tools", {}).get("tool-mcp", {}).get("config", {})  # pyright: ignore[reportAttributeAccessIssue]
-    )
-    _tool_mcp_config = {**_tool_mcp_static, "servers": mcp_servers or {}}
+    # Forward CLI-supplied MCP config to the tool-mcp module via its
+    # documented AMPLIFIER_MCP_CONFIG entry in the config priority chain
+    # (see amplifier_module_tool_mcp/config.py). The wrapper owns the file
+    # format; the engine just routes the path.
+    if mcp_config_path:
+        os.environ["AMPLIFIER_MCP_CONFIG"] = mcp_config_path
 
     # Pre-hydrate agent overlays from the vendored agent markdown files.
     # This is done once at handler-creation time (cold path) so each turn
@@ -112,7 +111,6 @@ async def handler(ctx: TurnContext) -> str:
             session_id=session_id,
             session_cwd=resolved_cwd,
             is_resumed=is_resumed,
-            tool_overrides={"tool-mcp": {"config": _tool_mcp_config}},  # pyright: ignore[reportCallIssue]
         )
 
         # Wire display and approval into the coordinator so hook events can
@@ -217,46 +215,45 @@ async def _spawn_fn(**kw: Any) -> dict[str, Any]:
 async def handle_initialize(params: dict[str, Any]) -> Any:
     """Wire-side initialize entry point.
 
-    Loads the prepared bundle from cache, threads wire-supplied
-    ``params["mcpServers"]`` into ``tool-mcp.mount()`` via ``tool_overrides``,
+    Loads the prepared bundle from cache, forwards a wire-supplied
+    ``params["mcpConfigPath"]`` to ``tool-mcp`` via ``AMPLIFIER_MCP_CONFIG``,
     and stores ``params.host.capabilities`` on ``session.metadata`` for
     future capability-flag logic without wire-protocol changes.
 
     Parameters
     ----------
     params:
         An ``InitializeParams``-shaped dict.  Reads ``sessionId``, ``resume``,
-        ``mcpServers``, and ``host.capabilities``.
+        ``mcpConfigPath``, and ``host.capabilities``.
 
     Returns
     -------
     The created session.
 
     Notes
     -----
-    The static ``tool-mcp`` config (e.g. ``verbose_servers``, ``max_content_size``)
-    declared in the bundle is merged with the dynamic ``servers`` dict supplied
-    over the wire.  The combined dict is passed to ``mount()`` with highest
-    priority per ``amplifier_module_tool_mcp/config.py``.
+    The wrapper writes the MCP config file in the format the module expects
+    (see amplifier-module-tool-mcp config.py). The engine just sets
+    ``AMPLIFIER_MCP_CONFIG`` so the module's ``_load_from_env`` picks it up
+    during mount alongside the bundle's static tool-mcp config.
+    See methods.py for the wire-protocol field.
     """
     prepared = await load_and_prepare_cached(aaa_version=__version__)
 
     session_id: str | None = params.get("sessionId") or None
     is_resumed: bool = bool(params.get("resume", False))
 
-    # ── A5: Q9 — thread MCP servers into tool-mcp.mount() ──
-    # PreparedBundle stubs are incomplete; .config is the merged bundle yaml.
-    _tool_mcp_static = (
-        prepared.config.get("tools", {}).get("tool-mcp", {}).get("config", {})  # pyright: ignore[reportAttributeAccessIssue]
-    )
-    tool_mcp_config = {**_tool_mcp_static, "servers": params.get("mcpServers") or {}}
+    # ── A5: Q9 — forward wire-supplied MCP config path to tool-mcp ──
+    # The wrapper writes the file in the format the module expects; the
+    # engine just sets the env var so the module's _load_from_env picks
+    # it up during mount. See methods.py for the wire-protocol field.
+    _wire_mcp_config_path = params.get("mcpConfigPath") or None
+    if _wire_mcp_config_path:
+        os.environ["AMPLIFIER_MCP_CONFIG"] = _wire_mcp_config_path
 
-    # ``tool_overrides`` is accepted by create_session per amplifier_module_tool_mcp/config.py:35-53,56-61
-    # — the config dict passed to mount() has highest priority.
     session = await prepared.create_session(
         session_id=session_id,
         is_resumed=is_resumed,
-        tool_overrides={"tool-mcp": {"config": tool_mcp_config}},  # pyright: ignore[reportCallIssue]
     )
 
     # ── A5: host capabilities storage ──

src/amplifier_agent_lib/protocol/methods.py

@@ -8,8 +8,17 @@
 
 from typing import Any, NotRequired, TypedDict
 
-PROTOCOL_VERSION = "0.1.0"
-"""Wire protocol version. Bump on breaking changes; semver applies."""
+PROTOCOL_VERSION = "0.2.0"
+"""Wire protocol version. Bump on breaking changes; semver applies.
+
+0.2.0 — MCP config delivery changed from inline ``mcpServers`` dict to a
+        path string (``mcpConfigPath``) pointing at a JSON file in the format
+        documented by amplifier-module-tool-mcp (top-level ``mcpServers`` key).
+        The engine sets ``AMPLIFIER_MCP_CONFIG`` from this path; the module
+        reads it via its standard config discovery (config.py priority chain).
+        See _runtime.py for the host-side semantics.
+0.1.0 — Initial Mode A v2 protocol.
+"""
 
 
 class ClientInfo(TypedDict):
@@ -82,7 +91,12 @@ class InitializeParams(TypedDict):
     resume: NotRequired[bool]
     providerOverride: NotRequired[str]
     cwd: NotRequired[str]
-    mcpServers: NotRequired[dict[str, McpServerConfig]]
+    # MCP config: pass a path to a JSON file in the format documented by
+    # amplifier-module-tool-mcp (top-level "mcpServers" key). The engine
+    # sets AMPLIFIER_MCP_CONFIG from this path; the module reads it via its
+    # standard config discovery. The wrapper handles dict-to-file
+    # translation for hosts that prefer the inline-dict API.
+    mcpConfigPath: NotRequired[str]
     host: NotRequired[InitializeHostParams]