[SEP-1686] Raise ValueError when sync functions have task=True

[jlowin]

Background tasks (SEP-1686) require async functions because Docket executes them in a worker process. Previously, sync functions with task=True would log a warning and silently fall back to immediate execution. This was confusing—users wouldn't know their function wasn't actually running as a background task.

Now, attempting to use a sync function with task=True raises a clear error at decoration time:

mcp = FastMCP("test", tasks=True)

@mcp.tool()  # ValueError: Tool 'my_tool' uses a sync function but has task=True.
def my_tool(x: int) -> int:
    return x * 2

@mcp.tool(task=False)  # Works - explicitly opt out
def my_tool(x: int) -> int:
    return x * 2

The validation is now in the from_function() class methods (FunctionTool, FunctionPrompt, FunctionResource, FunctionResourceTemplate) rather than the server decorators, ensuring it runs regardless of how objects are created.

Follows #2378

[coderabbitai]

Walkthrough

The pull request moves background-task validation from the server registration layer into factory methods. It removes server-side checks that disabled task support for synchronous functions and adds runtime validation in FunctionTool.from_function, FunctionPrompt.from_function, FunctionResource.from_function, and FunctionResourceTemplate.from_function. Each factory now raises a ValueError if task=True is used with a non-async function, instructing callers to use task=False for synchronous functions.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: raising a ValueError when sync functions have task=True, which is the core behavior modification across all modified files.
Description check	✅ Passed	The description provides clear context about why the change was made, concrete code examples showing the new behavior, explains the technical implementation (validation moved to from_function methods), and references the related issue.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch error-on-sync-task-functions

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (1)

src/fastmcp/tools/tool.py (1)

317-323: Consider extracting validation logic to reduce duplication.

The same validation pattern (checking task=True with sync functions) appears in four files with minor variations. Consider extracting to a shared utility function.

Example shared utility:

# In fastmcp/utilities/validation.py or similar
def validate_task_requires_async(
    fn: Callable[..., Any],
    task: bool | None,
    component_type: str,
    name: str,
) -> None:
    """Validate that task=True is only used with async functions."""
    if not task:
        return
    
    # Handle callable classes and staticmethods
    fn_to_check = fn
    if not inspect.isroutine(fn) and hasattr(fn, '__call__'):
        fn_to_check = fn.__call__
    if isinstance(fn_to_check, staticmethod):
        fn_to_check = fn_to_check.__func__
    
    if not inspect.iscoroutinefunction(fn_to_check):
        raise ValueError(
            f"{component_type} '{name}' uses a sync function but has task=True. "
            "Background tasks require async functions. Set task=False to disable."
        )

Then call it from each from_function method:

validate_task_requires_async(fn, task, "Tool", fn_name)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 66aaf42 and 7e88003.

⛔ Files ignored due to path filters (3)

• tests/server/tasks/test_sync_function_task_disabled.py is excluded by none and included by none
• tests/server/tasks/test_task_capabilities.py is excluded by none and included by none
• tests/server/tasks/test_task_return_types.py is excluded by none and included by none

📒 Files selected for processing (5)

• src/fastmcp/prompts/prompt.py (1 hunks)
• src/fastmcp/resources/resource.py (1 hunks)
• src/fastmcp/resources/template.py (1 hunks)
• src/fastmcp/server/server.py (0 hunks)
• src/fastmcp/tools/tool.py (1 hunks)

💤 Files with no reviewable changes (1)

• src/fastmcp/server/server.py

🧰 Additional context used

📓 Path-based instructions (1)

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bare except - be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code organization and style

Files:

• src/fastmcp/prompts/prompt.py
• src/fastmcp/resources/template.py
• src/fastmcp/resources/resource.py
• src/fastmcp/tools/tool.py

🧬 Code graph analysis (2)

src/fastmcp/resources/template.py (1)

examples/tasks/client.py (1)

• task (83-128)

src/fastmcp/resources/resource.py (2)

examples/tasks/client.py (1)

• task (83-128)

src/fastmcp/utilities/types.py (1)

• get_fn_name (34-35)

🪛 Ruff (0.14.7)

src/fastmcp/prompts/prompt.py

204-207: Avoid specifying long messages outside the exception class

(TRY003)

src/fastmcp/resources/template.py

313-316: Avoid specifying long messages outside the exception class

(TRY003)

src/fastmcp/resources/resource.py

202-205: Avoid specifying long messages outside the exception class

(TRY003)

src/fastmcp/tools/tool.py

320-323: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: label-issue-or-pr
• GitHub Check: Run tests: Python 3.10 on windows-latest
• GitHub Check: Run tests: Python 3.10 on ubuntu-latest
• GitHub Check: Run tests with lowest-direct dependencies

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (4)

src/fastmcp/prompts/prompt.py (1)

211-216: Async-only enforcement for task prompts looks correct

The validation now runs after unwrapping callable classes/staticmethods and cleanly raises on task=True with a non-async function, which matches the PR’s intent and avoids the earlier false positives. The long error message flagged by Ruff (TRY003) is acceptable for clarity; only shorten or centralize it if you want strict lint compliance.

src/fastmcp/tools/tool.py (1)

317-329: Task validation for tools correctly unwraps callables before async check

This block now inspects fn_to_check after unwrapping callable classes and staticmethods, then raises a clear ValueError when task=True is used with a non-async function, which is exactly what SEP-1686 requires. The behavior is consistent with resources/prompts and should prevent the previous silent fallback.

src/fastmcp/resources/template.py (1)

372-377: Consistent async-only task enforcement for resource templates

The new guard runs after unwrapping callable instances/staticmethods and rejects task=True for non-async functions with a clear error message, aligning resource templates with tools and prompts in how background-task capability is validated.

src/fastmcp/resources/resource.py (1)

200-211: Resource task validation correctly targets the underlying async callable

By unwrapping callable classes and staticmethods into fn_to_check before calling inspect.iscoroutinefunction, this check reliably raises for sync functions with task=True while allowing genuine async resources. The error message is clear; you only need to tweak it if you want to satisfy Ruff’s TRY003 lint rule.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e88003 and fdb4dc5.

⛔ Files ignored due to path filters (1)

• tests/server/tasks/test_sync_function_task_disabled.py is excluded by none and included by none

📒 Files selected for processing (4)

• src/fastmcp/prompts/prompt.py (1 hunks)
• src/fastmcp/resources/resource.py (1 hunks)
• src/fastmcp/resources/template.py (1 hunks)
• src/fastmcp/tools/tool.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bare except - be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code organization and style

Files:

• src/fastmcp/resources/resource.py
• src/fastmcp/resources/template.py
• src/fastmcp/prompts/prompt.py
• src/fastmcp/tools/tool.py

🧬 Code graph analysis (3)

src/fastmcp/resources/resource.py (1)

src/fastmcp/utilities/types.py (1)

• get_fn_name (34-35)

src/fastmcp/resources/template.py (1)

examples/tasks/client.py (1)

• task (83-128)

src/fastmcp/prompts/prompt.py (1)

examples/tasks/client.py (1)

• task (83-128)

🪛 Ruff (0.14.7)

src/fastmcp/resources/resource.py

208-211: Avoid specifying long messages outside the exception class

(TRY003)

src/fastmcp/resources/template.py

374-377: Avoid specifying long messages outside the exception class

(TRY003)

src/fastmcp/prompts/prompt.py

213-216: Avoid specifying long messages outside the exception class

(TRY003)

src/fastmcp/tools/tool.py

326-329: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Run tests: Python 3.10 on windows-latest
• GitHub Check: Run tests with lowest-direct dependencies
• GitHub Check: Run tests: Python 3.10 on ubuntu-latest

[chrisguidry]

Love it!

[marvin-context-protocol]

Test Failure Analysis

Summary: Two integration tests timed out after 30s due to rate limiting (HTTP 429) from GitHub Copilot's MCP API.

Root Cause: The failing tests (test_call_tool_ko and test_call_tool_list_commits) make actual HTTP requests to https://api.githubcopilot.com/mcp/. The tests timed out waiting for responses, but the teardown logs reveal the actual issue: HTTPStatusError: Client error '429 Too Many Requests'. GitHub's API is rate limiting the test requests.

Suggested Solution: These integration tests are flaky because they depend on external API rate limits. Consider one of these approaches:

1. Add retry logic with exponential backoff for 429 errors in the test fixture
2. Skip these specific tests in CI when rate limits are hit (using pytest.mark.xfail with a 429-specific condition)
3. Mock the GitHub API responses for these tests to avoid external dependencies
4. Add rate limit handling in the StreamableHttpTransport client to automatically retry on 429

Detailed Analysis

The tests connect to GitHub's real MCP API endpoint:

GITHUB_REMOTE_MCP_URL = "https://api.githubcopilot.com/mcp/"

Both failing tests show the same pattern:

• Test times out after 30 seconds (from pytest timeout marker)
• Teardown log shows: HTTPStatusError: Client error '429 Too Many Requests' for url 'https://api.githubcopilot.com/mcp/'

The timeout happens because the client hangs waiting for a response that never comes due to rate limiting. The actual error only surfaces during teardown when the client tries to disconnect.

Related Files

• tests/integration_tests/test_github_mcp_remote.py - The integration tests hitting GitHub's API
• src/fastmcp/client/transports.py - StreamableHttpTransport where retry logic could be added
• .github/workflows/run-tests.yml - CI workflow configuration

---

Note: This failure is unrelated to the changes in this PR (SEP-1686). The PR successfully adds validation for sync functions with task=True, but the integration tests are failing due to external API rate limits.