-
Notifications
You must be signed in to change notification settings - Fork 1.5k
refactor: isolate tasks implementation in an experimental
#1174
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
Closed
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
Co-authored-by: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com>
Removes some more now-redundant parameters from RequestTaskStore.createTask and removes a redundant throw from the example.
commit: |
Responses and errors were incorrectly going through the original stream path instead of being queued. Also, extra.sendRequest was not setting the input_required status. These issues have been fixed and tests have been added/updated for them.
Sequence diagram of the intended flow:
```mermaid
sequenceDiagram
participant C as Client Protocol
participant CT as Client Transport
participant ST as Server Transport
participant S as Server Protocol
participant TQ as TaskMessageQueue
participant TS as TaskStore
participant H as Async Handler
Note over C,H: Phase 1: Task Creation
activate C
C->>CT: tools/call { task: { ttl: 60000 } }
activate CT
CT->>ST: HTTP POST
activate ST
ST->>S: _onrequest()
activate S
S->>TS: createTask()
activate TS
TS-->>S: Task { taskId, status: 'working' }
deactivate TS
S--)H: Start async handler (non-blocking)
activate H
S-->>ST: CreateTaskResult { task }
deactivate S
ST-->>CT: HTTP Response
deactivate ST
CT-->>C: CreateTaskResult
deactivate CT
deactivate C
Note over C,H: Phase 2: Server Queues Elicitation Request
H->>S: extra.sendRequest(elicitation, { relatedTask })
activate S
S->>TQ: enqueue({ type: 'request', message: elicitation })
activate TQ
TQ-->>S: OK
deactivate TQ
S->>S: Store resolver in _requestResolvers
Note over S: Promise waiting...
deactivate S
H->>TS: updateTaskStatus('input_required')
activate TS
TS-->>H: OK
deactivate TS
Note over H: Blocked awaiting elicitation response
Note over C,H: Phase 3: Client Polls Status
activate C
C->>CT: tasks/get { taskId }
activate CT
CT->>ST: HTTP POST
activate ST
ST->>S: _onrequest(GetTask)
activate S
S->>TS: getTask(taskId)
activate TS
TS-->>S: Task { status: 'input_required' }
deactivate TS
S-->>ST: Task
deactivate S
ST-->>CT: HTTP Response
deactivate ST
CT-->>C: Task { status: 'input_required' }
deactivate CT
deactivate C
Note over C,H: Phase 4: Client Fetches Queued Messages
activate C
C->>CT: tasks/result { taskId }
activate CT
CT->>ST: HTTP POST
activate ST
ST->>S: _onrequest(GetTaskPayload)
activate S
S->>TQ: dequeue(taskId)
activate TQ
TQ-->>S: { type: 'request', message: elicitation }
deactivate TQ
S->>ST: send(elicitation, { relatedRequestId })
ST-->>CT: SSE Event: elicitation request
Note over S: Handler blocks (task not terminal)
Note over C,H: Phase 5: Client Handles & Responds
CT->>C: _onrequest(elicitation)
activate C
Note over C: Extract relatedTaskId from _meta
C->>C: Call ElicitRequestSchema handler
C->>C: Check: relatedTaskId && _taskMessageQueue
Note over C: _taskMessageQueue is undefined
C->>CT: transport.send(response)
CT->>ST: HTTP POST (elicitation response)
deactivate C
Note over C,H: Phase 6: Server Receives Response, Resolves Promise
ST->>S: _onresponse(elicitation response)
S->>S: Lookup resolver in _requestResolvers
S->>S: resolver(response)
Note over S: Promise resolves
S-->>H: Elicitation result { action: 'accept', content }
Note over H: Resumes execution
Note over C,H: Phase 7: Task Completes
H->>TS: storeTaskResult('completed', finalResult)
activate TS
TS-->>H: OK
deactivate TS
deactivate H
Note over S: GetTaskPayload handler wakes up
S->>TS: getTask(taskId)
activate TS
TS-->>S: Task { status: 'completed' }
deactivate TS
S->>TS: getTaskResult(taskId)
activate TS
TS-->>S: CallToolResult
deactivate TS
S-->>ST: Return final result
deactivate S
ST-->>CT: SSE Event: CallToolResult
deactivate ST
CT-->>C: CallToolResult { content: [...] }
deactivate CT
deactivate C
```
experimental
f0a6f60 to
797e23e
Compare
Phase 1-2 of tasks experimental isolation: - Create src/experimental/tasks/ directory structure - Move TaskStore, TaskMessageQueue, and related interfaces to experimental/tasks/interfaces.ts - Add experimental/tasks/types.ts for re-exporting spec types - Update shared/task.ts to re-export from experimental for backward compatibility - Add barrel exports for experimental module All tests pass (1399 tests).
Restore callTool() to its original implementation instead of delegating to experimental.tasks.callToolStream(). This aligns with Python SDK's approach where call_tool() is task-unaware and call_tool_as_task() is the explicit experimental method. Changes: - Add guard for taskSupport: 'required' tools with clear error message - Restore original output schema validation logic - Add _cachedRequiredTaskTools to track required-only task tools - Remove unused takeResult import Tools with taskSupport: 'optional' work normally with callTool() since the server returns CallToolResult. Only 'required' tools need the experimental API.
6993f64 to
4128d4d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation and Context
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context