-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: call tasks/result to deliver side-channel messages #1185
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
fix: call tasks/result to deliver side-channel messages #1185
Conversation
commit: |
436d05b to
0601671
Compare
He-Pin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR needs to update the server side to make it behave as the spec just said, the client should and will see an input_required asap the server requires it to make the task progress.
…ssages Add input_required handling to the polling loop in requestStream(). When task status is input_required, call tasks/result to deliver queued messages (elicitation, sampling) via SSE and block until terminal.
0601671 to
9608c03
Compare
|
btw, your implementations in your work probably all use databases, right? To be honest, I haven't fully implemented the server-side logic yet... It's much more complex than I imagined, because I also have various connectors as tools. |
| const result = await this.getTaskResult({ taskId }, resultSchema, options); | ||
| yield { type: 'result', result }; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool
|
@LucaButBoring would be great to get your thoughts on this one. |
|
Looks good, sorry about that — there was an existing integ test that checked for the |
Should we be doing anything in particular in the |
|
That is correct — |
|
only swith to tasks/result when the state changed to failed, completed, input_required |
|
Does the inspector depends on this sdk?I want to use it asap |

Fixes a bug discovered during manual testing of Tasks (SEP-1686):
requestStream()polledtasks/getbut never calledtasks/resultduring non-terminal states, so side-channel messages (elicitation, sampling) were never delivered to the client, causing interactive tools to hang indefinitely atinput_requiredstate.Motivation and Context
After merging #1041, manual testing revealed that interactive tools (elicitation/sampling) would hang indefinitely because the client never received the elicitation request messages.
How Has This Been Tested?
simpleTaskInteractive.tsserver and client examplescallToolStream with elicitationintaskLifecycle.test.tsWith the example clients added here:
Breaking Changes
None
Types of changes
Checklist
Additional context
The fix adds
input_requiredhandling to the polling loop. When the task status isinput_required, we calltasks/resultwhich delivers queued messages via SSE and blocks until the task reaches a terminal state. This preserves the polling loop's ability to yield intermediate status updates while ensuring side-channel messages are delivered.