Copilot commented on code in PR #6997:
URL: https://github.com/apache/shenyu/pull/6997#discussion_r4037757224
##########
shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apache/shenyu/plugin/mcp/server/transport/ShenyuStreamableHttpServerTransportProvider.java:
##########
@@ -1059,6 +1060,19 @@ private class StreamableHttpSessionTransport implements
McpServerTransport {
LOGGER.debug("Created StreamableHttpSessionTransport with
auto-generated sessionId: {}", this.sessionId);
}
+ /**
+ * Sets the session ID to match the MCP server session's ID.
+ * This ensures the transport's sessionId is consistent with the key
used
+ * to store the session in {@code sessions} and {@code
sessionTransports},
+ * so that {@link #close()} and {@link #closeGracefully()} correctly
+ * remove the session.
+ *
+ * @param sessionId the session identifier to use
+ */
+ public void setSessionId(final String sessionId) {
+ this.sessionId = sessionId;
Review Comment:
`sessionId` is declared `final` at line 1047 and is already assigned by both
constructors, so this setter assignment does not compile (`cannot assign a
value to final variable sessionId`). Remove `final` or redesign construction so
the MCP session ID is established without reassigning a final field.
##########
shenyu-plugin/shenyu-plugin-mcp-server/src/main/java/org/apache/shenyu/plugin/mcp/server/transport/ShenyuStreamableHttpServerTransportProvider.java:
##########
@@ -326,6 +326,7 @@ private Mono<MessageHandlingResult>
handleInitializeRequest(final ServerWebExcha
final StreamableHttpSessionTransport transport = new
StreamableHttpSessionTransport();
final McpServerSession session = sessionFactory.create(transport);
final String newSessionId = session.getId();
+ transport.setSessionId(newSessionId);
Review Comment:
Add a regression test for the reported initialize-path leak: initialize a
real session, close its transport, and assert that the returned session ID is
removed from `sessions`, `sessionTransports`, and `ShenyuMcpExchangeHolder`.
Existing tests cover cleanup only for the separate stale-session restore path,
so they would not prevent this key mismatch from recurring.
--
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]
