Skip to content

Remove legacy _task_capable_initialize() workaround#2612

Merged
jlowin merged 1 commit intomainfrom
remove-task-capable-initialize
Dec 14, 2025
Merged

Remove legacy _task_capable_initialize() workaround#2612
jlowin merged 1 commit intomainfrom
remove-task-capable-initialize

Conversation

@jlowin
Copy link
Copy Markdown
Member

@jlowin jlowin commented Dec 14, 2025

FastMCP had a custom _task_capable_initialize() function that set experimental={"tasks": {}} during client initialization. This predates the finalized MCP spec, which clarifies:

  1. Tasks are declared in capabilities.tasks (now a first-class field)
  2. Clients only need to declare task capabilities if they receive task-augmented requests from the server

For client→server task requests (e.g., call_tool(..., task=True)), only the server declares task capabilities. The SDK's native session.initialize() handles this correctly, so we can remove the workaround.

# Before
self._session_state.initialize_result = await _task_capable_initialize(self.session)

# After
self._session_state.initialize_result = await self.session.initialize()

The function set experimental={"tasks": {}} but per the MCP spec:
1. Tasks belong in capabilities.tasks, not capabilities.experimental
2. Clients only need to declare task capabilities if receiving task-augmented
   requests from the server (bi-directional support)

For client→server task requests (tools/call with task=True), only the server
needs to declare task capabilities. The SDK's native session.initialize()
handles this correctly.
@marvin-context-protocol marvin-context-protocol Bot added enhancement Improvement to existing functionality. For issues and smaller PR improvements. client Related to the FastMCP client SDK or client-side functionality. labels Dec 14, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 14, 2025

Walkthrough

The PR refactors the initialization flow in the MCP client. The _task_capable_initialize helper function is removed from src/fastmcp/client/tasks.py, along with related imports and task capability initialization logic. The initialize() method in src/fastmcp/client/client.py is updated to call self.session.initialize() directly instead of using the removed helper. The initialization behavior remains idempotent with cached results, timeout, and error handling unchanged. TaskNotificationHandler and task classes are unaffected.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description explains the rationale and context well, but does not include the required Contributors and Review checklists from the template. Add the Contributors Checklist and Review Checklist sections from the description template, and check all applicable items to confirm you have followed the development workflow.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing the _task_capable_initialize() legacy workaround, which is the primary focus of the pull request.
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 remove-task-capable-initialize

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1efc4bc and ac8cc4e.

📒 Files selected for processing (2)
  • src/fastmcp/client/client.py (1 hunks)
  • src/fastmcp/client/tasks.py (0 hunks)
💤 Files with no reviewable changes (1)
  • src/fastmcp/client/tasks.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Write Python code with Python ≥3.10 and include full type annotations
Use specific exception types in error handling - never use bare except
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if they're shorter

Files:

  • src/fastmcp/client/client.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: jlowin
Repo: jlowin/fastmcp PR: 0
File: :0-0
Timestamp: 2025-12-01T15:48:05.095Z
Learning: PR #2505 in fastmcp adds NEW functionality to get_access_token(): it now first checks request.scope["user"] for the token (which never existed before), then falls back to _sdk_get_access_token() (the only thing the original code did). This is not a reversal of order but entirely new functionality to fix stale token issues.
🧬 Code graph analysis (1)
src/fastmcp/client/client.py (1)
src/fastmcp/server/context.py (1)
  • session (393-403)
⏰ 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: Run tests: Python 3.10 on windows-latest
  • GitHub Check: Run tests: Python 3.13 on ubuntu-latest
  • GitHub Check: Run tests with lowest-direct dependencies
  • GitHub Check: Run tests: Python 3.10 on ubuntu-latest
🔇 Additional comments (1)
src/fastmcp/client/client.py (1)

446-463: Spec-aligned initialization looks correct; verify behavior against older/legacy task servers.

Swapping to await self.session.initialize() preserves the cached/idempotent behavior and keeps timeout handling unchanged, which matches the PR intent. The main risk is backwards compatibility if any deployed servers still require the legacy experimental={"tasks": {}} handshake—please confirm the project’s supported minimum server/protocol expectations (and ideally cover with an integration test / release note).


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

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

@jlowin jlowin merged commit ff4e3bb into main Dec 14, 2025
17 of 18 checks passed
@jlowin jlowin deleted the remove-task-capable-initialize branch December 14, 2025 01:29
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: The Windows test suite is timing out during initialization due to a SQLite database lock/hang in .

Root Cause: The test in times out when creates a instance at . The timeout occurs during sqlite3.connect() call within the diskcache library, suggesting a Windows-specific SQLite locking issue.

Despite the isolate_settings_home fixture in conftest.py that creates isolated temp directories per test, Windows tests still encounter this issue. The Windows test job intentionally runs tests serially (without --numprocesses) to avoid parallel SQLite access issues, but the hang still occurs.

Suggested Solution:

Option 1 - Mock DiskStore in auth provider tests (Recommended)
Modify auth provider tests to mock the DiskStore dependency, as these tests are focused on OAuth configuration, not storage behavior:

# In tests/server/auth/providers/test_google.py
from unittest.mock import patch, MagicMock

@pytest.fixture(autouse=True)
def mock_disk_store():
    with patch('fastmcp.server.auth.oauth_proxy.DiskStore') as mock:
        mock.return_value = MagicMock()
        yield mock

Option 2 - Use in-memory storage for tests
Configure tests to use an in-memory key-value store instead of DiskStore on Windows.

Option 3 - Increase timeout for Windows SQLite operations
Mark these specific tests with a longer timeout on Windows, though this doesn't address the root cause.

Detailed Analysis

Stack Trace

File "test_google.py", line 153, in test_extra_authorize_params_add_new_params
    provider = GoogleProvider(...)
File "google.py", line 320, in __init__
    super().__init__(...)
File "oauth_proxy.py", line 821, in __init__
    key_value=DiskStore(directory=settings.home / "oauth-proxy"),
File "diskcache/core.py", line 591, in __init__
    self._sql  # SQLite connection initialization
File "diskcache/core.py", line 623, in _con
    con = self._local.con = sqlite3.connect(...)
[TIMEOUT after 5 seconds]

Why This Happens on Windows

  • Windows file locking is more aggressive than Unix systems
  • SQLite on Windows uses different locking mechanisms that can cause hangs
  • Even with isolated directories per test, Windows may not release locks quickly enough
  • The 5-second pytest timeout is hit before SQLite can establish the connection

Related Files

  • tests/server/auth/providers/test_google.py:151 - Failing test
  • src/fastmcp/server/auth/oauth_proxy.py:821 - DiskStore initialization
  • tests/conftest.py:27-38 - isolate_settings_home fixture
  • Workflow: .github/workflows/run-tests.yml - Windows serial execution config
Related Files
  • tests/server/auth/providers/test_google.py:151-164 - Failing test that creates GoogleProvider
  • src/fastmcp/server/auth/oauth_proxy.py:821 - Where DiskStore is initialized with oauth-proxy directory
  • tests/conftest.py:27-38 - Fixture that isolates settings.home per test (not fully resolving Windows issue)
  • .github/workflows/run-tests.yml - Windows tests run serially to avoid SQLite conflicts

This analysis was generated automatically by the test failure triage bot. Workflow run: https://github.com/jlowin/fastmcp/actions/runs/20200715038

@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: The Windows test suite is timing out during GoogleProvider initialization due to a SQLite database lock/hang in DiskStore.

Root Cause: The test test_extra_authorize_params_add_new_params in tests/server/auth/providers/test_google.py:151 times out when GoogleProvider.__init__ creates a DiskStore instance at settings.home / "oauth-proxy". The timeout occurs during sqlite3.connect() call within the diskcache library, suggesting a Windows-specific SQLite locking issue.

Despite the isolate_settings_home fixture in conftest.py that creates isolated temp directories per test, Windows tests still encounter this issue. The Windows test job intentionally runs tests serially (without --numprocesses) to avoid parallel SQLite access issues, but the hang still occurs.

Suggested Solution:

Option 1 - Mock DiskStore in auth provider tests (Recommended)
Modify auth provider tests to mock the DiskStore dependency, as these tests are focused on OAuth configuration, not storage behavior:

# In tests/server/auth/providers/test_google.py or conftest.py
from unittest.mock import patch, MagicMock

@pytest.fixture(autouse=True)
def mock_disk_store():
    with patch('fastmcp.server.auth.oauth_proxy.DiskStore') as mock:
        mock.return_value = MagicMock()
        yield mock

Option 2 - Use in-memory storage for tests
Configure tests to use an in-memory key-value store instead of DiskStore on Windows.

Option 3 - Increase timeout for Windows SQLite operations
Mark these specific tests with a longer timeout on Windows, though this doesn't address the root cause.

Detailed Analysis

Stack Trace

File "test_google.py", line 153, in test_extra_authorize_params_add_new_params
    provider = GoogleProvider(...)
File "google.py", line 320, in __init__
    super().__init__(...)
File "oauth_proxy.py", line 821, in __init__
    key_value=DiskStore(directory=settings.home / "oauth-proxy"),
File "diskcache/core.py", line 591, in __init__
    self._sql  # SQLite connection initialization
File "diskcache/core.py", line 623, in _con
    con = self._local.con = sqlite3.connect(...)
[TIMEOUT after 5 seconds]

Why This Happens on Windows

  • Windows file locking is more aggressive than Unix systems
  • SQLite on Windows uses different locking mechanisms that can cause hangs
  • Even with isolated directories per test, Windows may not release locks quickly enough
  • The 5-second pytest timeout is hit before SQLite can establish the connection

Related Files

  • tests/server/auth/providers/test_google.py:151 - Failing test
  • src/fastmcp/server/auth/oauth_proxy.py:821 - DiskStore initialization
  • tests/conftest.py:27-38 - isolate_settings_home fixture
  • Workflow: .github/workflows/run-tests.yml - Windows serial execution config
Related Files
  • tests/server/auth/providers/test_google.py:151-164 - Failing test that creates GoogleProvider
  • src/fastmcp/server/auth/oauth_proxy.py:821 - Where DiskStore is initialized with oauth-proxy directory
  • tests/conftest.py:27-38 - Fixture that isolates settings.home per test (not fully resolving Windows issue)
  • .github/workflows/run-tests.yml - Windows tests run serially to avoid SQLite conflicts

This analysis was generated automatically by the test failure triage bot. Workflow run: https://github.com/jlowin/fastmcp/actions/runs/20200715038

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client Related to the FastMCP client SDK or client-side functionality. enhancement Improvement to existing functionality. For issues and smaller PR improvements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant