feat: support loading MCP server configurations from JSON config files

.agents/reviews/review-2026-04-13.md

@@ -0,0 +1,109 @@
+# Code Review - 2026-04-13
+
+**Commits Reviewed**: dad40f58d835b3e34e49d43ee0bb75285a9f5d10
+**Branch:** agent-tasks/482 → main
+**Type:** feat
+
+## Summary
+
+This PR adds declarative MCP server configuration support, allowing users to define multiple MCP servers in JSON config files (compatible with Claude Desktop/Cursor format) and load them automatically via `load_mcp_clients_from_config()`. It also extends `config_to_agent()` to accept an `mcp_servers` field. Supports stdio, SSE, and streamable-HTTP transports with JSON schema validation, tool filters (regex), and auto-detection of transport type.
+
+## Recommendation
+
+**Decision:** Request Changes
+
+**Rationale:** The feature design is sound and addresses a real user need. The code is well-structured with good separation of concerns and thorough test coverage. However, there are several outstanding issues from the prior automated review that remain unaddressed, plus a few additional concerns around test placement, export paths, and module-level I/O patterns.
+
+## Quick Stats
+
+- Files Changed: 7
+- Lines Added: 957
+- Lines Removed: 28
+- Test Coverage: yes (unit + integration, 94.69% patch coverage per Codecov)
+- Breaking Changes: no
+
+## Critical Issues
+
+No critical issues (security, data loss, breaking changes).
+
+## Important Issues
+
+### Issue 1: Test file placed in wrong directory
+- **File:** `tests/strands/tools/mcp/test_mcp_config.py`
+- **Problem:** The unit tests for `strands.experimental.mcp_config` are located at `tests/strands/tools/mcp/test_mcp_config.py`. Per AGENTS.md, tests should mirror the `src/` directory structure. The module lives in `src/strands/experimental/mcp_config.py`, so the tests should be in `tests/strands/experimental/`.
+- **Suggestion:** Move to `tests/strands/experimental/test_mcp_config.py`.
+
+### Issue 2: `load_mcp_clients_from_config` not exported from `strands.experimental`
+- **File:** `src/strands/experimental/__init__.py`
+- **Problem:** The new public function `load_mcp_clients_from_config` is not added to `strands.experimental.__init__.py`'s `__all__` or imports. Users must import from the internal module path `strands.experimental.mcp_config`, which is not a stable public API surface. The PR description also still shows the old import path `from strands.tools.mcp import load_mcp_clients_from_config`.
+- **Suggestion:** Add to `__init__.py`:
+```python
+from .mcp_config import load_mcp_clients_from_config
+
+__all__ = ["config_to_agent", "load_mcp_clients_from_config", "tools", "steering"]
+```
+
+### Issue 3: Module-level file I/O at import time
+- **File:** `src/strands/experimental/mcp_config.py:30-32`, `src/strands/experimental/agent_config.py:25-27`
+- **Problem:** Both modules read JSON schema files at import time using `Path(__file__).parent`. This is a new pattern in this codebase (the original `agent_config.py` had the schema inline as a Python dict). It introduces file I/O side effects on import and may not work correctly in zipped/packaged distributions.
+- **Suggestion:** Either:
+  1. Keep schemas as inline Python dicts (consistent with original pattern, zero I/O)
+  2. Use `importlib.resources` for reading package data files (recommended for packaged distributions)
+  3. Lazy-load via `functools.lru_cache` on a loader function
+
+### Issue 4: `$ref` in JSON schema manually resolved in Python
+- **File:** `src/strands/experimental/agent_config.py:31`, `src/strands/experimental/agent_config.schema.json:31`
+- **Problem:** The `agent_config.schema.json` contains `"$ref": "mcp_server_config.schema.json"` which is then manually replaced in Python code: `AGENT_CONFIG_SCHEMA["properties"]["mcp_servers"]["additionalProperties"] = MCP_SERVER_CONFIG_SCHEMA`. This makes the JSON file misleading when read standalone — any IDE or JSON Schema tooling would fail to resolve the `$ref`.
+- **Suggestion:** Either inline the MCP server schema directly in `agent_config.schema.json`, or use `jsonschema.RefResolver` to properly resolve `$ref`s. If keeping separate files, add a comment explaining the programmatic resolution.
+
+### Issue 5: Docstring unclear about `re.match` prefix-match semantics
+- **File:** `src/strands/experimental/mcp_config.py:40-42`
+- **Problem:** The docstring says `Exact-match strings like "^echo$" work correctly as regex since they match themselves.` But the actual matching in `mcp_client.py:989` uses `pattern.match()` (prefix match, not full match). So `"echo"` in a filter will match `"echo_extra"` too. Users writing `"echo"` in their JSON config likely expect exact matching.
+- **Suggestion:** Update the docstring to clearly document this behavior:
+```
+All filter strings are compiled as regex patterns and matched using ``re.match``
+(prefix match from start of string). Use ``"^echo$"`` for exact matching.
+``"echo"`` will match any tool name starting with "echo".
+```
+
+## Suggestions
+
+- The PR description still references the old import path `from strands.tools.mcp import load_mcp_clients_from_config`. Should be updated to reflect the actual location.
+- Consider whether `MCP_SERVER_CONFIG_SCHEMA` needs to be a public module-level export from `mcp_config.py`. If it's only used internally by `agent_config.py`, prefix it with `_`.
+- The integration tests at `tests_integ/mcp/test_mcp_config.py` are well-placed and thorough.
+
+## Positive Highlights
+
+- Clean separation between `mcp_config.py` (MCP-specific) and `agent_config.py` (agent-level integration)
+- All prior review feedback (regex heuristic, private naming, lambda→def, mcpServers wrapper, empty dict simplification) was addressed thoughtfully in the force-push
+- Comprehensive test coverage: unit tests for schema validation, tool filter parsing, transport creation, file loading; integration tests with actual echo server
+- Good error messages that include server names and JSON schema paths for debugging
+- Transport auto-detection from config fields is intuitive and matches user expectations
+- JSON schema validation catches config errors early with clear messages
+
+## Files Reviewed
+
+| File | Status | Notes |
+|------|--------|-------|
+| `src/strands/experimental/agent_config.py` | Modified | Added mcp_servers handling, moved schema to JSON file |
+| `src/strands/experimental/agent_config.schema.json` | Added | Agent config JSON schema (extracted from Python dict) |
+| `src/strands/experimental/mcp_config.py` | Added | Core MCP config parsing and MCPClient factory |
+| `src/strands/experimental/mcp_server_config.schema.json` | Added | MCP server config JSON schema |
+| `tests/strands/experimental/test_agent_config.py` | Modified | Added mcp_servers integration tests |
+| `tests/strands/tools/mcp/test_mcp_config.py` | Added | Unit tests (wrong directory - should be experimental/) |
+| `tests_integ/mcp/test_mcp_config.py` | Added | Integration tests with echo server |
+
+## Checklist
+
+- [x] Type annotations complete
+- [ ] Documentation present (not exported from `__init__.py`, PR description outdated)
+- [x] Tests included (comprehensive unit + integration)
+- [x] No breaking changes
+- [ ] Follows project patterns (test file placement, module-level I/O)
+- [x] Error handling appropriate
+
+## Additional Notes
+
+- This is the second iteration of the PR. The first round of automated review identified 7 issues, all of which were addressed in the force-push to `dad40f5`. The second round identified 6 more issues, which are the basis for this review.
+- The `mcpServers` (camelCase) wrapper key in `load_mcp_clients_from_config` vs `mcp_servers` (snake_case) in agent config is a good design choice — it maintains Python conventions in the agent config while supporting the Claude Desktop/Cursor JSON convention in the standalone loader.
+- Codecov reports 94.69% patch coverage with 6 lines missing coverage, primarily in `mcp_config.py` (3 missing + 1 partial) and `agent_config.py` (1 missing + 1 partial).

src/strands/experimental/__init__.py

@@ -5,5 +5,6 @@
 
 from . import steering, tools
 from .agent_config import config_to_agent
+from .mcp_config import load_mcp_clients_from_config
 
-__all__ = ["config_to_agent", "tools", "steering"]
+__all__ = ["config_to_agent", "load_mcp_clients_from_config", "tools", "steering"]

src/strands/experimental/agent_config.py

@@ -12,20 +12,24 @@
 """
 
 import json
-from pathlib import Path
+import logging
 from typing import Any
 
 import jsonschema
 from jsonschema import ValidationError
 
+from .mcp_config import MCP_SERVER_CONFIG_SCHEMA, load_mcp_clients_from_config
+
+logger = logging.getLogger(__name__)
+
 # JSON Schema for agent configuration
-AGENT_CONFIG_SCHEMA = {
+AGENT_CONFIG_SCHEMA: dict[str, Any] = {
     "$schema": "http://json-schema.org/draft-07/schema#",
     "title": "Agent Configuration",
-    "description": "Configuration schema for creating agents",
+    "description": "Configuration schema for creating agents.",
     "type": "object",
     "properties": {
-        "name": {"description": "Name of the agent", "type": ["string", "null"], "default": None},
+        "name": {"description": "Name of the agent.", "type": ["string", "null"], "default": None},
         "model": {
             "description": "The model ID to use for this agent. If not specified, uses the default model.",
             "type": ["string", "null"],
@@ -43,6 +47,12 @@
             "items": {"type": "string"},
             "default": [],
         },
+        "mcp_servers": {
+            "description": "MCP server configurations. Each key is a server name and the value is "
+            "a server configuration object with transport-specific settings.",
+            "type": "object",
+            "additionalProperties": MCP_SERVER_CONFIG_SCHEMA,
+        },
     },
     "additionalProperties": False,
 }
@@ -129,6 +139,15 @@
         if config_key in config_dict and config_dict[config_key] is not None:
             agent_kwargs[agent_param] = config_dict[config_key]
 
+    # Handle mcp_servers: create MCPClient instances and append to tools
+    if config_dict.get("mcp_servers"):
+        mcp_clients = load_mcp_clients_from_config({"mcpServers": config_dict["mcp_servers"]})
+        tools_list = agent_kwargs.get("tools", [])
+        if not isinstance(tools_list, list):
+            tools_list = list(tools_list)
+        tools_list.extend(mcp_clients.values())
+        agent_kwargs["tools"] = tools_list
+
     # Override with any additional kwargs provided
     agent_kwargs.update(kwargs)