Skip to content

Commit 0c3bde3

Browse files
committed
Emit warning when servers attribute accessed
1 parent 71e20d1 commit 0c3bde3

File tree

3 files changed

+70
-23
lines changed

3 files changed

+70
-23
lines changed

src/mcp/client/config/mcp_servers_config.py

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,50 @@ class SSEServerConfig(MCPServerConfig):
133133

134134

135135
class MCPServersConfig(BaseModel):
136-
"""Configuration for multiple MCP servers."""
136+
"""Configuration for multiple MCP servers.
137+
138+
Note:
139+
Direct access to the 'servers' field is discouraged.
140+
Use the server() method instead for proper input validation and substitution.
141+
"""
137142

138143
servers: dict[str, ServerConfigUnion]
139144
inputs: list[InputDefinition] | None = None
140145

146+
def __getattribute__(self, name: str) -> Any:
147+
"""Get an attribute from the config.
148+
149+
This emits a warning if the `servers` field is accessed directly.
150+
This is to discourage direct access to the `servers` field.
151+
Use the `server()` method instead for proper input validation and substitution.
152+
"""
153+
154+
if name == "servers":
155+
import inspect
156+
import warnings
157+
158+
# Get the calling frame to check if it's internal access
159+
frame = inspect.currentframe()
160+
if frame and frame.f_back:
161+
caller_filename = frame.f_back.f_code.co_filename
162+
caller_function = frame.f_back.f_code.co_name
163+
164+
# Don't warn for internal methods, tests, or if called from within this class
165+
is_internal_call = (
166+
caller_function in ("server", "list_servers", "has_server", "__init__", "model_validate")
167+
or "mcp_servers_config.py" in caller_filename
168+
)
169+
170+
if not is_internal_call:
171+
warnings.warn(
172+
f"Direct access to 'servers' field of {self.__class__.__name__} is discouraged. "
173+
+ "Use server() method instead for proper input validation and substitution.",
174+
UserWarning,
175+
stacklevel=2,
176+
)
177+
178+
return super().__getattribute__(name)
179+
141180
def server(
142181
self,
143182
name: str,
@@ -163,6 +202,14 @@ def server(
163202

164203
return server
165204

205+
def list_servers(self) -> list[str]:
206+
"""Get a list of available server names."""
207+
return list(self.servers.keys())
208+
209+
def has_server(self, name: str) -> bool:
210+
"""Check if a server with the given name exists."""
211+
return name in self.servers
212+
166213
@model_validator(mode="before")
167214
@classmethod
168215
def handle_field_aliases(cls, data: dict[str, Any]) -> dict[str, Any]:

tests/client/config/test_mcp_servers_config.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,9 @@ def test_servers_field_takes_precedence():
183183
config = MCPServersConfig.model_validate(config_data)
184184

185185
# Should only have the 'servers' content, not 'mcpServers'
186-
assert "new_server" in config.servers
187-
assert "old_server" not in config.servers
188-
assert len(config.servers) == 1
186+
assert config.has_server("new_server")
187+
assert not config.has_server("old_server")
188+
assert len(config.list_servers()) == 1
189189

190190

191191
def test_from_file_with_inputs(tmp_path: Path):
@@ -582,8 +582,8 @@ def test_jsonc_comment_stripping():
582582
stripped = MCPServersConfig._strip_json_comments(content_with_comments)
583583
config = MCPServersConfig.model_validate(json.loads(stripped))
584584

585-
assert "test_server" in config.servers
586-
server = config.servers["test_server"]
585+
assert config.has_server("test_server")
586+
server = config.server("test_server")
587587
assert isinstance(server, StdioServerConfig)
588588
assert server.command == "python test.py"
589589

@@ -704,10 +704,10 @@ def test_jsonc_multiline_strings_with_comments():
704704
config = MCPServersConfig.model_validate(json.loads(stripped))
705705

706706
assert len(config.servers) == 2
707-
assert "test1" in config.servers
708-
assert "test2" in config.servers
707+
assert config.has_server("test1")
708+
assert config.has_server("test2")
709709

710-
test1 = config.servers["test1"]
710+
test1 = config.server("test1")
711711
assert isinstance(test1, StdioServerConfig)
712712
assert test1.command == "python server.py"
713713

tests/client/config/test_yaml_functionality.py

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ def test_yaml_extension_auto_detection(mcp_yaml_config_file: Path):
2020
config = MCPServersConfig.from_file(mcp_yaml_config_file)
2121

2222
# Should successfully load the YAML file with all 9 servers
23-
assert "stdio_server" in config.servers
24-
assert "streamable_http_server_with_headers" in config.servers
25-
assert "sse_server_with_explicit_type" in config.servers
23+
assert config.has_server("stdio_server")
24+
assert config.has_server("streamable_http_server_with_headers")
25+
assert config.has_server("sse_server_with_explicit_type")
2626

2727
# Verify a specific server
28-
stdio_server = config.servers["stdio_server"]
28+
stdio_server = config.server("stdio_server")
2929
assert isinstance(stdio_server, StdioServerConfig)
3030
assert stdio_server.command == "python"
3131
assert stdio_server.args == ["-m", "my_server"]
@@ -40,24 +40,24 @@ def test_use_pyyaml_parameter_with_json_file():
4040
config = MCPServersConfig.from_file(json_file, use_pyyaml=True)
4141

4242
# Should work fine - PyYAML can parse JSON
43-
assert len(config.servers) == 7
44-
assert "stdio_server" in config.servers
43+
assert len(config.list_servers()) == 7
44+
assert config.has_server("stdio_server")
4545

4646
# Verify it produces the same result as normal JSON parsing
4747
config_json = MCPServersConfig.from_file(json_file, use_pyyaml=False)
48-
assert len(config.servers) == len(config_json.servers)
49-
assert list(config.servers.keys()) == list(config_json.servers.keys())
48+
assert len(config.list_servers()) == len(config_json.list_servers())
49+
assert list(config.list_servers()) == list(config_json.list_servers())
5050

5151

5252
def test_uvx_time_server(mcp_yaml_config_file: Path):
5353
"""Test the time server configuration with uvx command."""
5454
config = MCPServersConfig.from_file(mcp_yaml_config_file)
5555

5656
# Should have the time server
57-
assert "time" in config.servers
57+
assert config.has_server("time")
5858

5959
# Verify the server configuration
60-
time_server = config.servers["time"]
60+
time_server = config.server("time")
6161
assert isinstance(time_server, StdioServerConfig)
6262
assert time_server.type == "stdio" # Should be auto-inferred from command field
6363
assert time_server.command == "uvx mcp-server-time"
@@ -74,10 +74,10 @@ def test_streamable_http_server(mcp_yaml_config_file: Path):
7474
config = MCPServersConfig.from_file(mcp_yaml_config_file)
7575

7676
# Should have the new streamable_http_server
77-
assert "streamable_http_server" in config.servers
77+
assert config.has_server("streamable_http_server")
7878

7979
# Verify the server configuration
80-
http_server = config.servers["streamable_http_server"]
80+
http_server = config.server("streamable_http_server")
8181
assert isinstance(http_server, StreamableHTTPServerConfig)
8282
assert http_server.type == "streamable_http" # Should be auto-inferred
8383
assert http_server.url == "https://api.example.com/mcp"
@@ -89,10 +89,10 @@ def test_npx_filesystem_server(mcp_yaml_config_file: Path):
8989
config = MCPServersConfig.from_file(mcp_yaml_config_file)
9090

9191
# Should have the filesystem server
92-
assert "filesystem" in config.servers
92+
assert config.has_server("filesystem")
9393

9494
# Verify the server configuration
95-
filesystem_server = config.servers["filesystem"]
95+
filesystem_server = config.server("filesystem")
9696
assert isinstance(filesystem_server, StdioServerConfig)
9797
assert filesystem_server.type == "stdio" # Should be auto-inferred from command field
9898
assert (

0 commit comments

Comments
 (0)