Adding MCP call (via HTTP) - #1116
Conversation
ricardozanini
left a comment
There was a problem hiding this comment.
Thank you! I have a few questions if you don't mind.
|
@amin-nikanjam Could you please sign-off your commits? |
Done! |
|
@amin-nikanjam DCO is still not passing. Please follow the steps enumerated in the following link to ensure it checks: https://github.com/serverlessworkflow/specification/pull/1116/checks?check_run_id=49885563716 |
|
@amin-nikanjam Why are we not supporting stdio transport, too? I feel it is but trivial to add support for it, isn't it? |
Right, we are not supporting the stdio now. While in MCP spec it is clarified that "Clients SHOULD support stdio whenever possible", we can add it. |
|
@amin-nikanjam apart from adding |
b57bac1 to
fbade4a
Compare
|
@amin-nikanjam can you please squash and sign your commit? |
sure, I will wrap it up in a few days. |
@ricardozanini @cdavernas So, I checked the MCP spec and several implementations, in the spec it's clarified that for STDIO, "the client launches the MCP server as a subprocess", so the client has to have the "server_path" to run it. Moreover, after initialization, the client should send "JSON-RPC messages to stdin and receive messages from the server using stdout". What is the best way to include it in the specification? like a "Shell Process"? |
|
@amin-nikanjam I think there’s some confusion here. The client launching the server as a stdio subprocess is simply an implementation detail—it doesn’t require a dedicated task in the spec. The MCP call itself must support both transport modes; it’s up to implementers how they handle the subprocess. In practice, supporting both is straightforward, and there’s no need to over-specify things at the level of “shell process” in the spec. |
Got it. I made the necessary changes and pushed the commit. |
|
@amin-nikanjam I think you rebased your fork with the latest commits from main. You can either rebase it again and work through your local git logs to pick the correct commits or open a new PR with a fresh branch. |
Please specify parts of this PR update:
Discussion or Issue link:
What this PR does:
Additional information: