From 530f04274608300f8bbafa46467500aed5fdb315 Mon Sep 17 00:00:00 2001 From: Hyunjoon Park Date: Fri, 18 Jul 2025 18:46:57 +0900 Subject: [PATCH 1/2] Add validation for Tool annotation names - Add regex pattern validation for tool names in ToolUtils - Tool names can only contain alphanumeric characters, underscores, hyphens, and dots - Add comprehensive unit tests for tool name validation - Update @Tool annotation documentation with naming constraints - Throw IllegalArgumentException for invalid tool names with clear error message This prevents runtime failures when LLMs (like GPT 4.1-mini) encounter tool names with spaces or special characters that they cannot process. Fixes #3832 Signed-off-by: Hyunjoon Park --- .../ai/tool/annotation/Tool.java | 11 ++ .../ai/tool/support/ToolUtils.java | 31 +++- .../ai/tool/support/ToolUtilsTests.java | 153 ++++++++++++++++++ 3 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 spring-ai-model/src/test/java/org/springframework/ai/tool/support/ToolUtilsTests.java diff --git a/spring-ai-model/src/main/java/org/springframework/ai/tool/annotation/Tool.java b/spring-ai-model/src/main/java/org/springframework/ai/tool/annotation/Tool.java index d17b2fc4773..9b1ff391f1d 100644 --- a/spring-ai-model/src/main/java/org/springframework/ai/tool/annotation/Tool.java +++ b/spring-ai-model/src/main/java/org/springframework/ai/tool/annotation/Tool.java @@ -38,6 +38,17 @@ /** * The name of the tool. If not provided, the method name will be used. + *

+ * Tool names can only contain alphanumeric characters, underscores, hyphens, and + * dots. Spaces and special characters are not allowed. + *

+ *

+ * Examples of valid names: "get_weather", "search-docs", "tool.v1" + *

+ *

+ * Examples of invalid names: "get weather" (contains space), "tool()" (contains + * parentheses) + *

*/ String name() default ""; diff --git a/spring-ai-model/src/main/java/org/springframework/ai/tool/support/ToolUtils.java b/spring-ai-model/src/main/java/org/springframework/ai/tool/support/ToolUtils.java index 4186d935acc..dee245f5b1d 100644 --- a/spring-ai-model/src/main/java/org/springframework/ai/tool/support/ToolUtils.java +++ b/spring-ai-model/src/main/java/org/springframework/ai/tool/support/ToolUtils.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.springframework.ai.tool.ToolCallback; @@ -38,16 +39,27 @@ */ public final class ToolUtils { + /** + * Regular expression pattern for valid tool names. Tool names can only contain + * alphanumeric characters, underscores, hyphens, and dots. + */ + private static final Pattern TOOL_NAME_PATTERN = Pattern.compile("^[a-zA-Z0-9_\\.-]+$"); + private ToolUtils() { } public static String getToolName(Method method) { Assert.notNull(method, "method cannot be null"); var tool = AnnotatedElementUtils.findMergedAnnotation(method, Tool.class); + String toolName; if (tool == null) { - return method.getName(); + toolName = method.getName(); + } + else { + toolName = StringUtils.hasText(tool.name()) ? tool.name() : method.getName(); } - return StringUtils.hasText(tool.name()) ? tool.name() : method.getName(); + validateToolName(toolName); + return toolName; } public static String getToolDescriptionFromName(String toolName) { @@ -102,4 +114,19 @@ public static List getDuplicateToolNames(ToolCallback... toolCallbacks) return getDuplicateToolNames(Arrays.asList(toolCallbacks)); } + /** + * Validates that a tool name conforms to the required pattern. Tool names can only + * contain alphanumeric characters, underscores, hyphens, and dots. + * @param toolName the tool name to validate + * @throws IllegalArgumentException if the tool name contains invalid characters + */ + private static void validateToolName(String toolName) { + Assert.hasText(toolName, "Tool name cannot be null or empty"); + if (!TOOL_NAME_PATTERN.matcher(toolName).matches()) { + throw new IllegalArgumentException( + "Tool name '%s' contains invalid characters. Tool names can only contain alphanumeric characters, underscores, hyphens, and dots." + .formatted(toolName)); + } + } + } diff --git a/spring-ai-model/src/test/java/org/springframework/ai/tool/support/ToolUtilsTests.java b/spring-ai-model/src/test/java/org/springframework/ai/tool/support/ToolUtilsTests.java new file mode 100644 index 00000000000..76b1d02c0b2 --- /dev/null +++ b/spring-ai-model/src/test/java/org/springframework/ai/tool/support/ToolUtilsTests.java @@ -0,0 +1,153 @@ +/* + * Copyright 2023-2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.ai.tool.support; + +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; + +import org.springframework.ai.tool.annotation.Tool; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Unit tests for {@link ToolUtils}. + * + * @author Hyunjoon Park + * @since 1.0.0 + */ +class ToolUtilsTests { + + @Test + void getToolNameFromMethodWithoutAnnotation() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("simpleMethod"); + String toolName = ToolUtils.getToolName(method); + assertThat(toolName).isEqualTo("simpleMethod"); + } + + @Test + void getToolNameFromMethodWithAnnotationButNoName() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("annotatedMethodWithoutName"); + String toolName = ToolUtils.getToolName(method); + assertThat(toolName).isEqualTo("annotatedMethodWithoutName"); + } + + @Test + void getToolNameFromMethodWithValidName() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("methodWithValidName"); + String toolName = ToolUtils.getToolName(method); + assertThat(toolName).isEqualTo("valid_tool-name.v1"); + } + + @Test + void getToolNameFromMethodWithInvalidNameContainingSpaces() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("methodWithSpacesInName"); + assertThatThrownBy(() -> ToolUtils.getToolName(method)).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Tool name 'invalid tool name' contains invalid characters") + .hasMessageContaining( + "Tool names can only contain alphanumeric characters, underscores, hyphens, and dots"); + } + + @Test + void getToolNameFromMethodWithInvalidNameContainingSpecialChars() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("methodWithSpecialCharsInName"); + assertThatThrownBy(() -> ToolUtils.getToolName(method)).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Tool name 'tool@name!' contains invalid characters"); + } + + @Test + void getToolNameFromMethodWithInvalidNameContainingParentheses() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("methodWithParenthesesInName"); + assertThatThrownBy(() -> ToolUtils.getToolName(method)).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Tool name 'tool()' contains invalid characters"); + } + + @Test + void getToolNameFromMethodWithEmptyName() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("methodWithEmptyName"); + // When name is empty, it falls back to method name which is valid + String toolName = ToolUtils.getToolName(method); + assertThat(toolName).isEqualTo("methodWithEmptyName"); + } + + @Test + void getToolDescriptionFromMethodWithoutAnnotation() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("simpleMethod"); + String description = ToolUtils.getToolDescription(method); + assertThat(description).isEqualTo("simple method"); + } + + @Test + void getToolDescriptionFromMethodWithAnnotationButNoDescription() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("annotatedMethodWithoutName"); + String description = ToolUtils.getToolDescription(method); + assertThat(description).isEqualTo("annotatedMethodWithoutName"); + } + + @Test + void getToolDescriptionFromMethodWithDescription() throws NoSuchMethodException { + Method method = TestTools.class.getMethod("methodWithDescription"); + String description = ToolUtils.getToolDescription(method); + assertThat(description).isEqualTo("This is a tool description"); + } + + // Test helper class with various tool methods + public static class TestTools { + + public void simpleMethod() { + // Method without @Tool annotation + } + + @Tool + public void annotatedMethodWithoutName() { + // Method with @Tool but no name specified + } + + @Tool(name = "valid_tool-name.v1") + public void methodWithValidName() { + // Method with valid tool name + } + + @Tool(name = "invalid tool name") + public void methodWithSpacesInName() { + // Method with spaces in tool name (invalid) + } + + @Tool(name = "tool@name!") + public void methodWithSpecialCharsInName() { + // Method with special characters in tool name (invalid) + } + + @Tool(name = "tool()") + public void methodWithParenthesesInName() { + // Method with parentheses in tool name (invalid) + } + + @Tool(name = "") + public void methodWithEmptyName() { + // Method with empty name (falls back to method name) + } + + @Tool(description = "This is a tool description") + public void methodWithDescription() { + // Method with description + } + + } + +} From 5604fcf9d674a66fc750a1eb2e49a719f4c414e1 Mon Sep 17 00:00:00 2001 From: Hyunjoon Park Date: Wed, 23 Jul 2025 11:59:14 +0900 Subject: [PATCH 2/2] Change Tool name validation from exception to warning Based on feedback, the validation has been changed from throwing an exception to logging a warning. This approach is more flexible as the Tool annotation is generic and not specific to any particular LLM. Changes: - Modified ToolUtils to log warnings instead of throwing exceptions - Updated Tool annotation JavaDoc to recommend naming conventions - Adjusted tests to verify tool names are returned (no exceptions thrown) - Added test for unicode characters to support non-English contexts The warning message guides users toward compatible naming while allowing flexibility for different LLMs and use cases. Fixes spring-projects#3832 Signed-off-by: Hyunjoon Park --- .../ai/tool/annotation/Tool.java | 11 +++--- .../ai/tool/support/ToolUtils.java | 24 +++++++----- .../ai/tool/support/ToolUtilsTests.java | 37 +++++++++++++------ 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/spring-ai-model/src/main/java/org/springframework/ai/tool/annotation/Tool.java b/spring-ai-model/src/main/java/org/springframework/ai/tool/annotation/Tool.java index 9b1ff391f1d..5e3735afc68 100644 --- a/spring-ai-model/src/main/java/org/springframework/ai/tool/annotation/Tool.java +++ b/spring-ai-model/src/main/java/org/springframework/ai/tool/annotation/Tool.java @@ -39,15 +39,16 @@ /** * The name of the tool. If not provided, the method name will be used. *

- * Tool names can only contain alphanumeric characters, underscores, hyphens, and - * dots. Spaces and special characters are not allowed. + * For maximum compatibility across different LLMs, it is recommended to use only + * alphanumeric characters, underscores, hyphens, and dots in tool names. Using spaces + * or special characters may cause issues with some LLMs (e.g., OpenAI). *

*

- * Examples of valid names: "get_weather", "search-docs", "tool.v1" + * Examples of recommended names: "get_weather", "search-docs", "tool.v1" *

*

- * Examples of invalid names: "get weather" (contains space), "tool()" (contains - * parentheses) + * Examples of names that may cause compatibility issues: "get weather" (contains + * space), "tool()" (contains parentheses) *

*/ String name() default ""; diff --git a/spring-ai-model/src/main/java/org/springframework/ai/tool/support/ToolUtils.java b/spring-ai-model/src/main/java/org/springframework/ai/tool/support/ToolUtils.java index dee245f5b1d..baa03b830cb 100644 --- a/spring-ai-model/src/main/java/org/springframework/ai/tool/support/ToolUtils.java +++ b/spring-ai-model/src/main/java/org/springframework/ai/tool/support/ToolUtils.java @@ -23,6 +23,9 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import org.springframework.ai.tool.ToolCallback; import org.springframework.ai.tool.annotation.Tool; import org.springframework.ai.tool.execution.DefaultToolCallResultConverter; @@ -39,11 +42,14 @@ */ public final class ToolUtils { + private static final Logger logger = LoggerFactory.getLogger(ToolUtils.class); + /** - * Regular expression pattern for valid tool names. Tool names can only contain - * alphanumeric characters, underscores, hyphens, and dots. + * Regular expression pattern for recommended tool names. Tool names should contain + * only alphanumeric characters, underscores, hyphens, and dots for maximum + * compatibility across different LLMs. */ - private static final Pattern TOOL_NAME_PATTERN = Pattern.compile("^[a-zA-Z0-9_\\.-]+$"); + private static final Pattern RECOMMENDED_NAME_PATTERN = Pattern.compile("^[a-zA-Z0-9_\\.-]+$"); private ToolUtils() { } @@ -115,17 +121,15 @@ public static List getDuplicateToolNames(ToolCallback... toolCallbacks) } /** - * Validates that a tool name conforms to the required pattern. Tool names can only - * contain alphanumeric characters, underscores, hyphens, and dots. + * Validates that a tool name follows recommended naming conventions. Logs a warning + * if the tool name contains characters that may not be compatible with some LLMs. * @param toolName the tool name to validate - * @throws IllegalArgumentException if the tool name contains invalid characters */ private static void validateToolName(String toolName) { Assert.hasText(toolName, "Tool name cannot be null or empty"); - if (!TOOL_NAME_PATTERN.matcher(toolName).matches()) { - throw new IllegalArgumentException( - "Tool name '%s' contains invalid characters. Tool names can only contain alphanumeric characters, underscores, hyphens, and dots." - .formatted(toolName)); + if (!RECOMMENDED_NAME_PATTERN.matcher(toolName).matches()) { + logger.warn("Tool name '{}' may not be compatible with some LLMs (e.g., OpenAI). " + + "Consider using only alphanumeric characters, underscores, hyphens, and dots.", toolName); } } diff --git a/spring-ai-model/src/test/java/org/springframework/ai/tool/support/ToolUtilsTests.java b/spring-ai-model/src/test/java/org/springframework/ai/tool/support/ToolUtilsTests.java index 76b1d02c0b2..dbf432d4b7c 100644 --- a/spring-ai-model/src/test/java/org/springframework/ai/tool/support/ToolUtilsTests.java +++ b/spring-ai-model/src/test/java/org/springframework/ai/tool/support/ToolUtilsTests.java @@ -55,26 +55,28 @@ void getToolNameFromMethodWithValidName() throws NoSuchMethodException { } @Test - void getToolNameFromMethodWithInvalidNameContainingSpaces() throws NoSuchMethodException { + void getToolNameFromMethodWithNameContainingSpaces() throws NoSuchMethodException { + // Tool names with spaces are now allowed but will generate a warning log Method method = TestTools.class.getMethod("methodWithSpacesInName"); - assertThatThrownBy(() -> ToolUtils.getToolName(method)).isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Tool name 'invalid tool name' contains invalid characters") - .hasMessageContaining( - "Tool names can only contain alphanumeric characters, underscores, hyphens, and dots"); + String toolName = ToolUtils.getToolName(method); + assertThat(toolName).isEqualTo("invalid tool name"); } @Test - void getToolNameFromMethodWithInvalidNameContainingSpecialChars() throws NoSuchMethodException { + void getToolNameFromMethodWithNameContainingSpecialChars() throws NoSuchMethodException { + // Tool names with special characters are now allowed but will generate a warning + // log Method method = TestTools.class.getMethod("methodWithSpecialCharsInName"); - assertThatThrownBy(() -> ToolUtils.getToolName(method)).isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Tool name 'tool@name!' contains invalid characters"); + String toolName = ToolUtils.getToolName(method); + assertThat(toolName).isEqualTo("tool@name!"); } @Test - void getToolNameFromMethodWithInvalidNameContainingParentheses() throws NoSuchMethodException { + void getToolNameFromMethodWithNameContainingParentheses() throws NoSuchMethodException { + // Tool names with parentheses are now allowed but will generate a warning log Method method = TestTools.class.getMethod("methodWithParenthesesInName"); - assertThatThrownBy(() -> ToolUtils.getToolName(method)).isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Tool name 'tool()' contains invalid characters"); + String toolName = ToolUtils.getToolName(method); + assertThat(toolName).isEqualTo("tool()"); } @Test @@ -106,6 +108,14 @@ void getToolDescriptionFromMethodWithDescription() throws NoSuchMethodException assertThat(description).isEqualTo("This is a tool description"); } + @Test + void getToolNameFromMethodWithUnicodeCharacters() throws NoSuchMethodException { + // Tool names with unicode characters should be allowed for non-English contexts + Method method = TestTools.class.getMethod("methodWithUnicodeName"); + String toolName = ToolUtils.getToolName(method); + assertThat(toolName).isEqualTo("获取天气"); + } + // Test helper class with various tool methods public static class TestTools { @@ -148,6 +158,11 @@ public void methodWithDescription() { // Method with description } + @Tool(name = "获取天气") + public void methodWithUnicodeName() { + // Method with unicode characters in tool name (Chinese: "get weather") + } + } }