feat: add streaming methods for elicitation and sampling#1210
feat: add streaming methods for elicitation and sampling#1210LucaButBoring wants to merge 16 commits intomodelcontextprotocol:mainfrom
Conversation
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
f8755d8 to
7520307
Compare
7520307 to
4ed61b6
Compare
|
Doing some manual testing on this, converted back to draft temporarily |
|
Validated with new example tool. |
ba50e0a to
771f101
Compare
…ript-sdk into feat/elicitation-sampling-streaming
|
…ript-sdk into LucaButBoring-feat/elicitation-sampling-streaming # Conflicts: # examples/client/src/simpleStreamableHttp.ts
|
@LucaButBoring could you resolve the conflicts on this PR? Hoping to get it merged soon. |
…ript-sdk into feat/elicitation-sampling-streaming
- Replace schema-based setRequestHandler with string methods - Update context API from extra.taskStore to extra.task?.store - Add CreateTaskResult to ResultTypeMap for elicitation/create sampling/createMessage
|
@cliffhall updated 👍 |
| type ServerWithCapabilities = { | ||
| getClientCapabilities(): { elicitation?: { form?: boolean; url?: boolean } } | undefined; | ||
| }; | ||
| const clientCapabilities = (this._server as unknown as ServerWithCapabilities).getClientCapabilities(); |
There was a problem hiding this comment.
Hi, could we achieve this without the as unknown? It's something we're actively trying to avoid. Is there something not matching on the ClientCapabilities.elicitation that we need to fix here? (them not being a boolean but an object | undefined)? Would rather fix that than have a proprietary casting here, basically forfeiting the type system fully for it
The checks bellow would pass even if elicitation.form or elicitation.url were objects and not boolean, so is that really needed
There was a problem hiding this comment.
Oh, I think that was due to other type inference failures around _server at the time this was originally written which are no longer applicable, I'm doing a scrub for this again.
There was a problem hiding this comment.
FWIW the hard casts are needed around getTask and getTaskResult still either way, and that won't be addressed until #1449 is merged.
There was a problem hiding this comment.
Alright, updated - this cast and the similar ones in sampling are no longer needed and have been removed.
Following #1041, this adds streaming methods for elicitation and sampling to simplify usage with Tasks.
Motivation and Context
Introduces a consistent way to use Tasks with elicitation and sampling just like we have with tool calls.
How Has This Been Tested?
New unit tests and example tool.
Breaking Changes
N/A
Types of changes
Checklist
Additional context