Write updated docs and port SEP-2663 content#2
Conversation
| ``` | ||
|
|
||
| The notification includes the full task object, allowing clients to access the complete task state and final results without polling the `tasks/get` method. Clients **MAY** continue polling `tasks/get` in addition to subscribing to task status notifications, but need not do so. | ||
|
|
There was a problem hiding this comment.
What about Progress notifications? can these also come on this stream? We could set the _meta field to include the taskId. This would preserve existing functionality and give folks a path to partial results?
Or we can omit for now and solve later. But we should add language to expressly disallow progress notifications on this stream then.
There was a problem hiding this comment.
Highlighting this one for @kurtisvg to chime in on — I think for now we should explicitly disallow this?
We actually have three options here:
- Allow progress notifications for tasks on
subscriptions/listen, which is simple to implement but inconsistent with how progress works in all other cases. This is inconsistent withsubscriptions/listenin that it imposes a disjoint requirement. - Disallow it, but continue to allow it piecemeal over SSE responses from individual
tasks/getrequests, which I've done and it's not easy, but this is also the status quo and is consistent with thesubscriptions/listenspec. - Disallow it, and also disallow it over
tasks/get: This is what I think we should do — it means progress effectively isn't usable with Tasks anymore, but leaves the door open for us to find a better home for progress information in Tasks later. This is inconsistent withsubscriptions/listenin that it is more restrictive.
I'm going to disallow it explicitly period for now, but I'm laying out the options here so we can have informed discussions about it.
edit: disallowed in a0824a2
Co-authored-by: Caitie McCaffrey <caitiem20@github.com>
Co-authored-by: Caitie McCaffrey <caitiem20@github.com>
|
Addressed all feedback up to this point. |
Writes new overview docs and ports content from modelcontextprotocol/modelcontextprotocol#2663