Skip to content

Add validation for prompt name uniqueness across the service. #833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 6, 2025
+721 −7
Merged

Add validation for prompt name uniqueness across the service. #833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .../mcp-server/src/main/resources/software/amazon/smithy/java/example/server/mcp/main.smithy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use smithy.ai#prompts
arguments: GetCodingStatisticsInput
preferWhen: "User wants to analyze developer productivity, review coding activity, or understand technology usage patterns"
}
employee_lookup: {
Test_employee: {
description: "General employee lookup and information retrieval service"
template: "This service provides employee information including personal details and coding statistics. Use get_employee_info for basic details or get_coding_stats for development metrics."
preferWhen: "User needs any employee-related information or wants to understand available employee data"
Expand Down
4 changes: 3 additions & 1 deletion mcp/mcp-server/src/main/java/software/amazon/smithy/java/mcp/server/McpServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package software.amazon.smithy.java.mcp.server;

import static software.amazon.smithy.java.mcp.server.PromptLoader.normalize;

import java.io.InputStream;
import java.io.OutputStream;
import java.io.PrintWriter;
Expand Down Expand Up @@ -146,7 +148,7 @@ private void handleRequest(JsonRpcRequest req) {
var promptName = req.getParams().getMember("name").asString();
var promptArguments = req.getParams().getMember("arguments");

var prompt = prompts.get(promptName);
var prompt = prompts.get(normalize(promptName));

if (prompt == null) {
internalError(req, new RuntimeException("Prompt not found: " + promptName));
Expand Down
20 changes: 18 additions & 2 deletions mcp/mcp-server/src/main/java/software/amazon/smithy/java/mcp/server/PromptLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public static Map<String, Prompt> loadPrompts(Collection<Service> services) {

});
for (Map.Entry<String, PromptTemplateDefinition> entry : promptDefinitions.entrySet()) {
var promptName = entry.getKey().toLowerCase();
var promptName = entry.getKey();
var promptTemplateDefinition = entry.getValue();
var templateString = promptTemplateDefinition.getTemplate();

Expand All @@ -76,8 +76,20 @@ public static Map<String, Prompt> loadPrompts(Collection<Service> services) {
: List.of())
.build();

var normalizedName = normalize(promptName);

if (prompts.containsKey(normalizedName)) {
var existingPrompt = prompts.get(normalizedName);
throw new RuntimeException(String.format(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better exception I can throw here that will probably alert user that the server isn't loading due to prompt names?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message should be surfaced to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the only way would be to surface this at client during runtime, when the MCP Client starts it up.

"Duplicate normalized prompt name '%s' found. " +
"Original name '%s' conflicts with previously registered name '%s'.",
normalizedName,
promptName,
existingPrompt.promptInfo().getName()));
}

prompts.put(
promptName,
normalizedName,
new Prompt(promptInfo, finalTemplateString));
}
}
Expand Down Expand Up @@ -118,4 +130,8 @@ public static List<PromptArgument> convertArgumentShapeToPromptArgument(Schema a

return promptArguments;
}

public static String normalize(String promptName) {
return promptName.toLowerCase();
}
}
89 changes: 89 additions & 0 deletions mcp/mcp-server/src/test/java/software/amazon/smithy/java/mcp/server/McpServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,16 @@ void testPromptsList() {
assertEquals("search_users", servicePromptMap.get("name").asString());
assertEquals("Test Template", servicePromptMap.get("description").asString());
assertTrue(servicePromptMap.get("arguments").asList().isEmpty());

var promptNames = prompts.stream()
.map(p -> p.asStringMap().get("name").asString())
.toList();
assertTrue(promptNames.contains("search_users"));
assertTrue(promptNames.contains("perform_operation"));

for (String name : promptNames) {
assertEquals(name.toLowerCase(), name, "Prompt name should be normalized to lowercase: " + name);
}
}

@Test
Expand Down Expand Up @@ -430,6 +440,76 @@ void testPromptsGetWithValidPrompt() {
assertEquals("Search for if many results expected.", content.get("text").asString());
}

@Test
void testPromptsGetWithDifferentCasing() {
server = McpServer.builder()
.input(input)
.output(output)
.addService("test-mcp",
ProxyService.builder()
.service(ShapeId.from("smithy.test#TestService"))
.proxyEndpoint("http://localhost")
.model(MODEL)
.build())
.build();

server.start();

// Test with uppercase prompt name
write("prompts/get",
Document.of(Map.of(
"name",
Document.of("SEARCH_USERS"))));
var response = read();
var result = response.getResult().asStringMap();

assertEquals("Test Template", result.get("description").asString());
var messages = result.get("messages").asList();
assertEquals(1, messages.size());

var message = messages.get(0).asStringMap();
assertEquals("user", message.get("role").asString());
var content = message.get("content").asStringMap();
assertEquals("text", content.get("type").asString());
assertEquals("Search for if many results expected.", content.get("text").asString());

// Test with mixed case prompt name
write("prompts/get",
Document.of(Map.of(
"name",
Document.of("Search_Users"))));
response = read();
result = response.getResult().asStringMap();

assertEquals("Test Template", result.get("description").asString());
messages = result.get("messages").asList();
assertEquals(1, messages.size());

message = messages.get(0).asStringMap();
assertEquals("user", message.get("role").asString());
content = message.get("content").asStringMap();
assertEquals("text", content.get("type").asString());
assertEquals("Search for if many results expected.", content.get("text").asString());

// Test with perform_operation prompt in different case
write("prompts/get",
Document.of(Map.of(
"name",
Document.of("PERFORM_OPERATION"))));
response = read();
result = response.getResult().asStringMap();

assertEquals("perform operation", result.get("description").asString());
messages = result.get("messages").asList();
assertEquals(1, messages.size());

message = messages.get(0).asStringMap();
assertEquals("user", message.get("role").asString());
content = message.get("content").asStringMap();
assertEquals("text", content.get("type").asString());
assertEquals("use tool TestOperation with some information.", content.get("text").asString());
}

@Test
void testPromptsGetWithInvalidPrompt() {
server = McpServer.builder()
Expand All @@ -452,6 +532,15 @@ void testPromptsGetWithInvalidPrompt() {
var response = read();
assertNotNull(response.getError());
assertTrue(response.getError().getMessage().contains("Prompt not found: nonexistent_prompt"));

// Test with invalid prompt in different case - should still fail
write("prompts/get",
Document.of(Map.of(
"name",
Document.of("NONEXISTENT_PROMPT"))));
response = read();
assertNotNull(response.getError());
assertTrue(response.getError().getMessage().contains("Prompt not found: NONEXISTENT_PROMPT"));
}

@Test
Expand Down
Loading
Loading