fix: address review feedback for MCP config feature

- Move test file to tests/strands/experimental/ to mirror src/ structure
- Export load_mcp_clients_from_config from strands.experimental.__init__
- Convert JSON schema files to inline Python dicts (eliminates module-level file I/O)
- Remove  hack by directly referencing MCP_SERVER_CONFIG_SCHEMA in agent config
- Delete unused .schema.json files
- Update _parse_tool_filters docstring to clarify re.match prefix-match semantics

Author: Unshure

.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

@@ -13,7 +13,6 @@
 
 import json
 import logging
-from pathlib import Path
 from typing import Any
 
 import jsonschema
@@ -23,12 +22,40 @@
 
 logger = logging.getLogger(__name__)
 
-_SCHEMA_PATH = Path(__file__).parent / "agent_config.schema.json"
-with open(_SCHEMA_PATH) as _f:
-    AGENT_CONFIG_SCHEMA: dict[str, Any] = json.load(_f)
-
-# Resolve the $ref in mcp_servers.additionalProperties to the actual MCP server schema
-AGENT_CONFIG_SCHEMA["properties"]["mcp_servers"]["additionalProperties"] = MCP_SERVER_CONFIG_SCHEMA
+# JSON Schema for agent configuration
+AGENT_CONFIG_SCHEMA: dict[str, Any] = {
+    "$schema": "http://json-schema.org/draft-07/schema#",
+    "title": "Agent Configuration",
+    "description": "Configuration schema for creating agents.",
+    "type": "object",
+    "properties": {
+        "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"],
+            "default": None,
+        },
+        "prompt": {
+            "description": "The system prompt for the agent. Provides high level context to the agent.",
+            "type": ["string", "null"],
+            "default": None,
+        },
+        "tools": {
+            "description": "List of tools the agent can use. Can be file paths, "
+            "Python module names, or @tool annotated functions in files.",
+            "type": "array",
+            "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,
+}
 
 # Pre-compile validator for better performance
 _VALIDATOR = jsonschema.Draft7Validator(AGENT_CONFIG_SCHEMA)

src/strands/experimental/agent_config.schema.json

@@ -12,7 +12,6 @@
 import json
 import logging
 import re
-from pathlib import Path
 from typing import Any
 
 import jsonschema
@@ -26,18 +25,72 @@
 
 logger = logging.getLogger(__name__)
 
-_SCHEMA_PATH = Path(__file__).parent / "mcp_server_config.schema.json"
-with open(_SCHEMA_PATH) as _f:
-    MCP_SERVER_CONFIG_SCHEMA: dict[str, Any] = json.load(_f)
+MCP_SERVER_CONFIG_SCHEMA: dict[str, Any] = {
+    "$schema": "http://json-schema.org/draft-07/schema#",
+    "title": "MCP Server Configuration",
+    "description": "Configuration for a single MCP server.",
+    "type": "object",
+    "properties": {
+        "transport": {
+            "description": "Transport type. Auto-detected from 'command' (stdio) or 'url' (sse) if omitted.",
+            "type": "string",
+            "enum": ["stdio", "sse", "streamable-http"],
+        },
+        "command": {"description": "Command to run for stdio transport.", "type": "string"},
+        "args": {
+            "description": "Arguments for the stdio command.",
+            "type": "array",
+            "items": {"type": "string"},
+            "default": [],
+        },
+        "env": {
+            "description": "Environment variables for the stdio command.",
+            "type": "object",
+            "additionalProperties": {"type": "string"},
+        },
+        "cwd": {"description": "Working directory for the stdio command.", "type": "string"},
+        "url": {"description": "URL for sse or streamable-http transport.", "type": "string"},
+        "headers": {
+            "description": "HTTP headers for sse or streamable-http transport.",
+            "type": "object",
+            "additionalProperties": {"type": "string"},
+        },
+        "prefix": {"description": "Prefix to apply to tool names from this server.", "type": "string"},
+        "startup_timeout": {
+            "description": "Timeout in seconds for server initialization. Defaults to 30.",
+            "type": "integer",
+            "default": 30,
+        },
+        "tool_filters": {
+            "description": "Filters for controlling which tools are loaded.",
+            "type": "object",
+            "properties": {
+                "allowed": {
+                    "description": "List of regex patterns for tools to include.",
+                    "type": "array",
+                    "items": {"type": "string"},
+                },
+                "rejected": {
+                    "description": "List of regex patterns for tools to exclude.",
+                    "type": "array",
+                    "items": {"type": "string"},
+                },
+            },
+            "additionalProperties": False,
+        },
+    },
+    "additionalProperties": False,
+}
 
 _SERVER_VALIDATOR = jsonschema.Draft7Validator(MCP_SERVER_CONFIG_SCHEMA)
 
 
 def _parse_tool_filters(config: dict[str, Any] | None) -> ToolFilters | None:
     """Parse a tool filter configuration into a ToolFilters instance.
 
-    All filter strings are compiled as regex patterns. Exact-match strings like ``"^echo$"``
-    work correctly as regex since they match themselves.
+    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" (e.g. "echo_extra").
 
     Args:
         config: Tool filter configuration dict with 'allowed' and/or 'rejected' lists,