Skip to content

Commit 674783f

Browse files
committed
fix: decide template vs static purely on URI variables
The @resource() decorator now classifies resources based solely on whether the URI contains template variables, not on whether the handler has parameters. Previously, a handler taking only a Context parameter on a non-template URI would register as a zero-variable template. The template matched with an empty dict, which the walrus check in resource_manager treated as falsy, making the resource permanently unreachable. This has never worked. Now such a handler errors at decoration time with a clear message noting that Context injection for static resources is planned but not yet supported. Handlers with non-Context parameters on non-template URIs also get a clearer error than the old 'Mismatch' message. Also changes the resource_manager walrus check to compare against None explicitly, as defense-in-depth against any future case where matches() legitimately returns an empty dict.
1 parent c4f7db0 commit 674783f

File tree

3 files changed

+44
-12
lines changed

3 files changed

+44
-12
lines changed

src/mcp/server/mcpserver/resources/resource_manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ async def get_resource(self, uri: AnyUrl | str, context: Context[LifespanContext
9393

9494
# Then check templates
9595
for template in self._templates.values():
96-
if params := template.matches(uri_str):
96+
if (params := template.matches(uri_str)) is not None:
9797
try:
9898
return await template.create_resource(uri_str, params, context=context)
9999
except Exception as e: # pragma: no cover

src/mcp/server/mcpserver/server.py

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -702,18 +702,13 @@ async def get_weather(city: str) -> str:
702702
uri_params = set(parsed.variable_names)
703703

704704
def decorator(fn: _CallableT) -> _CallableT:
705-
# Check if this should be a template
706705
sig = inspect.signature(fn)
707-
has_func_params = bool(sig.parameters)
708-
709-
if uri_params or has_func_params:
710-
# Check for Context parameter to exclude from validation
711-
context_param = find_context_parameter(fn)
712-
713-
# We need to remove the context_param from the resource function if
714-
# there is any.
715-
func_params = {p for p in sig.parameters.keys() if p != context_param}
706+
context_param = find_context_parameter(fn)
707+
func_params = {p for p in sig.parameters.keys() if p != context_param}
716708

709+
# Template/static is decided purely by the URI: variables
710+
# present means template, none means static.
711+
if uri_params:
717712
if uri_params != func_params:
718713
raise ValueError(
719714
f"Mismatch between URI parameters {uri_params} and function parameters {func_params}"
@@ -733,6 +728,20 @@ def decorator(fn: _CallableT) -> _CallableT:
733728
meta=meta,
734729
)
735730
else:
731+
if func_params:
732+
raise ValueError(
733+
f"Resource {uri!r} has no URI template variables, but the "
734+
f"handler declares parameters {func_params}. Add matching "
735+
f"{{...}} variables to the URI or remove the parameters."
736+
)
737+
if context_param is not None:
738+
raise ValueError(
739+
f"Resource {uri!r} has no URI template variables, but the "
740+
f"handler declares a Context parameter. Context injection "
741+
f"for static resources is not yet supported but is planned. "
742+
f"For now, add a template variable to the URI or remove the "
743+
f"Context parameter."
744+
)
736745
# Register as regular resource
737746
resource = FunctionResource.from_function(
738747
fn=fn,

tests/server/mcpserver/test_server.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,7 @@ async def test_resource_with_params(self):
851851
parameters don't match"""
852852
mcp = MCPServer()
853853

854-
with pytest.raises(ValueError, match="Mismatch between URI parameters"):
854+
with pytest.raises(ValueError, match="has no URI template variables"):
855855

856856
@mcp.resource("resource://data")
857857
def get_data_fn(param: str) -> str: # pragma: no cover
@@ -1192,6 +1192,29 @@ def resource_with_context(name: str, ctx: Context) -> str:
11921192
# Should have either request_id or indication that context was injected
11931193
assert "Resource test - context injected" == content.text
11941194

1195+
async def test_static_resource_with_context_param_errors(self):
1196+
"""A non-template URI with a Context-only handler should error
1197+
at decoration time with a clear message, not silently register
1198+
an unreachable resource."""
1199+
mcp = MCPServer()
1200+
1201+
with pytest.raises(ValueError, match="Context injection for static resources is not yet supported"):
1202+
1203+
@mcp.resource("weather://current")
1204+
def current_weather(ctx: Context) -> str:
1205+
raise NotImplementedError
1206+
1207+
async def test_static_resource_with_extra_params_errors(self):
1208+
"""A non-template URI with non-Context params should error at
1209+
decoration time."""
1210+
mcp = MCPServer()
1211+
1212+
with pytest.raises(ValueError, match="has no URI template variables"):
1213+
1214+
@mcp.resource("data://fixed")
1215+
def get_data(name: str) -> str:
1216+
raise NotImplementedError
1217+
11951218
async def test_resource_without_context(self):
11961219
"""Test that resources without context work normally."""
11971220
mcp = MCPServer()

0 commit comments

Comments
 (0)