lizhimins commented on PR #4637:
URL:
https://github.com/apache/rocketmq-dashboard/pull/4637#issuecomment-5761161351
Thanks for the detailed write-up and for the two Go tests — the proxy
plumbing you describe is accurately reasoned, and the framing frames are
handled correctly in isolation.
**We are closing this because nothing in the Studio server initiates a
server→client request today**, so there is no reachable defect to fix.
- `server/src/main/resources/application.yml` declares only
`spring.ai.mcp.server.capabilities.tool: true`; `resource`, `prompt` and
`completion` are all `false`, and no `keep-alive-interval` is configured, so
Spring AI has nothing to ping with.
- `git grep "createMessage|elicitation|roots/list|SamplingMessage"` over
`server/src/main/java` returns nothing — no tool implementation calls sampling,
elicitation or roots.
That means the "no handler → -32601" branch in mcp-go's
`client/transport/streamable_http.go` is not reachable against this server, and
the three methods the description names (`sampling/createMessage`,
`roots/list`, `elicitation/create`) have no call site here. The second half of
the change is unreachable for the same reason: a stdio client only emits an
`{id, result}` frame after the proxy has forwarded a request, and the
forwarding path never runs. Trunk's behaviour for such a frame is one error log
from `decodeMCPMessage` ("invalid MCP JSON-RPC message: missing method") — not
a crash and not data loss. As written this is forward-looking protocol support
for a capability the server does not have yet, rather than a fix.
The review also found four things that would need to change before this
could land as a feature, listed here so a future attempt does not repeat them:
1. Forwarded requests go through `enqueueNotification`, which is a
non-blocking send into a 64-slot buffered channel (`notificationBufferSize` in
`mcp_http.go`). When the buffer is full the request frame is dropped silently,
`forwardRequest` then waits until `ctx.Done()`, the server sees a timeout, and
the log says "MCP notification dropped". Requests need their own channel, or a
blocking/error-returning enqueue.
2. The `forwardRequest` comment says mcp-go invokes the handler from the SSE
reader with a bounded context. In the pinned v0.58.0 it dispatches the handler
in a goroutine (with its own `recover`). The described execution model is wrong.
3. `deliverResponse` only warns and returns nil when there is no pending
request, so the stdio client gets success semantics while the frame never
reaches the server. The documented `Notifications()` contract,
`cmd/mcp_stdio.go` and `fakeMcpSession` all still describe notifications only.
4. `SendMessage` now decodes before `transport.Start(ctx)`, which changes
error precedence for the existing request and notification paths.
**What would change the answer:** a reproduction that is reachable in trunk.
If you can show a configuration or a tool that makes the server open a request
stream — enabling `keep-alive-interval`, or adding elicitation — please open an
issue describing that first and we will take the proxy support alongside it.
Separately, a much smaller PR that only teaches `decodeMCPMessage` to reject a
response frame with an explicit message instead of "missing method" would be
easy to review and is welcome on its own.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
