feat: enforce underscore MCP tool names
Some checks failed
Deploy to OCIWP / deploy (push) Failing after 0s
Some checks failed
Deploy to OCIWP / deploy (push) Failing after 0s
This commit is contained in:
@@ -0,0 +1,21 @@
|
||||
package io.shinhanlife.dap.lib.config;
|
||||
|
||||
import io.shinhanlife.glow.communication.module.http.component.GlowHttpComponent;
|
||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.web.client.RestClient;
|
||||
|
||||
/**
|
||||
* Registers the temporary Glow HTTP compatibility component from the DAP library scan scope.
|
||||
* The bean is only created when an official GlowHttpComponent has not already been supplied.
|
||||
*/
|
||||
@Configuration(proxyBeanMethods = false)
|
||||
public class AxhubHttpConfiguration {
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean(GlowHttpComponent.class)
|
||||
public GlowHttpComponent glowHttpComponent(RestClient.Builder restClientBuilder) {
|
||||
return new GlowHttpComponent(restClientBuilder);
|
||||
}
|
||||
}
|
||||
@@ -940,7 +940,7 @@ public class ToolScaffolder {
|
||||
String[] words = normalizedName.split("\\s+");
|
||||
String service = words[0];
|
||||
String action = words.length == 1 ? "execute" : words[words.length - 1];
|
||||
return "%s.%s.%s.%s".formatted(
|
||||
return "%s_%s_%s_%s".formatted(
|
||||
pod.toLowerCase(Locale.ROOT),
|
||||
group.toLowerCase(Locale.ROOT),
|
||||
service,
|
||||
|
||||
@@ -28,7 +28,7 @@ import java.util.regex.Pattern;
|
||||
public final class McpToolNameValidator {
|
||||
|
||||
private static final Pattern TOOL_NAME_PATTERN = Pattern.compile("\\bname\\s*=\\s*\\\"([^\\\"]+)\\\"");
|
||||
private static final Pattern TOOL_NAME_CONVENTION = Pattern.compile("^[a-z][a-z0-9-]*\\.[a-z][a-z0-9-]*\\.[a-z][a-z0-9-]*\\.[a-z][a-z0-9-]*$");
|
||||
private static final Pattern TOOL_NAME_CONVENTION = Pattern.compile("^[a-zA-Z0-9_-]{1,128}$");
|
||||
|
||||
private McpToolNameValidator() {
|
||||
}
|
||||
@@ -139,7 +139,7 @@ public final class McpToolNameValidator {
|
||||
}
|
||||
|
||||
private static String buildInvalidNameMessage(List<Map.Entry<String, List<ToolDeclaration>>> invalidNames) {
|
||||
StringBuilder message = new StringBuilder("Invalid MCP tool name(s): expected pod.domain.service.action using lowercase letters, digits, or hyphens.");
|
||||
StringBuilder message = new StringBuilder("Invalid MCP tool name(s): expected 1-128 characters using letters, digits, underscores, or hyphens.");
|
||||
for (Map.Entry<String, List<ToolDeclaration>> invalidName : invalidNames) {
|
||||
message.append("\n\n").append(invalidName.getKey());
|
||||
invalidName.getValue().stream()
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
package io.shinhanlife.dap.lib.config;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
import io.shinhanlife.glow.communication.module.http.component.GlowHttpComponent;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.context.annotation.Import;
|
||||
import org.springframework.web.client.RestClient;
|
||||
|
||||
class AxhubHttpConfigurationTest {
|
||||
|
||||
@Test
|
||||
void registersGlowHttpComponentFromDapLibConfiguration() {
|
||||
try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfiguration.class)) {
|
||||
assertThat(context.getBean(GlowHttpComponent.class)).isNotNull();
|
||||
}
|
||||
}
|
||||
|
||||
@Configuration
|
||||
@Import(AxhubHttpConfiguration.class)
|
||||
static class TestConfiguration {
|
||||
|
||||
@Bean
|
||||
RestClient.Builder restClientBuilder() {
|
||||
return RestClient.builder();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -18,10 +18,10 @@ class McpToolMethodRegistryTest {
|
||||
|
||||
registry.initialize();
|
||||
|
||||
McpToolMethodRegistry.RegisteredTool tool = registry.find("oth.cmm.echo.search");
|
||||
McpToolMethodRegistry.RegisteredTool tool = registry.find("oth_cmm_echo_search");
|
||||
assertNotNull(tool);
|
||||
assertEquals("execute", tool.method().getName());
|
||||
assertEquals("oth.cmm.echo.search", tool.annotation().name());
|
||||
assertEquals("oth_cmm_echo_search", tool.annotation().name());
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -42,14 +42,14 @@ class McpToolMethodRegistryTest {
|
||||
}
|
||||
|
||||
static class EchoTool {
|
||||
@McpTool(name = "oth.cmm.echo.search")
|
||||
@McpTool(name = "oth_cmm_echo_search")
|
||||
public String execute(String request) {
|
||||
return request;
|
||||
}
|
||||
}
|
||||
|
||||
static class DuplicateEchoTool {
|
||||
@McpTool(name = "oth.cmm.echo.search")
|
||||
@McpTool(name = "oth_cmm_echo_search")
|
||||
public String execute(String request) {
|
||||
return request;
|
||||
}
|
||||
|
||||
@@ -25,7 +25,7 @@ class ToolScaffolderTest {
|
||||
String useCase = Files.readString(root.resolve("usecase/ClaimSearchUseCase.java"));
|
||||
String response = Files.readString(root.resolve("dto/ClaimSearchResponse.java"));
|
||||
|
||||
assertTrue(useCase.contains("name = \"oth.cmm.claim.search\""));
|
||||
assertTrue(useCase.contains("name = \"oth_cmm_claim_search\""));
|
||||
assertTrue(useCase.contains("@ToolHint(register = true, categoryKey = \"cmm\", mappingId = \"CLM0001\")"));
|
||||
assertTrue(response.contains("private String resultCode;"));
|
||||
assertTrue(response.contains("private String resultMessage;"));
|
||||
@@ -41,7 +41,7 @@ class ToolScaffolderTest {
|
||||
"src/main/java/io/shinhanlife/dap/mcc/biz/cmm/usecase/NotificationSendUseCase.java");
|
||||
String useCase = Files.readString(useCasePath);
|
||||
|
||||
assertTrue(useCase.contains("name = \"sms.cmm.notification.send\""));
|
||||
assertTrue(useCase.contains("name = \"sms_cmm_notification_send\""));
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -121,7 +121,7 @@ class ToolScaffolderTest {
|
||||
ToolScaffolder.scaffold("claim search", "CLM0001", "Claim search", "cmm", "MCI", moduleName,
|
||||
"tester", "2026.08.10", true, null, null, null, List.of(), outputFields);
|
||||
|
||||
Path mockResponse = root.resolve("dap-was-oth/src/main/resources/mock-responses/oth.cmm.claim.search.json");
|
||||
Path mockResponse = root.resolve("dap-was-oth/src/main/resources/mock-responses/oth_cmm_claim_search.json");
|
||||
Path useCaseTest = root.resolve("dap-was-oth/src/test/java/io/shinhanlife/dap/mcc/biz/cmm/usecase/ClaimSearchUseCaseTest.java");
|
||||
|
||||
assertTrue(Files.exists(mockResponse));
|
||||
|
||||
@@ -57,13 +57,13 @@ class ToolSchemaResolverTest {
|
||||
}
|
||||
|
||||
static class AutomaticSchemaTool {
|
||||
@McpTool(name = "oth.test.automatic.search")
|
||||
@McpTool(name = "oth_test_automatic_search")
|
||||
void search(AutomaticRequest request) {
|
||||
}
|
||||
}
|
||||
|
||||
static class AutomaticOutputSchemaTool {
|
||||
@McpTool(name = "oth.test.output.search")
|
||||
@McpTool(name = "oth_test_output_search")
|
||||
SimpleResponse search(AutomaticRequest request) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -21,16 +21,16 @@ class ToolSourceUpdaterTest {
|
||||
import org.springaicommunity.mcp.annotation.McpTool;
|
||||
import io.shinhanlife.dap.lib.annotation.ToolHint;
|
||||
interface SampleUseCase {
|
||||
@McpTool(name = "oth.cmm.sample.search", description = "old")
|
||||
@McpTool(name = "oth_cmm_sample_search", description = "old")
|
||||
@ToolHint(register = false, requiresApproval = false)
|
||||
void search();
|
||||
}
|
||||
""");
|
||||
|
||||
ToolSourceUpdater.updateToolSource(temporaryRoot, "oth.cmm.sample.search", "customer", "new", true, true);
|
||||
ToolSourceUpdater.updateToolSource(temporaryRoot, "oth_cmm_sample_search", "customer", "new", true, true);
|
||||
|
||||
String updated = Files.readString(source);
|
||||
assertTrue(updated.contains("@McpTool(name = \"oth.cmm.sample.search\", description = \"new\")"));
|
||||
assertTrue(updated.contains("@McpTool(name = \"oth_cmm_sample_search\", description = \"new\")"));
|
||||
assertTrue(updated.contains("@ToolHint(register = true, requiresApproval = true"));
|
||||
assertTrue(updated.contains("categoryKey = \"customer\""), updated);
|
||||
}
|
||||
|
||||
@@ -31,35 +31,53 @@ class McpToolNameValidatorTest {
|
||||
|
||||
@Test
|
||||
void rejectsDuplicateMcpToolNamesAcrossToolModules() throws IOException {
|
||||
writeToolSource("dap-was-first", "FirstTool.java", "first", "oth.sms.notification.send");
|
||||
writeToolSource("dap-was-second", "SecondTool.java", "second", "oth.sms.notification.send");
|
||||
writeToolSource("dap-was-first", "FirstTool.java", "first", "oth_sms_notification_send");
|
||||
writeToolSource("dap-was-second", "SecondTool.java", "second", "oth_sms_notification_send");
|
||||
|
||||
IllegalStateException exception = assertThrows(IllegalStateException.class,
|
||||
() -> McpToolNameValidator.assertUnique(temporaryRoot));
|
||||
|
||||
assertTrue(exception.getMessage().contains("oth.sms.notification.send"));
|
||||
assertTrue(exception.getMessage().contains("oth_sms_notification_send"));
|
||||
assertTrue(exception.getMessage().contains("dap-was-first"));
|
||||
assertTrue(exception.getMessage().contains("dap-was-second"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void validationRunnerRejectsDuplicateMcpToolNamesBeforePackaging() throws IOException {
|
||||
writeToolSource("dap-was-first", "FirstTool.java", "first", "oth.sms.notification.send");
|
||||
writeToolSource("dap-was-second", "SecondTool.java", "second", "oth.sms.notification.send");
|
||||
writeToolSource("dap-was-first", "FirstTool.java", "first", "oth_sms_notification_send");
|
||||
writeToolSource("dap-was-second", "SecondTool.java", "second", "oth_sms_notification_send");
|
||||
|
||||
assertThrows(IllegalStateException.class,
|
||||
() -> McpToolNameValidationRunner.validate(temporaryRoot));
|
||||
}
|
||||
|
||||
@Test
|
||||
void rejectsToolNameOutsidePodDomainServiceActionConvention() throws IOException {
|
||||
writeToolSource("dap-was-first", "FirstTool.java", "first", "bond_issue");
|
||||
void rejectsToolNameOutsideConfiguredPattern() throws IOException {
|
||||
writeToolSource("dap-was-first", "FirstTool.java", "first", "bond.issue");
|
||||
|
||||
IllegalStateException exception = assertThrows(IllegalStateException.class,
|
||||
() -> McpToolNameValidator.assertUnique(temporaryRoot));
|
||||
|
||||
assertTrue(exception.getMessage().contains("Invalid MCP tool name(s)"));
|
||||
assertTrue(exception.getMessage().contains("bond_issue"));
|
||||
assertTrue(exception.getMessage().contains("bond.issue"));
|
||||
}
|
||||
@Test
|
||||
void acceptsLettersDigitsUnderscoresAndDashesWithin128Characters() throws IOException {
|
||||
String validName = "Tool_Name-" + "a".repeat(118);
|
||||
writeToolSource("dap-was-first", "FirstTool.java", "first", validName);
|
||||
|
||||
assertDoesNotThrow(() -> McpToolNameValidator.assertUnique(temporaryRoot));
|
||||
}
|
||||
|
||||
@Test
|
||||
void rejectsToolNameLongerThan128Characters() throws IOException {
|
||||
String invalidName = "a".repeat(129);
|
||||
writeToolSource("dap-was-first", "FirstTool.java", "first", invalidName);
|
||||
|
||||
IllegalStateException exception = assertThrows(IllegalStateException.class,
|
||||
() -> McpToolNameValidator.assertUnique(temporaryRoot));
|
||||
|
||||
assertTrue(exception.getMessage().contains(invalidName));
|
||||
}
|
||||
@Test
|
||||
void acceptsCurrentProjectToolNames() {
|
||||
@@ -71,11 +89,11 @@ class McpToolNameValidatorTest {
|
||||
Path root = findProjectRoot();
|
||||
|
||||
assertToolName(root, "dap-was-oth/src/main/java/io/shinhanlife/dap/mcc/biz/cmm/usecase/CustomerInfoUseCase.java",
|
||||
"oth.cmm.customer.detail", "detail");
|
||||
"oth_cmm_customer_detail", "detail");
|
||||
assertToolName(root, "dap-was-oth/src/main/java/io/shinhanlife/dap/mcc/biz/cmm/usecase/BillingProcessUseCase.java",
|
||||
"oth.cmm.billing.process", "process");
|
||||
"oth_cmm_billing_process", "process");
|
||||
assertToolName(root, "dap-was-oth/src/main/java/io/shinhanlife/dap/mcc/biz/cmm/usecase/BondIssueUseCase.java",
|
||||
"oth.cmm.bond.issue", "issue");
|
||||
"oth_cmm_bond_issue", "issue");
|
||||
}
|
||||
|
||||
private void assertToolName(Path root, String relativePath, String expectedName, String legacyName) throws IOException {
|
||||
|
||||
@@ -52,7 +52,7 @@ class ToolManifestServiceTest {
|
||||
void rejectsEntireManifestWhenToolNameDoesNotMatchConfiguredPrefix() {
|
||||
McpProperties properties = manifestProperties("insurance-processing", "processing.");
|
||||
ToolManifestService service = new ToolManifestService(
|
||||
() -> List.of(tool("notification.sms.send", "1.0.0", 3000)), objectMapper, properties);
|
||||
() -> List.of(tool("notification_sms_send", "1.0.0", 3000)), objectMapper, properties);
|
||||
|
||||
IllegalStateException error = assertThrows(IllegalStateException.class, service::currentManifest);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user