From 130dfd52781623bbd0ed1b16cba3ba9769547fdf Mon Sep 17 00:00:00 2001 From: matevip Date: Sat, 16 May 2026 14:50:46 +0800 Subject: [PATCH] fix(skill): synthesize SKILL.md for MCP-derived virtual skills (#136) --- .../skill/controller/SkillController.java | 26 ++- .../vip/mate/skill/mcp/McpSkillBridge.java | 167 +++++++++++++--- .../SkillControllerListEnabledTest.java | 16 ++ .../SkillControllerVirtualGuardTest.java | 44 ++++- .../skill/mcp/McpSkillBridgeContentTest.java | 184 ++++++++++++++++++ .../skill/mcp/McpSkillBridgeManifestTest.java | 14 +- mateclaw-ui/src/views/SkillMarket.vue | 43 ++-- 7 files changed, 430 insertions(+), 64 deletions(-) create mode 100644 mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeContentTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java b/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java index 3a86bdb5..fc984d9c 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java @@ -145,6 +145,14 @@ public class SkillController { return filterShadowedVirtualSkills(virtualSkills, realSkillNames).size(); } + /** Keep only enabled rows — gates virtual skills into enabled-only endpoints. */ + static List enabledOnly(List skills) { + if (skills == null || skills.isEmpty()) return List.of(); + return skills.stream() + .filter(s -> s != null && Boolean.TRUE.equals(s.getEnabled())) + .toList(); + } + /** * Keep MyBatis-Plus as the source of truth for DB pagination and append * live virtual ACP/MCP rows after the DB rows. This produces one stable @@ -361,18 +369,21 @@ public class SkillController { // include all real skill names — including disabled ones — so a // disabled real skill correctly suppresses its same-named virtual // twin, matching /skills and /counts. + // The bridges surface disabled MCP/ACP servers too (so the Skills + // page can show a toggled-off card); this endpoint is enabled-only, + // so the virtual rows are filtered to enabled before merging. List result = new ArrayList<>(skillService.listEnabledSkills(workspaceId)); Set realNames = realSkillNames(workspaceId); try { - result.addAll(filterShadowedVirtualSkills( - mcpSkillBridge.listMcpDerivedSkillEntities(), realNames)); + result.addAll(enabledOnly(filterShadowedVirtualSkills( + mcpSkillBridge.listMcpDerivedSkillEntities(), realNames))); } catch (Exception e) { // Bridge failure must not 500 the picker — same defensive stance as /counts. } try { - result.addAll(filterShadowedVirtualSkills( - acpSkillBridge.listAcpDerivedSkillEntities(), realNames)); + result.addAll(enabledOnly(filterShadowedVirtualSkills( + acpSkillBridge.listAcpDerivedSkillEntities(), realNames))); } catch (Exception e) { // Bridge failure must not 500 the picker — same defensive stance as /counts. } @@ -469,6 +480,13 @@ public class SkillController { @RequireWorkspaceRole("admin") public R toggle(@PathVariable Long id, @RequestParam boolean enabled, @RequestHeader(value = "X-Workspace-Id", required = false) Long workspaceId) { + // A virtual MCP skill mirrors an MCP server — toggling it enables / + // disables that server, keeping the Skills page and Settings ▸ MCP + // Connections in sync. ACP virtual skills have no such mapping and + // stay read-only via rejectVirtualSkillMutation below. + if (vip.mate.skill.mcp.McpSkillBridge.isVirtualMcpSkillId(id)) { + return R.ok(mcpSkillBridge.toggleVirtualSkill(id, enabled)); + } rejectVirtualSkillMutation(id); verifyResourceWorkspace(skillService.getSkill(id), workspaceId); return R.ok(skillService.toggleSkill(id, enabled)); diff --git a/mateclaw-server/src/main/java/vip/mate/skill/mcp/McpSkillBridge.java b/mateclaw-server/src/main/java/vip/mate/skill/mcp/McpSkillBridge.java index 6f91dca1..5e383232 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/mcp/McpSkillBridge.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/mcp/McpSkillBridge.java @@ -112,23 +112,45 @@ public class McpSkillBridge { } /** - * Snapshot every enabled MCP server as a virtual {@link SkillEntity}. - * Used by the Skills list endpoint; rows are non-persistent and - * regenerated on each call. + * Snapshot every MCP server as a virtual {@link SkillEntity}. Used by + * the Skills list endpoint; rows are non-persistent and regenerated on + * each call. Disabled servers are included so a skill the user toggled + * off still shows on the Skills page (as a disabled card) and can be + * toggled back on — {@code enabled} mirrors the server's flag. */ public List listMcpDerivedSkillEntities() { - return listEnabledServers().stream().map(this::serverToEntity).toList(); + return listAllServers().stream().map(this::serverToEntity).toList(); } /** - * Snapshot every enabled MCP server as a virtual {@link ResolvedSkill} - * with synthesized manifest, ready to be merged into the runtime - * status feed. Status reflects connection health: OK → READY default - * feature; ERROR / disconnected → SETUP_NEEDED with a diagnostic - * missing-dependency entry. + * Snapshot every MCP server as a virtual {@link ResolvedSkill} with + * synthesized manifest, ready to be merged into the runtime status + * feed. Status reflects connection health: OK → READY default feature; + * ERROR / disconnected → SETUP_NEEDED with a diagnostic missing-dependency + * entry. Disabled servers are included for the admin status view; the + * active-skill gate ({@code SkillRuntimeService.passesActiveGate}) keeps + * them out of the agent runtime. */ public List listMcpDerivedResolvedSkills() { - return listEnabledServers().stream().map(this::serverToResolved).toList(); + return listAllServers().stream().map(this::serverToResolved).toList(); + } + + /** + * Enable or disable the MCP server behind a virtual MCP skill. + * + *

A virtual MCP skill has no {@code mate_skill} row — its enabled + * state is the underlying MCP server's {@code enabled} flag. Toggling + * the skill therefore toggles the server, which also connects or + * disconnects it. Returns the rebuilt virtual {@link SkillEntity} + * reflecting the new state. + */ + public SkillEntity toggleVirtualSkill(Long virtualId, boolean enabled) { + Long serverId = extractMcpServerId(virtualId); + if (serverId == null) { + throw new IllegalArgumentException("Not a virtual MCP skill id: " + virtualId); + } + McpServerEntity updated = mcpServerService.toggle(serverId, enabled); + return serverToEntity(updated); } /** @@ -147,16 +169,17 @@ public class McpSkillBridge { } } - private List listEnabledServers() { + private List listAllServers() { try { - return mcpServerService.listEnabled(); + return mcpServerService.listAll(); } catch (Exception e) { - log.warn("MCP bridge could not list enabled servers: {}", e.getMessage()); + log.warn("MCP bridge could not list servers: {}", e.getMessage()); return List.of(); } } private SkillEntity serverToEntity(McpServerEntity server) { + List tools = readToolDescriptors(server); SkillEntity s = new SkillEntity(); s.setId(virtualIdFor(server)); s.setName(slugForServer(server)); @@ -171,13 +194,15 @@ public class McpSkillBridge { s.setBuiltin(false); s.setTags("mcp"); s.setSecurityScanStatus("PASSED"); // MCP servers don't go through SkillSecurityService + s.setSkillContent(buildSkillContent(server, tools)); s.setConfigJson(buildConfigJson(server)); - s.setManifestJson(serializeManifest(buildManifestFrom(server, readToolRawNames(server)))); + s.setManifestJson(serializeManifest(buildManifestFrom(server, toRawNames(tools)))); return s; } private ResolvedSkill serverToResolved(McpServerEntity server) { - List rawNames = readToolRawNames(server); + List tools = readToolDescriptors(server); + List rawNames = toRawNames(tools); Map toolDisplayNames = new LinkedHashMap<>(); for (String raw : rawNames) { String prefixed = McpToolNameResolver.prefixedName(server.getId(), raw); @@ -203,7 +228,7 @@ public class McpSkillBridge { .id(virtualIdFor(server)) .name(slugForServer(server)) .description(buildDescription(server)) - .content("") // no SKILL.md + .content(buildSkillContent(server, tools)) .source("mcp") .skillDir(null) .configuredSkillDir(null) @@ -293,24 +318,27 @@ public class McpSkillBridge { } /** - * Resolve the raw tool name list for a server with cache-first / live-fallback - * semantics. Returns an empty list (never null) so the manifest builder - * stays simple. + * Resolve the tool list for a server with cache-first / live-fallback + * semantics. Each entry carries the raw name and (when available) the + * upstream description. Returns an empty list (never null) so callers + * stay simple. */ - private List readToolRawNames(McpServerEntity server) { - List fromCache = parseCachedToolNames(server.getToolsCacheJson()); + private List readToolDescriptors(McpServerEntity server) { + List fromCache = parseCachedToolDescriptors(server.getToolsCacheJson()); if (!fromCache.isEmpty()) { return fromCache; } try { List discovered = mcpClientManager.getServerTools(server.getId()); - List names = new ArrayList<>(discovered.size()); + List out = new ArrayList<>(discovered.size()); for (McpSchema.Tool t : discovered) { if (t == null) continue; String n = t.name(); - if (n != null && !n.isBlank()) names.add(n); + if (n != null && !n.isBlank()) { + out.add(new McpToolDescriptor(n, t.description())); + } } - return names; + return out; } catch (Exception e) { log.debug("MCP bridge manifest build: getServerTools({}) failed: {}", server.getId(), e.getMessage()); @@ -320,22 +348,25 @@ public class McpSkillBridge { /** * Parse the {@code tools_cache_json} column written by - * {@code McpServerService} after each successful connect. Returns an - * empty list if the column is null/blank/malformed — the bridge is - * required to keep working when the cache hasn't been populated yet - * (e.g. first-ever connect just succeeded a moment ago). + * {@code McpServerService} after each successful connect — an array of + * {@code {name, description, inputSchema}} entries. Returns an empty + * list if the column is null/blank/malformed — the bridge is required + * to keep working when the cache hasn't been populated yet (e.g. + * first-ever connect just succeeded a moment ago). */ - private List parseCachedToolNames(String json) { + private List parseCachedToolDescriptors(String json) { if (json == null || json.isBlank()) { return List.of(); } try { cn.hutool.json.JSONArray arr = cn.hutool.json.JSONUtil.parseArray(json); - List out = new ArrayList<>(arr.size()); + List out = new ArrayList<>(arr.size()); for (Object obj : arr) { if (!(obj instanceof cn.hutool.json.JSONObject jo)) continue; String name = jo.getStr("name"); - if (name != null && !name.isBlank()) out.add(name); + if (name != null && !name.isBlank()) { + out.add(new McpToolDescriptor(name, jo.getStr("description"))); + } } return out; } catch (Exception e) { @@ -344,6 +375,80 @@ public class McpSkillBridge { } } + private static List toRawNames(List tools) { + List names = new ArrayList<>(tools.size()); + for (McpToolDescriptor t : tools) { + names.add(t.name()); + } + return names; + } + + /** + * Synthesize a SKILL.md body for an MCP-derived virtual skill. + * + *

Persisted and uploaded skills ship a hand-written SKILL.md that the + * agent serves on demand through {@code readSkillFile}; it tells the + * model what the skill is for and how to drive it. An MCP-derived skill + * has no such file — the upstream server only exposes a tool list — so + * without a synthesized body {@code readSkillFile} returns "content not + * available" and the model has nothing beyond the one-line description + * to reason about. + * + *

This builds an equivalent body from the live tool snapshot: a + * one-line summary, the tool catalog with per-tool descriptions, and a + * short usage note. Regenerated on every list call, so it tracks the + * upstream tool set with no persistence step. + */ + private String buildSkillContent(McpServerEntity server, List tools) { + String displayName = displayName(server); + StringBuilder md = new StringBuilder(); + md.append("# ").append(displayName).append("\n\n"); + md.append(buildDescription(server)).append("\n\n"); + md.append("This capability is provided by the MCP server **").append(displayName).append("**"); + String transport = nullSafe(server.getTransport()); + if (!transport.isBlank()) { + md.append(" (").append(transport).append(" transport)"); + } + md.append(". Its tools are available to you as ordinary function calls — ") + .append("invoke them directly by name; no shell or scripts are involved.\n\n"); + + md.append("## Available Tools\n\n"); + if (tools.isEmpty()) { + md.append("The tool list is not available yet. The MCP server may be ") + .append("disconnected or still starting up — check its status in ") + .append("Settings ▸ MCP Connections.\n"); + return md.toString(); + } + md.append("This server exposes ").append(tools.size()) + .append(tools.size() == 1 ? " tool:\n\n" : " tools:\n\n"); + for (McpToolDescriptor t : tools) { + md.append("- **").append(t.name()).append("**"); + String desc = oneLine(t.description()); + if (!desc.isBlank()) { + md.append(" — ").append(desc); + } + md.append("\n"); + } + md.append("\n## Usage Notes\n\n"); + md.append("- These tools appear in your tool list under `mcp_`-prefixed names; ") + .append("pick whichever one matches the user's request.\n"); + md.append("- If a call fails with a connection error, the MCP server is likely ") + .append("disconnected — it can be reconnected in Settings ▸ MCP Connections.\n"); + return md.toString(); + } + + /** Collapse whitespace and clamp a tool description to a prompt-friendly length. */ + private static String oneLine(String s) { + if (s == null) { + return ""; + } + String collapsed = s.replaceAll("\\s+", " ").trim(); + return collapsed.length() > 200 ? collapsed.substring(0, 200) + "…" : collapsed; + } + + /** Minimal MCP tool projection: just what the manifest and SKILL.md body need. */ + private record McpToolDescriptor(String name, String description) {} + private String slugify(String raw) { if (raw == null) return ""; return raw.toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9_-]", "-"); diff --git a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerListEnabledTest.java b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerListEnabledTest.java index 6084ee65..d253f875 100644 --- a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerListEnabledTest.java +++ b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerListEnabledTest.java @@ -135,6 +135,22 @@ class SkillControllerListEnabledTest { assertTrue(response.getData().stream().anyMatch(s -> "github".equals(s.getName()))); } + @Test + @DisplayName("listEnabled excludes a disabled MCP virtual skill") + void excludesDisabledVirtualMcpSkill() { + // The bridge now surfaces disabled MCP servers too (so the Skills + // page can show a toggled-off card); the enabled-only picker must + // filter them back out. + SkillEntity disabledMcp = skill("github", "mcp"); + disabledMcp.setEnabled(false); + when(mcpSkillBridge.listMcpDerivedSkillEntities()).thenReturn(List.of(disabledMcp)); + + R> response = controller.listEnabled(null); + + assertEquals(0, response.getData().size(), + "a disabled MCP virtual skill must not appear in the enabled-only picker"); + } + private static SkillEntity skill(String name, String type) { SkillEntity s = new SkillEntity(); s.setName(name); diff --git a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerVirtualGuardTest.java b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerVirtualGuardTest.java index 435a2526..a9f267ac 100644 --- a/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerVirtualGuardTest.java +++ b/mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerVirtualGuardTest.java @@ -2,19 +2,25 @@ package vip.mate.skill.controller; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import vip.mate.common.result.R; import vip.mate.exception.MateClawException; import vip.mate.skill.acp.AcpSkillBridge; import vip.mate.skill.mcp.McpSkillBridge; import vip.mate.skill.model.SkillEntity; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** - * Mutation paths refuse virtual MCP/ACP skill ids upfront so the user - * gets a clear redirect to the connection page instead of the previous - * "技能不存在" 500 surfacing from a doomed mate_skill lookup. + * The edit / delete mutation paths refuse virtual MCP/ACP skill ids upfront + * so the user gets a clear redirect to the connection page instead of the + * old "技能不存在" 500 from a doomed mate_skill lookup. Toggle is the one + * exception: a virtual MCP skill mirrors an MCP server, so toggling it + * forwards to that server's enable/disable. */ class SkillControllerVirtualGuardTest { @@ -45,14 +51,42 @@ class SkillControllerVirtualGuardTest { } @Test - @DisplayName("delete / toggle / rescan all reject virtual ids the same way") + @DisplayName("delete / rescan still reject virtual ids the same way") void mutationFamilyAllGuarded() { long virtualId = McpSkillBridge.VIRTUAL_ID_BASE + 42L; assertThrows(MateClawException.class, () -> controller.delete(virtualId, null)); - assertThrows(MateClawException.class, () -> controller.toggle(virtualId, true, null)); assertThrows(MateClawException.class, () -> controller.rescan(virtualId, null)); } + @Test + @DisplayName("toggle on a virtual MCP skill forwards to the bridge instead of rejecting") + void toggleForwardsVirtualMcpToBridge() { + McpSkillBridge bridge = mock(McpSkillBridge.class); + SkillController c = new SkillController( + null, null, null, null, null, null, null, null, null, null, null, + bridge, null); + long virtualMcpId = McpSkillBridge.VIRTUAL_ID_BASE + 42L; + SkillEntity toggled = new SkillEntity(); + toggled.setName("github"); + toggled.setEnabled(false); + when(bridge.toggleVirtualSkill(virtualMcpId, false)).thenReturn(toggled); + + R resp = c.toggle(virtualMcpId, false, null); + + verify(bridge).toggleVirtualSkill(virtualMcpId, false); + assertEquals("github", resp.getData().getName()); + } + + @Test + @DisplayName("toggle on a virtual ACP skill is still rejected — no MCP-server mapping") + void toggleRejectsVirtualAcp() { + long virtualAcpId = AcpSkillBridge.VIRTUAL_ID_BASE + 7L; + assertTrue(AcpSkillBridge.isVirtualAcpSkillId(virtualAcpId), + "test fixture id is not in ACP virtual range; ACP base layout changed?"); + assertThrows(MateClawException.class, + () -> controller.toggle(virtualAcpId, true, null)); + } + @Test @DisplayName("real skill ids fall through to the service (no false-positive guard)") void realIdNotGuarded() { diff --git a/mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeContentTest.java b/mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeContentTest.java new file mode 100644 index 00000000..368ab14c --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeContentTest.java @@ -0,0 +1,184 @@ +package vip.mate.skill.mcp; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import vip.mate.skill.model.SkillEntity; +import vip.mate.skill.runtime.model.ResolvedSkill; +import vip.mate.tool.mcp.model.McpServerEntity; +import vip.mate.tool.mcp.runtime.McpClientManager; +import vip.mate.tool.mcp.service.McpServerService; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Asserts that MCP-derived virtual skills carry a synthesized SKILL.md body + * instead of the empty string. Without it, {@code readSkillFile("SKILL.md")} + * returns "content not available" and the agent cannot reason about the + * MCP server's tools. + */ +class McpSkillBridgeContentTest { + + private McpServerService mcpServerService; + private McpClientManager mcpClientManager; + private McpSkillBridge bridge; + + @BeforeEach + void setUp() { + mcpServerService = mock(McpServerService.class); + mcpClientManager = mock(McpClientManager.class); + bridge = new McpSkillBridge(mcpServerService, mcpClientManager, new ObjectMapper()); + } + + @Test + @DisplayName("resolved skill carries a non-empty SKILL.md listing the server's tools") + void resolvedSkillHasSynthesizedContent() { + McpServerEntity server = newServer(42L, "github"); + server.setToolsCacheJson(toolsJson( + "create_issue", "Open a new issue in a repository", + "list_issues", "List issues filtered by state and labels")); + when(mcpServerService.listAll()).thenReturn(List.of(server)); + + ResolvedSkill resolved = bridge.listMcpDerivedResolvedSkills().get(0); + + String content = resolved.getContent(); + assertFalse(content == null || content.isBlank(), "SKILL.md content must not be empty"); + assertTrue(content.contains("github"), "content should name the MCP server"); + assertTrue(content.contains("create_issue"), "content should list the create_issue tool"); + assertTrue(content.contains("list_issues"), "content should list the list_issues tool"); + assertTrue(content.contains("Open a new issue in a repository"), + "content should carry the upstream tool description"); + } + + @Test + @DisplayName("virtual SkillEntity carries skillContent so the detail drawer can render it") + void entityHasSynthesizedSkillContent() { + McpServerEntity server = newServer(42L, "github"); + server.setToolsCacheJson(toolsJson("create_issue", "Open a new issue")); + when(mcpServerService.listAll()).thenReturn(List.of(server)); + + SkillEntity entity = bridge.listMcpDerivedSkillEntities().get(0); + + assertFalse(entity.getSkillContent() == null || entity.getSkillContent().isBlank(), + "skillContent must be populated for MCP-derived skills"); + assertTrue(entity.getSkillContent().contains("create_issue")); + } + + @Test + @DisplayName("a server with no known tools still gets a content body that explains the gap") + void emptyToolListStillProducesContent() { + McpServerEntity server = newServer(42L, "github"); + server.setToolsCacheJson(""); + server.setLastStatus("disconnected"); + when(mcpServerService.listAll()).thenReturn(List.of(server)); + when(mcpClientManager.getServerTools(42L)).thenReturn(List.of()); + + ResolvedSkill resolved = bridge.listMcpDerivedResolvedSkills().get(0); + + String content = resolved.getContent(); + assertFalse(content == null || content.isBlank(), + "content must not be empty even when the tool list is unavailable"); + assertTrue(content.contains("MCP Connections"), + "content should point the user at the MCP Connections page"); + } + + @Test + @DisplayName("tool descriptions are clamped to a single prompt-friendly line") + void longDescriptionsAreClampedToOneLine() { + McpServerEntity server = newServer(42L, "github"); + String longDesc = "x".repeat(400); + server.setToolsCacheJson(toolsJson("create_issue", longDesc)); + when(mcpServerService.listAll()).thenReturn(List.of(server)); + + ResolvedSkill resolved = bridge.listMcpDerivedResolvedSkills().get(0); + + String toolLine = resolved.getContent().lines() + .filter(l -> l.startsWith("- **create_issue**")) + .findFirst() + .orElseThrow(); + assertTrue(toolLine.length() < longDesc.length(), + "an over-long description should be truncated, got: " + toolLine.length()); + assertTrue(toolLine.endsWith("…"), "truncated descriptions should end with an ellipsis"); + } + + private static McpServerEntity newServer(long id, String name) { + McpServerEntity s = new McpServerEntity(); + s.setId(id); + s.setName(name); + s.setEnabled(true); + s.setTransport("stdio"); + s.setCommand("/usr/bin/echo"); + s.setLastStatus("connected"); + return s; + } + + /** Builds a tools_cache_json array from alternating name/description pairs. */ + private static String toolsJson(String... nameDescPairs) { + StringBuilder sb = new StringBuilder("["); + for (int i = 0; i + 1 < nameDescPairs.length; i += 2) { + if (i > 0) { + sb.append(","); + } + sb.append("{\"name\":\"").append(nameDescPairs[i]) + .append("\",\"description\":\"").append(nameDescPairs[i + 1]) + .append("\",\"inputSchema\":{}}"); + } + sb.append("]"); + return sb.toString(); + } + + @Test + @DisplayName("two name/description pairs round-trip into the catalog") + void multipleToolsRenderAsCatalogRows() { + McpServerEntity server = newServer(7L, "filesystem"); + server.setToolsCacheJson(toolsJson( + "read_file", "Read the contents of a file", + "write_file", "Write content to a file")); + when(mcpServerService.listAll()).thenReturn(List.of(server)); + + String content = bridge.listMcpDerivedResolvedSkills().get(0).getContent(); + + long rows = content.lines().filter(l -> l.startsWith("- **")).count(); + assertEquals(2, rows, "each MCP tool should be one catalog row"); + } + + @Test + @DisplayName("toggleVirtualSkill forwards to mcpServerService.toggle and reflects the new state") + void toggleVirtualSkillForwardsToServer() { + McpServerEntity disabled = newServer(42L, "github"); + disabled.setEnabled(false); + long virtualId = McpSkillBridge.virtualIdFor(disabled); + + McpServerEntity enabledAfter = newServer(42L, "github"); + enabledAfter.setEnabled(true); + when(mcpServerService.toggle(42L, true)).thenReturn(enabledAfter); + + SkillEntity result = bridge.toggleVirtualSkill(virtualId, true); + + verify(mcpServerService).toggle(42L, true); + assertEquals("github", result.getName()); + assertEquals(Boolean.TRUE, result.getEnabled()); + } + + @Test + @DisplayName("a disabled MCP server still surfaces as a (disabled) virtual skill row") + void disabledServerStillListedAsDisabledSkill() { + McpServerEntity server = newServer(42L, "github"); + server.setEnabled(false); + when(mcpServerService.listAll()).thenReturn(List.of(server)); + + List entities = bridge.listMcpDerivedSkillEntities(); + + assertEquals(1, entities.size(), + "disabled MCP servers must still appear so the toggle can be flipped back on"); + assertEquals(Boolean.FALSE, entities.get(0).getEnabled()); + } +} diff --git a/mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeManifestTest.java b/mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeManifestTest.java index 8cf26848..f4f0ea69 100644 --- a/mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeManifestTest.java +++ b/mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeManifestTest.java @@ -47,7 +47,7 @@ class McpSkillBridgeManifestTest { void allowedToolsArePrefixed() { McpServerEntity server = newServer(42L, "github"); server.setToolsCacheJson(toolsJson("create_issue", "list_issues")); - when(mcpServerService.listEnabled()).thenReturn(List.of(server)); + when(mcpServerService.listAll()).thenReturn(List.of(server)); SkillEntity entity = bridge.listMcpDerivedSkillEntities().get(0); @@ -65,7 +65,7 @@ class McpSkillBridgeManifestTest { void readsFromCacheFirst() { McpServerEntity server = newServer(42L, "github"); server.setToolsCacheJson(toolsJson("create_issue")); - when(mcpServerService.listEnabled()).thenReturn(List.of(server)); + when(mcpServerService.listAll()).thenReturn(List.of(server)); bridge.listMcpDerivedSkillEntities(); @@ -77,7 +77,7 @@ class McpSkillBridgeManifestTest { void fallsBackToLiveWhenCacheMissing() { McpServerEntity server = newServer(42L, "github"); server.setToolsCacheJson(null); // first-ever connect just happened, cache not yet written - when(mcpServerService.listEnabled()).thenReturn(List.of(server)); + when(mcpServerService.listAll()).thenReturn(List.of(server)); when(mcpClientManager.getServerTools(42L)).thenReturn(List.of( fakeTool("create_issue"), fakeTool("list_issues"))); @@ -94,7 +94,7 @@ class McpSkillBridgeManifestTest { McpServerEntity server = newServer(42L, "github"); server.setToolsCacheJson(""); server.setLastStatus("disconnected"); - when(mcpServerService.listEnabled()).thenReturn(List.of(server)); + when(mcpServerService.listAll()).thenReturn(List.of(server)); when(mcpClientManager.getServerTools(42L)).thenReturn(List.of()); SkillEntity entity = bridge.listMcpDerivedSkillEntities().get(0); @@ -150,7 +150,7 @@ class McpSkillBridgeManifestTest { McpServerEntity b = newServer(43L, "客户档案信息查询服务"); a.setToolsCacheJson(toolsJson("search")); b.setToolsCacheJson(toolsJson("search")); - when(mcpServerService.listEnabled()).thenReturn(List.of(a, b)); + when(mcpServerService.listAll()).thenReturn(List.of(a, b)); List entities = bridge.listMcpDerivedSkillEntities(); @@ -167,7 +167,7 @@ class McpSkillBridgeManifestTest { void asciiNamePreservesExistingSlug() { McpServerEntity server = newServer(42L, "GitHub"); server.setToolsCacheJson(toolsJson("create_issue")); - when(mcpServerService.listEnabled()).thenReturn(List.of(server)); + when(mcpServerService.listAll()).thenReturn(List.of(server)); SkillEntity entity = bridge.listMcpDerivedSkillEntities().get(0); @@ -181,7 +181,7 @@ class McpSkillBridgeManifestTest { a.setToolsCacheJson(toolsJson("search")); McpServerEntity b = newServer(43L, "filesystem"); b.setToolsCacheJson(toolsJson("search")); - when(mcpServerService.listEnabled()).thenReturn(List.of(a, b)); + when(mcpServerService.listAll()).thenReturn(List.of(a, b)); List entities = bridge.listMcpDerivedSkillEntities(); diff --git a/mateclaw-ui/src/views/SkillMarket.vue b/mateclaw-ui/src/views/SkillMarket.vue index 078c10d9..f0ac9d01 100644 --- a/mateclaw-ui/src/views/SkillMarket.vue +++ b/mateclaw-ui/src/views/SkillMarket.vue @@ -98,12 +98,16 @@

{{ skill.name }}
- -