mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-20 04:24:17 +02:00
Add supports_tasks() method to replace string mode checks (#2664)
* Add supports_tasks() method to replace string mode checks Consolidates task config mode checks into a readable method on TaskConfig. Instead of `task_config.mode == "forbidden"` or `task_config.mode != "forbidden"`, code now uses `task_config.supports_tasks()` for clearer intent. Updated 20 instances across the codebase and added type assertions in tests to resolve type checker warnings. * Update test to match new error message
This commit is contained in:
parent
19fdac7b02
commit
d6654a2379
15 changed files with 59 additions and 31 deletions
|
|
@ -1,6 +0,0 @@
|
|||
{
|
||||
"setup-worktree": [
|
||||
"uv sync",
|
||||
"uv run pre-commit install"
|
||||
]
|
||||
}
|
||||
2
.github/workflows/marvin.yml
vendored
2
.github/workflows/marvin.yml
vendored
|
|
@ -65,7 +65,7 @@ jobs:
|
|||
trigger_phrase: "/marvin"
|
||||
allowed_bots: "*"
|
||||
claude_args: |
|
||||
--allowedTools WebSearch,WebFetch,Bash(uv:*),Bash(pre-commit:*),Bash(pytest:*),Bash(ruff:*),Bash(ty:*),Bash(git:*),Bash(gh:*),mcp__github__add_issue_comment,mcp__github__create_issue,mcp__github__get_issue,mcp__github__list_issues,mcp__github__search_issues,mcp__github__update_issue,mcp__github__update_issue_comment,mcp__github__create_pull_request,mcp__github__get_pull_request,mcp__github__get_pull_request_comments,mcp__github__get_pull_request_files,mcp__github__get_pull_request_reviews,mcp__github__get_pull_request_status,mcp__github__list_pull_requests,mcp__github__update_pull_request,mcp__github__update_pull_request_branch,mcp__github__update_pull_request_comment,mcp__github__merge_pull_request
|
||||
--allowedTools WebSearch,WebFetch,Bash(uv:*),Bash(pre-commit:*),Bash(prek:*),Bash(pytest:*),Bash(ruff:*),Bash(ty:*),Bash(git:*),Bash(gh:*),mcp__github__add_issue_comment,mcp__github__create_issue,mcp__github__get_issue,mcp__github__list_issues,mcp__github__search_issues,mcp__github__update_issue,mcp__github__update_issue_comment,mcp__github__create_pull_request,mcp__github__get_pull_request,mcp__github__get_pull_request_comments,mcp__github__get_pull_request_files,mcp__github__get_pull_request_reviews,mcp__github__get_pull_request_status,mcp__github__list_pull_requests,mcp__github__update_pull_request,mcp__github__update_pull_request_branch,mcp__github__update_pull_request_comment,mcp__github__merge_pull_request
|
||||
additional_permissions: |
|
||||
actions: read
|
||||
settings: |
|
||||
|
|
|
|||
|
|
@ -294,7 +294,7 @@ class Prompt(FastMCPComponent):
|
|||
|
||||
def register_with_docket(self, docket: Docket) -> None:
|
||||
"""Register this prompt with docket for background execution."""
|
||||
if self.task_config.mode == "forbidden":
|
||||
if not self.task_config.supports_tasks():
|
||||
return
|
||||
docket.register(self.render, names=[self.key])
|
||||
|
||||
|
|
@ -519,7 +519,7 @@ class FunctionPrompt(Prompt):
|
|||
FunctionPrompt registers the underlying function, which has the user's
|
||||
Depends parameters for docket to resolve.
|
||||
"""
|
||||
if self.task_config.mode == "forbidden":
|
||||
if not self.task_config.supports_tasks():
|
||||
return
|
||||
docket.register(self.fn, names=[self.key]) # type: ignore[arg-type]
|
||||
|
||||
|
|
|
|||
|
|
@ -305,7 +305,7 @@ class Resource(FastMCPComponent):
|
|||
|
||||
def register_with_docket(self, docket: Docket) -> None:
|
||||
"""Register this resource with docket for background execution."""
|
||||
if self.task_config.mode == "forbidden":
|
||||
if not self.task_config.supports_tasks():
|
||||
return
|
||||
docket.register(self.read, names=[self.key])
|
||||
|
||||
|
|
@ -421,6 +421,6 @@ class FunctionResource(Resource):
|
|||
FunctionResource registers the underlying function, which has the user's
|
||||
Depends parameters for docket to resolve.
|
||||
"""
|
||||
if self.task_config.mode == "forbidden":
|
||||
if not self.task_config.supports_tasks():
|
||||
return
|
||||
docket.register(self.fn, names=[self.key])
|
||||
|
|
|
|||
|
|
@ -270,7 +270,7 @@ class ResourceTemplate(FastMCPComponent):
|
|||
|
||||
def register_with_docket(self, docket: Docket) -> None:
|
||||
"""Register this template with docket for background execution."""
|
||||
if self.task_config.mode == "forbidden":
|
||||
if not self.task_config.supports_tasks():
|
||||
return
|
||||
docket.register(self.read, names=[self.key])
|
||||
|
||||
|
|
@ -384,7 +384,7 @@ class FunctionResourceTemplate(ResourceTemplate):
|
|||
FunctionResourceTemplate registers the underlying function, which has the
|
||||
user's Depends parameters for docket to resolve.
|
||||
"""
|
||||
if self.task_config.mode == "forbidden":
|
||||
if not self.task_config.supports_tasks():
|
||||
return
|
||||
docket.register(self.fn, names=[self.key])
|
||||
|
||||
|
|
|
|||
|
|
@ -255,23 +255,23 @@ class Provider:
|
|||
tools=[
|
||||
t
|
||||
for t in all_tools
|
||||
if isinstance(t, FunctionTool) and t.task_config.mode != "forbidden"
|
||||
if isinstance(t, FunctionTool) and t.task_config.supports_tasks()
|
||||
],
|
||||
resources=[
|
||||
r
|
||||
for r in all_resources
|
||||
if isinstance(r, FunctionResource) and r.task_config.mode != "forbidden"
|
||||
if isinstance(r, FunctionResource) and r.task_config.supports_tasks()
|
||||
],
|
||||
templates=[
|
||||
t
|
||||
for t in all_templates
|
||||
if isinstance(t, FunctionResourceTemplate)
|
||||
and t.task_config.mode != "forbidden"
|
||||
and t.task_config.supports_tasks()
|
||||
],
|
||||
prompts=[
|
||||
p
|
||||
for p in all_prompts
|
||||
if isinstance(p, FunctionPrompt) and p.task_config.mode != "forbidden"
|
||||
if isinstance(p, FunctionPrompt) and p.task_config.supports_tasks()
|
||||
],
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -570,22 +570,22 @@ class FastMCPProvider(Provider):
|
|||
tools: list[Tool] = [
|
||||
t
|
||||
for t in self.server._tool_manager._tools.values()
|
||||
if t.task_config.mode != "forbidden"
|
||||
if t.task_config.supports_tasks()
|
||||
]
|
||||
resources: list[Resource] = [
|
||||
r
|
||||
for r in self.server._resource_manager._resources.values()
|
||||
if r.task_config.mode != "forbidden"
|
||||
if r.task_config.supports_tasks()
|
||||
]
|
||||
templates: list[ResourceTemplate] = [
|
||||
t
|
||||
for t in self.server._resource_manager._templates.values()
|
||||
if t.task_config.mode != "forbidden"
|
||||
if t.task_config.supports_tasks()
|
||||
]
|
||||
prompts: list[Prompt] = [
|
||||
p
|
||||
for p in self.server._prompt_manager._prompts.values()
|
||||
if p.task_config.mode != "forbidden"
|
||||
if p.task_config.supports_tasks()
|
||||
]
|
||||
|
||||
# Recursively get tasks from nested providers
|
||||
|
|
|
|||
|
|
@ -59,6 +59,14 @@ class TaskConfig:
|
|||
"""
|
||||
return cls(mode="optional" if value else "forbidden")
|
||||
|
||||
def supports_tasks(self) -> bool:
|
||||
"""Check if this component supports task execution.
|
||||
|
||||
Returns:
|
||||
True if mode is "optional" or "required", False if "forbidden".
|
||||
"""
|
||||
return self.mode != "forbidden"
|
||||
|
||||
def validate_function(self, fn: Callable[..., Any], name: str) -> None:
|
||||
"""Validate that function is compatible with this task config.
|
||||
|
||||
|
|
@ -72,7 +80,7 @@ class TaskConfig:
|
|||
Raises:
|
||||
ValueError: If task execution is enabled but function is sync.
|
||||
"""
|
||||
if self.mode == "forbidden":
|
||||
if not self.supports_tasks():
|
||||
return
|
||||
|
||||
# Unwrap callable classes and staticmethods
|
||||
|
|
|
|||
|
|
@ -60,7 +60,7 @@ async def check_background_task(
|
|||
)
|
||||
|
||||
# Enforce mode="forbidden" - cannot be called with task metadata
|
||||
if task_config.mode == "forbidden" and task_meta:
|
||||
if not task_config.supports_tasks() and task_meta:
|
||||
raise McpError(
|
||||
ErrorData(
|
||||
code=METHOD_NOT_FOUND,
|
||||
|
|
|
|||
|
|
@ -307,7 +307,7 @@ class Tool(FastMCPComponent):
|
|||
|
||||
def register_with_docket(self, docket: Docket) -> None:
|
||||
"""Register this tool with docket for background execution."""
|
||||
if self.task_config.mode == "forbidden":
|
||||
if not self.task_config.supports_tasks():
|
||||
return
|
||||
docket.register(self.run, names=[self.key])
|
||||
|
||||
|
|
@ -388,8 +388,8 @@ class FunctionTool(Tool):
|
|||
)
|
||||
|
||||
# Add task execution mode per SEP-1686
|
||||
# Only set execution if not overridden and mode is not "forbidden"
|
||||
if self.task_config.mode != "forbidden" and "execution" not in overrides:
|
||||
# Only set execution if not overridden and task execution is supported
|
||||
if self.task_config.supports_tasks() and "execution" not in overrides:
|
||||
mcp_tool.execution = ToolExecution(taskSupport=self.task_config.mode)
|
||||
|
||||
return mcp_tool
|
||||
|
|
@ -483,7 +483,7 @@ class FunctionTool(Tool):
|
|||
FunctionTool registers the underlying function, which has the user's
|
||||
Depends parameters for docket to resolve.
|
||||
"""
|
||||
if self.task_config.mode == "forbidden":
|
||||
if not self.task_config.supports_tasks():
|
||||
return
|
||||
docket.register(self.fn, names=[self.key])
|
||||
|
||||
|
|
|
|||
|
|
@ -143,10 +143,10 @@ class FastMCPComponent(FastMCPBaseModel):
|
|||
|
||||
The **kwargs are passed through to docket.add() (e.g., key=task_key).
|
||||
"""
|
||||
if self.task_config.mode == "forbidden":
|
||||
if not self.task_config.supports_tasks():
|
||||
raise RuntimeError(
|
||||
f"Cannot add {self.__class__.__name__} '{self.name}' to docket: "
|
||||
f"task_config.mode is 'forbidden'"
|
||||
f"task execution not supported"
|
||||
)
|
||||
raise NotImplementedError(
|
||||
f"{self.__class__.__name__} does not implement add_to_docket()"
|
||||
|
|
|
|||
|
|
@ -6,7 +6,12 @@ import pytest
|
|||
from fastmcp import Context, FastMCP
|
||||
from fastmcp.exceptions import NotFoundError, PromptError
|
||||
from fastmcp.prompts import Prompt
|
||||
from fastmcp.prompts.prompt import FunctionPrompt, PromptMessage, TextContent
|
||||
from fastmcp.prompts.prompt import (
|
||||
FunctionPrompt,
|
||||
PromptMessage,
|
||||
PromptResult,
|
||||
TextContent,
|
||||
)
|
||||
from fastmcp.prompts.prompt_manager import PromptManager
|
||||
from fastmcp.utilities.tests import caplog_for_fastmcp
|
||||
from tests.conftest import get_fn_name
|
||||
|
|
@ -165,6 +170,7 @@ class TestRenderPrompt:
|
|||
prompt = Prompt.from_function(fn)
|
||||
manager.add_prompt(prompt)
|
||||
result = await manager.render_prompt("fn")
|
||||
assert isinstance(result, PromptResult)
|
||||
assert result.description == "An example prompt."
|
||||
assert result.messages == [
|
||||
PromptMessage(
|
||||
|
|
@ -183,6 +189,7 @@ class TestRenderPrompt:
|
|||
prompt = Prompt.from_function(fn)
|
||||
manager.add_prompt(prompt)
|
||||
result = await manager.render_prompt("fn", arguments={"name": "World"})
|
||||
assert isinstance(result, PromptResult)
|
||||
assert result.description == "An example prompt."
|
||||
assert result.messages == [
|
||||
PromptMessage(
|
||||
|
|
@ -204,6 +211,7 @@ class TestRenderPrompt:
|
|||
prompt = Prompt.from_function(MyPrompt())
|
||||
manager.add_prompt(prompt)
|
||||
result = await manager.render_prompt("MyPrompt", arguments={"name": "World"})
|
||||
assert isinstance(result, PromptResult)
|
||||
assert result.description == "A callable object that can be used as a prompt."
|
||||
assert result.messages == [
|
||||
PromptMessage(
|
||||
|
|
@ -225,6 +233,7 @@ class TestRenderPrompt:
|
|||
prompt = Prompt.from_function(MyPrompt())
|
||||
manager.add_prompt(prompt)
|
||||
result = await manager.render_prompt("MyPrompt", arguments={"name": "World"})
|
||||
assert isinstance(result, PromptResult)
|
||||
assert result.description == "A callable object that can be used as a prompt."
|
||||
assert result.messages == [
|
||||
PromptMessage(
|
||||
|
|
@ -402,6 +411,7 @@ class TestContextHandling:
|
|||
async with context:
|
||||
result = await prompt.render(arguments={"x": 42})
|
||||
|
||||
assert isinstance(result, PromptResult)
|
||||
assert len(result.messages) == 1
|
||||
assert result.messages[0].content.text == "42" # type: ignore[attr-defined]
|
||||
|
||||
|
|
@ -424,6 +434,7 @@ class TestContextHandling:
|
|||
arguments={"x": 42},
|
||||
)
|
||||
|
||||
assert isinstance(result, PromptResult)
|
||||
assert len(result.messages) == 1
|
||||
assert result.messages[0].content.text == "42" # type: ignore[attr-defined]
|
||||
|
||||
|
|
@ -461,4 +472,5 @@ class TestContextHandling:
|
|||
|
||||
async with context:
|
||||
result = await prompt.render(arguments={"topic": "cats"})
|
||||
assert isinstance(result, PromptResult)
|
||||
assert result.messages[0].content.text == "Write about cats" # type: ignore[attr-defined]
|
||||
|
|
|
|||
|
|
@ -304,6 +304,7 @@ class TestResourceManager:
|
|||
resource = await manager.get_resource(AnyUrl("greet://world"))
|
||||
assert isinstance(resource, FunctionResource)
|
||||
result = await resource.read()
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "Hello, world!"
|
||||
|
||||
async def test_get_unknown_resource(self):
|
||||
|
|
@ -481,6 +482,7 @@ class TestQueryOnlyTemplates:
|
|||
|
||||
# Should also work via read_resource
|
||||
result = await manager.read_resource("data://config")
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "Config in json format"
|
||||
|
||||
async def test_template_with_only_query_params_with_query_string(self):
|
||||
|
|
@ -506,6 +508,7 @@ class TestQueryOnlyTemplates:
|
|||
|
||||
# Should also work via read_resource
|
||||
result = await manager.read_resource("data://config?format=xml")
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "Config in xml format"
|
||||
|
||||
async def test_template_with_only_multiple_query_params(self):
|
||||
|
|
@ -524,14 +527,17 @@ class TestQueryOnlyTemplates:
|
|||
|
||||
# No query params - use all defaults
|
||||
result = await manager.read_resource("data://items")
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "Data in json (limit: 10)"
|
||||
|
||||
# Partial query params
|
||||
result = await manager.read_resource("data://items?format=xml")
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "Data in xml (limit: 10)"
|
||||
|
||||
# All query params
|
||||
result = await manager.read_resource("data://items?format=xml&limit=20")
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "Data in xml (limit: 20)"
|
||||
|
||||
async def test_has_resource_with_query_only_template(self):
|
||||
|
|
|
|||
|
|
@ -184,6 +184,7 @@ class TestResourceTemplate:
|
|||
|
||||
assert isinstance(resource, FunctionResource)
|
||||
result = await resource.read()
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert isinstance(result.content, str)
|
||||
data = json.loads(result.content)
|
||||
assert data == {"key": "foo", "value": 123}
|
||||
|
|
@ -207,6 +208,7 @@ class TestResourceTemplate:
|
|||
|
||||
assert isinstance(resource, FunctionResource)
|
||||
result = await resource.read()
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "Hello, world!"
|
||||
|
||||
async def test_async_binary_resource(self):
|
||||
|
|
@ -228,6 +230,7 @@ class TestResourceTemplate:
|
|||
|
||||
assert isinstance(resource, FunctionResource)
|
||||
result = await resource.read()
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == b"test"
|
||||
|
||||
async def test_basemodel_conversion(self):
|
||||
|
|
@ -253,6 +256,7 @@ class TestResourceTemplate:
|
|||
|
||||
assert isinstance(resource, FunctionResource)
|
||||
result = await resource.read()
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert isinstance(result.content, str)
|
||||
data = json.loads(result.content)
|
||||
assert data == {"key": "foo", "value": 123}
|
||||
|
|
@ -283,6 +287,7 @@ class TestResourceTemplate:
|
|||
|
||||
assert isinstance(resource, FunctionResource)
|
||||
result = await resource.read()
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == '"hello"'
|
||||
|
||||
async def test_wildcard_param_can_create_resource(self):
|
||||
|
|
@ -393,6 +398,7 @@ class TestResourceTemplate:
|
|||
|
||||
assert isinstance(resource, FunctionResource)
|
||||
result = await resource.read()
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "X was foo"
|
||||
|
||||
|
||||
|
|
@ -679,6 +685,7 @@ class TestContextHandling:
|
|||
|
||||
assert isinstance(resource, FunctionResource)
|
||||
result = await resource.read()
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "42"
|
||||
|
||||
async def test_context_optional(self):
|
||||
|
|
@ -705,6 +712,7 @@ class TestContextHandling:
|
|||
|
||||
assert isinstance(resource, FunctionResource)
|
||||
result = await resource.read()
|
||||
assert isinstance(result, ResourceContent)
|
||||
assert result.content == "42"
|
||||
|
||||
async def test_context_with_functools_wraps_decorator(self):
|
||||
|
|
|
|||
|
|
@ -168,7 +168,7 @@ class TestFastMCPComponentDocketMethods:
|
|||
component = FastMCPComponent(name="test")
|
||||
mock_docket = MagicMock()
|
||||
|
||||
with pytest.raises(RuntimeError, match="task_config.mode is 'forbidden'"):
|
||||
with pytest.raises(RuntimeError, match="task execution not supported"):
|
||||
await component.add_to_docket(mock_docket)
|
||||
|
||||
async def test_add_to_docket_raises_not_implemented_when_allowed(self):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue