-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Replace Docket shims with native 0.13.1 APIs #2417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Migrates from manual task state/result storage to Docket's native execution tracking. The `_temporary_docket_shims.py` file provided workarounds for missing Docket features - those are now available in Docket 0.13.1. Uses Docket's native APIs: - `docket.get_execution(task_key)` for execution lookups - `execution.get_result()` for result retrieval - `execution.state` and `execution.sync()` for state tracking - Docket's `execution_ttl` for automatic result expiration - Store `self._docket` on server for cross-task access Sync function handling: - Sync functions automatically disabled for tasks (Docket requires async) - Warns when user explicitly sets `task=True` on sync function - Quietly disables when inherited from server settings Simplified `_temporary_mcp_shims.py`: - Removed `_task_keep_alive` (use default 60s) - Removed `cancel_task()` and `delete_task()` wrappers - Kept `_task_id_mapping` (MCP SDK limitation) - Kept `_cancelled_tasks` (Docket lacks CANCELLED state) Client improvements: - Added private `_send_custom_request()` for raw JSON-RPC (network transports) - FastMCPTransport calls server handlers directly (SDK validates both sides) - Clear TODO SEP-1686 comments mark temporary workarounds Test reorganization: - Created `test_client_tool_tasks.py` and `test_task_tools.py` - Renamed `test_client_tasks.py` → `test_client_task_protocol.py` - Tool/prompt/resource tests now parallel - Added 9 sync function tests Results: 147 pass, 3 skipped (per-task TTL - Docket uses global) Related: chrisguidry/docket#167, chrisguidry/docket#166, chrisguidry/docket#88 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The client needs to send tasks/get, tasks/result, and tasks/delete protocol messages, but the MCP SDK doesn't support them yet. We tried adding handlers but discovered the SDK validates incoming requests against a closed union of types. Implemented monkey-patching solution: - Extend ClientRequest and ServerRequest unions with our custom TasksGetRequest, TasksResultRequest, TasksDeleteRequest types - Use `model_rebuild(force=True)` to force Pydantic to rebuild validation schema (it caches on first use) - Register handlers directly in the server for these custom request types - Create TasksResponse wrapper that intelligently parses dicts back to MCP types The old code in MiddlewareServerSession was directly calling task methods, which prevented proper protocol flow. Removed that and now everything goes through registered handlers. All marked with `TODO SEP-1686` comments for cleanup once the SDK officially supports these methods. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
When TasksResponse.model_validate() parses a ReadResourceResult dict back into an object, ResourceTask.result() was failing to handle it properly. The code checked for dicts but not for the parsed MCP objects.
The issue: when iterating over a Pydantic model directly, Python iterates over field names ("meta", "contents") as tuples, not the field values. This caused tests to fail with "AttributeError: 'tuple' object has no attribute 'text'".
Fixed by checking if mcp_result is already a ReadResourceResult object first, then extracting .contents directly. Falls back to dict/list handling for other cases.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Removed the unused _send_custom_request method that was replaced by monkey-patching approach. Updated list_tasks to send tasks/list protocol message instead of iterating and calling get_task_status repeatedly. Falls back to client-side tracking if server returns empty (which is the current design for session isolation). Added TasksListRequest/Params to monkey-patch and registered handler in server. All task operations now go through the protocol consistently. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Added proper tasks/cancel protocol support: - Created TasksCancelRequest and TasksCancelParams - Added to monkey-patch unions for both ClientRequest and ServerRequest - Implemented _tasks_cancel_mcp server handler that cancels via Docket and returns cancelled status - Updated client cancel_task() to send tasks/cancel protocol message Removed all direct server access from cancel_task - it now sends proper protocol messages like the other task methods. Tasks/cancel returns the task status (unlike delete which removes the task entirely). All 147 tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Now that Docket 0.13.2 provides native ExecutionState.CANCELLED support, removed the manual tracking workaround and rely on Docket's state management. Changes: - Updated pydocket requirement to >=0.13.2 - Added ExecutionState.CANCELLED to DOCKET_TO_MCP_STATE mapping - Removed _cancelled_tasks set from _temporary_mcp_shims - Removed manual cancelled tracking in tasks/get, tasks/cancel, tasks/delete - Cleaned up test fixtures that referenced _cancelled_tasks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Extracted ~730 lines of task-related code from the 3900+ line server.py into
focused modules under src/fastmcp/server/tasks/ and replaced the in-memory
task mapping with Redis-backed storage using Docket's Redis instance.
Changes:
- New handlers.py: Task execution handlers (handle_*_as_task functions)
- New protocol.py: Protocol handlers (tasks/get, tasks/result, tasks/list, tasks/cancel, tasks/delete)
- New converters.py: Result conversion logic (convert_*_result functions)
- Updated __init__.py: Exports public API
- Converted instance methods to standalone functions taking server param
- Used Context wrapper: `async with Context(fastmcp=server) as ctx:` for clean session_id access
- Replaced `_task_id_mapping` dict with Redis storage at `fastmcp:task:{session_id}:{client_task_id}`
- TTL: Docket's execution_ttl + 15 minutes (TASK_MAPPING_TTL_BUFFER_SECONDS constant)
- Removed `_task_id_mapping`, `_lock`, `set_state()`, `resolve_task_id()` from _temporary_mcp_shims
- Removed `_cancelled_tasks` tracking (now uses native Docket ExecutionState.CANCELLED)
- Updated pydocket requirement to >=0.13.2 for CANCELLED state support
- Updated test fixtures and metadata tests to use client protocol methods
Result: server.py reduced from 3938 → 3209 lines (19% reduction), all 147 tests passing
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
dependencies
Updates to project dependencies. Automatically applied to dependabot PRs.
enhancement
Improvement to existing functionality. For issues and smaller PR improvements.
tests
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Migrates from manual task state/result storage to Docket's native execution tracking. The
_temporary_docket_shims.pyfile provided workarounds for missing Docket features - those are now available in Docket 0.13.1.Uses Docket's native APIs:
docket.get_execution(task_key)for execution lookupsexecution.get_result()for result retrievalexecution.stateandexecution.sync()for state trackingexecution_ttlfor automatic result expirationself._docketon server for cross-task accessSync function handling:
task=Trueon sync functionSimplified
_temporary_mcp_shims.py:_task_keep_alive(use default 60s)cancel_task()anddelete_task()wrappers_task_id_mapping(MCP SDK limitation)_cancelled_tasks(Docket lacks CANCELLED state)Client improvements:
_send_custom_request()for raw JSON-RPC (network transports)Test reorganization:
test_client_tool_tasks.pyandtest_task_tools.pytest_client_tasks.py→test_client_task_protocol.pyResults: 147 pass, 3 skipped (per-task TTL - Docket uses global)
Related: chrisguidry/docket#167, chrisguidry/docket#166, chrisguidry/docket#88
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]