fix(skill): give CJK-only MCP/ACP names a stable id-based slug

This commit is contained in:
matevip 2026-05-14 18:18:15 +08:00
parent 729ac3bae7
commit 139a478bcd
3 changed files with 83 additions and 8 deletions

View File

@ -212,7 +212,7 @@ public class AcpSkillBridge {
private void registerWrappers(AcpEndpointEntity ep) { private void registerWrappers(AcpEndpointEntity ep) {
if (ep == null || !Boolean.TRUE.equals(ep.getEnabled())) return; if (ep == null || !Boolean.TRUE.equals(ep.getEnabled())) return;
String slug = slugify(ep.getName()); String slug = slugForEndpoint(ep);
if (slug.isEmpty()) { if (slug.isEmpty()) {
log.warn("ACP endpoint id={} has blank name; cannot register wrapper", ep.getId()); log.warn("ACP endpoint id={} has blank name; cannot register wrapper", ep.getId());
return; return;
@ -289,7 +289,7 @@ public class AcpSkillBridge {
private SkillEntity endpointToEntity(AcpEndpointEntity ep) { private SkillEntity endpointToEntity(AcpEndpointEntity ep) {
SkillEntity s = new SkillEntity(); SkillEntity s = new SkillEntity();
s.setId(virtualIdFor(ep)); s.setId(virtualIdFor(ep));
s.setName(slugify(ep.getName())); s.setName(slugForEndpoint(ep));
s.setNameEn(displayName(ep)); s.setNameEn(displayName(ep));
s.setNameZh(ep.getDescription() != null && !ep.getDescription().isBlank() s.setNameZh(ep.getDescription() != null && !ep.getDescription().isBlank()
? displayName(ep) : null); ? displayName(ep) : null);
@ -342,7 +342,7 @@ public class AcpSkillBridge {
return ResolvedSkill.builder() return ResolvedSkill.builder()
.id(virtualIdFor(ep)) .id(virtualIdFor(ep))
.name(slugify(ep.getName())) .name(slugForEndpoint(ep))
.description(buildDescription(ep)) .description(buildDescription(ep))
.content("") // no SKILL.md .content("") // no SKILL.md
.source("acp") .source("acp")
@ -374,7 +374,7 @@ public class AcpSkillBridge {
* it up the same way as a hand-authored skill manifest. * it up the same way as a hand-authored skill manifest.
*/ */
private SkillManifest buildManifest(AcpEndpointEntity ep) { private SkillManifest buildManifest(AcpEndpointEntity ep) {
String slug = slugify(ep.getName()); String slug = slugForEndpoint(ep);
String toolName = "acp_" + slug + "_prompt"; String toolName = "acp_" + slug + "_prompt";
List<String> tools = List.of(toolName); List<String> tools = List.of(toolName);
@ -447,6 +447,28 @@ public class AcpSkillBridge {
return raw.toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9_-]", "-"); return raw.toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9_-]", "-");
} }
/**
* Stable slug for an ACP endpoint. Falls back to {@code acp-{id}} when
* the source name has no ASCII letter/digit (e.g. pure CJK), because
* the naive slugify would otherwise return a run of dashes and two
* differently-named all-CJK endpoints would collide on the same slug,
* which is also the basis for the {@code acp_<slug>_prompt} wrapper
* tool name registered in the global tool registry.
*/
private String slugForEndpoint(AcpEndpointEntity ep) {
String slug = slugify(ep.getName());
return hasAsciiAlphaNumeric(slug) ? slug : "acp-" + ep.getId();
}
private static boolean hasAsciiAlphaNumeric(String s) {
if (s == null || s.isEmpty()) return false;
for (int i = 0; i < s.length(); i++) {
char c = s.charAt(i);
if ((c >= 'a' && c <= 'z') || (c >= '0' && c <= '9')) return true;
}
return false;
}
private String displayName(AcpEndpointEntity ep) { private String displayName(AcpEndpointEntity ep) {
if (ep.getDisplayName() != null && !ep.getDisplayName().isBlank()) return ep.getDisplayName(); if (ep.getDisplayName() != null && !ep.getDisplayName().isBlank()) return ep.getDisplayName();
return ep.getName() != null ? ep.getName() : "acp-" + ep.getId(); return ep.getName() != null ? ep.getName() : "acp-" + ep.getId();

View File

@ -133,7 +133,7 @@ public class McpSkillBridge {
private SkillEntity serverToEntity(McpServerEntity server) { private SkillEntity serverToEntity(McpServerEntity server) {
SkillEntity s = new SkillEntity(); SkillEntity s = new SkillEntity();
s.setId(virtualIdFor(server)); s.setId(virtualIdFor(server));
s.setName(slugify(server.getName())); s.setName(slugForServer(server));
s.setNameEn(displayName(server)); s.setNameEn(displayName(server));
s.setNameZh(displayName(server)); s.setNameZh(displayName(server));
s.setDescription(buildDescription(server)); s.setDescription(buildDescription(server));
@ -175,7 +175,7 @@ public class McpSkillBridge {
return ResolvedSkill.builder() return ResolvedSkill.builder()
.id(virtualIdFor(server)) .id(virtualIdFor(server))
.name(slugify(server.getName())) .name(slugForServer(server))
.description(buildDescription(server)) .description(buildDescription(server))
.content("") // no SKILL.md .content("") // no SKILL.md
.source("mcp") .source("mcp")
@ -242,9 +242,10 @@ public class McpSkillBridge {
.description("MCP server '" + server.getName() + "' must be connected. Configure in Settings ▸ MCP Connections.") .description("MCP server '" + server.getName() + "' must be connected. Configure in Settings ▸ MCP Connections.")
.build(); .build();
String slug = slugForServer(server);
return SkillManifest.builder() return SkillManifest.builder()
.id(slugify(server.getName())) .id(slug)
.name(slugify(server.getName())) .name(slug)
.description(buildDescription(server)) .description(buildDescription(server))
.icon(iconFor(server)) .icon(iconFor(server))
.version("1.0.0") .version("1.0.0")
@ -322,6 +323,27 @@ public class McpSkillBridge {
return raw.toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9_-]", "-"); return raw.toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9_-]", "-");
} }
/**
* Stable slug for an MCP server. Falls back to {@code mcp-{id}} when
* the source name has no ASCII letter/digit (e.g. pure CJK), because
* the naive slugify would otherwise return a run of dashes making
* two differently-named all-CJK servers collide on the same display
* key and breaking name-based skill lookup.
*/
private String slugForServer(McpServerEntity server) {
String slug = slugify(server.getName());
return hasAsciiAlphaNumeric(slug) ? slug : "mcp-" + server.getId();
}
private static boolean hasAsciiAlphaNumeric(String s) {
if (s == null || s.isEmpty()) return false;
for (int i = 0; i < s.length(); i++) {
char c = s.charAt(i);
if ((c >= 'a' && c <= 'z') || (c >= '0' && c <= '9')) return true;
}
return false;
}
private String displayName(McpServerEntity server) { private String displayName(McpServerEntity server) {
return server.getName() != null ? server.getName() : "mcp-" + server.getId(); return server.getName() != null ? server.getName() : "mcp-" + server.getId();
} }

View File

@ -108,6 +108,37 @@ class McpSkillBridgeManifestTest {
"no prefixed tool name expected, got: " + entity.getManifestJson()); "no prefixed tool name expected, got: " + entity.getManifestJson());
} }
@Test
@DisplayName("CJK-only server names slug to a stable id-based fallback instead of an all-dash collision")
void cjkOnlyNameFallsBackToIdSlug() {
McpServerEntity a = newServer(42L, "知识图谱对象数据查询服务");
McpServerEntity b = newServer(43L, "客户档案信息查询服务");
a.setToolsCacheJson(toolsJson("search"));
b.setToolsCacheJson(toolsJson("search"));
when(mcpServerService.listEnabled()).thenReturn(List.of(a, b));
List<SkillEntity> entities = bridge.listMcpDerivedSkillEntities();
assertEquals("mcp-42", entities.get(0).getName(),
"all-CJK name should fall back to id-based slug");
assertEquals("mcp-43", entities.get(1).getName(),
"second all-CJK name must not collide with the first");
assertTrue(entities.get(0).getManifestJson().contains("\"id\":\"mcp-42\""),
"manifest id should mirror the fallback slug, got: " + entities.get(0).getManifestJson());
}
@Test
@DisplayName("ASCII server names keep their existing slug — no regression for English names")
void asciiNamePreservesExistingSlug() {
McpServerEntity server = newServer(42L, "GitHub");
server.setToolsCacheJson(toolsJson("create_issue"));
when(mcpServerService.listEnabled()).thenReturn(List.of(server));
SkillEntity entity = bridge.listMcpDerivedSkillEntities().get(0);
assertEquals("github", entity.getName());
}
@Test @Test
@DisplayName("two servers exposing the same raw tool name produce distinct prefixed names") @DisplayName("two servers exposing the same raw tool name produce distinct prefixed names")
void twoServersSameRawNameDistinct() { void twoServersSameRawNameDistinct() {