xintongsong commented on code in PR #539:
URL: https://github.com/apache/flink-agents/pull/539#discussion_r2844388909
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -432,18 +432,22 @@ private void extractJavaMCPServer(Method method) throws
Exception {
JavaSerializableResourceProvider.createResourceProvider(toolName, TOOL, tool));
}
- // Call listPrompts() via reflection
- Method listPromptsMethod =
mcpServer.getClass().getMethod("listPrompts");
- @SuppressWarnings("unchecked")
- Iterable<? extends SerializableResource> prompts =
- (Iterable<? extends SerializableResource>)
listPromptsMethod.invoke(mcpServer);
-
- for (SerializableResource prompt : prompts) {
- Method getNameMethod = prompt.getClass().getMethod("getName");
- String promptName = (String) getNameMethod.invoke(prompt);
- addResourceProvider(
- JavaSerializableResourceProvider.createResourceProvider(
- promptName, PROMPT, prompt));
+ // Call listPrompts() only if the server supports prompts (optional
per MCP spec).
+ // MCP servers like AgentCore Gateway only support tools, not prompts.
+ Method supportsPromptsMethod =
mcpServer.getClass().getMethod("supportsPrompts");
+ if ((boolean) supportsPromptsMethod.invoke(mcpServer)) {
Review Comment:
I think it might be better to check `supportsPrompts` inside `listPrompts`,
so that `listPrompts` returns an empty list rather than fail when the server
doesn't support prompts. `AgentPlan` doesn't need to be aware of this
complexity.
WDYT?
##########
integrations/mcp/src/test/java/org/apache/flink/agents/integrations/mcp/MCPServerTest.java:
##########
@@ -243,4 +243,16 @@ void testClose() {
server.close();
server.close(); // Calling twice should be safe
}
+
+ @Test
+ @DisabledOnJre(JRE.JAVA_11)
+ @DisplayName("supportsPrompts method exists and is callable via
reflection")
+ void testSupportsPromptsMethodExists() throws Exception {
+ MCPServer server = new MCPServer(DEFAULT_ENDPOINT);
+
+ // Verify the method exists (AgentPlan calls it via reflection)
+ java.lang.reflect.Method method =
server.getClass().getMethod("supportsPrompts");
+ assertThat(method).isNotNull();
+ assertThat(method.getReturnType()).isEqualTo(boolean.class);
+ }
Review Comment:
This test case just verifies a method exist and can be called via
reflection. It seems a bit unnecessary.
I think what we need to verify is that, when the mcp server does not support
prompts, the framework should not fail. This would become easier if we move the
`supportsPrompt` check into `listPrompts`, as we just need to verify the
behavior of `MCPServer`, without having to involve `AgentPlan` for the test.
--
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]
