opensource/mateclaw
2
0
Fork 0
mirror of https://gitee.com/mateos/mateclaw.git synced 2026-09-13 03:13:41 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix(skill): synthesize SKILL.md for MCP-derived virtual skills (#136)

Browse Source
This commit is contained in:
matevip 2026-05-16 14:50:46 +08:00
parent 51c922f395
commit 130dfd5278
7 changed files with 430 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java
View File

@ -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<SkillEntity> enabledOnly(List<SkillEntity> 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<SkillEntity> result = new ArrayList<>(skillService.listEnabledSkills(workspaceId));
Set<String> 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<SkillEntity> 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));

167
mateclaw-server/src/main/java/vip/mate/skill/mcp/McpSkillBridge.java
View File

@ -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<SkillEntity> 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<ResolvedSkill> 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.
*
* <p>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<McpServerEntity> listEnabledServers() {
private List<McpServerEntity> 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<McpToolDescriptor> 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<String> rawNames = readToolRawNames(server);
List<McpToolDescriptor> tools = readToolDescriptors(server);
List<String> rawNames = toRawNames(tools);
Map<String, String> 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<String> readToolRawNames(McpServerEntity server) {
List<String> fromCache = parseCachedToolNames(server.getToolsCacheJson());
private List<McpToolDescriptor> readToolDescriptors(McpServerEntity server) {
List<McpToolDescriptor> fromCache = parseCachedToolDescriptors(server.getToolsCacheJson());
if (!fromCache.isEmpty()) {
return fromCache;
}
try {
List<McpSchema.Tool> discovered = mcpClientManager.getServerTools(server.getId());
List<String> names = new ArrayList<>(discovered.size());
List<McpToolDescriptor> 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<String> parseCachedToolNames(String json) {
private List<McpToolDescriptor> parseCachedToolDescriptors(String json) {
if (json == null || json.isBlank()) {
return List.of();
}
try {
cn.hutool.json.JSONArray arr = cn.hutool.json.JSONUtil.parseArray(json);
List<String> out = new ArrayList<>(arr.size());
List<McpToolDescriptor> 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<String> toRawNames(List<McpToolDescriptor> tools) {
List<String> 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.
*
* <p>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.
*
* <p>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<McpToolDescriptor> 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_-]", "-");

16
mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerListEnabledTest.java
View File

@ -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<List<SkillEntity>> 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);

44
mateclaw-server/src/test/java/vip/mate/skill/controller/SkillControllerVirtualGuardTest.java
View File

@ -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<SkillEntity> 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() {

184
mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeContentTest.java Normal file
View File

@ -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<SkillEntity> 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());
}
}

14
mateclaw-server/src/test/java/vip/mate/skill/mcp/McpSkillBridgeManifestTest.java
View File

@ -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<SkillEntity> 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<SkillEntity> entities = bridge.listMcpDerivedSkillEntities();

43
mateclaw-ui/src/views/SkillMarket.vue
View File

@ -98,12 +98,16 @@
<!-- RFC-042 §2.2.4 — slug under display name when they differ -->
<div v-if="hasI18nName(skill)" class="skill-slug">{{ skill.name }}</div>
</div>
<!-- Issue #83: virtual MCP/ACP skills are view-only mirrors of the
underlying MCP/ACP server row, with no mate_skill row to flip.
Hiding the toggle here matches how the configure / delete
buttons are gated below; users enable/disable from the
Settings ▸ MCP connection page instead. -->
<label v-if="!isSkillRowVirtual(skill)" class="toggle-switch" @click.stop>
<!-- MCP-derived skills accept the enable/disable toggle — it
forwards to the underlying MCP server connection, so the
Skills page and Settings ▸ MCP Connections stay in sync.
ACP-derived skills have no such mapping and stay read-only
(padlock); configure / delete are gated for both below. -->
<label
v-if="!isSkillRowVirtual(skill) || isMcpSkillRow(skill)"
class="toggle-switch"
@click.stop
>
<input type="checkbox" :checked="skill.enabled" @change="toggleSkill(skill)" />
<span class="toggle-slider"></span>
</label>
@ -750,17 +754,22 @@ const editBodyForm = ref<{ skillContent: string; sourceCode: string }>({
* at creation time. Everything else is filled in via the drawer. */
const newForm = ref<{ name: string; description: string; icon: string }>({ name: '', description: '', icon: '' })
/** Virtual MCP-derived skills synthesize their id from
* {@link McpSkillBridge#VIRTUAL_ID_BASE} (= 9e18). The DB update path
* doesn't know about them, so the drawer hides the Edit affordance.
* Using string-length is robust against JS number precision loss past 2^53. */
/** Virtual MCP/ACP-derived skills are read-only mirrors of a connection
* row — there is no mate_skill row to edit or delete, so the drawer hides
* the Edit affordance. The bridge encodes their ids with the sign bit set,
* so a virtual id is always negative while real Snowflake ids are always
* positive. Checking the leading '-' is precision-safe — no Number()
* round-trip that would corrupt ids past 2^53. */
function isVirtualSkillId(id: unknown): boolean {
if (id === null || id === undefined) return false
const idStr = String(id)
return idStr.length >= 19 && idStr.startsWith('9')
return String(id).trim().startsWith('-')
}
/** Per-row check used by the card-level configure / delete buttons. */
const isSkillRowVirtual = (skill: { id?: unknown } | null | undefined) => isVirtualSkillId(skill?.id)
/** MCP-derived rows stay read-only for edit/delete, but their enable/disable
* toggle is honored — it forwards to the underlying MCP server connection. */
const isMcpSkillRow = (skill: { skillType?: unknown } | null | undefined) =>
skill?.skillType === 'mcp'
const isVirtualSkill = computed(() => isVirtualSkillId(detailSkill.value?.id))
const isBuiltinDetail = computed(() => detailSkill.value?.skillType === 'builtin' || !!detailSkill.value?.builtin)
@ -1212,11 +1221,11 @@ async function deleteSkill(idOrSkill: string | number | Skill) {
}
async function toggleSkill(skill: Skill) {
// Issue #83: short-circuit if a programmatic caller reaches this for a
// virtual skill (the UI hides the toggle, but defense-in-depth keeps the
// toast accurate when the backend would otherwise return err.skill.not_found
// on builds that pre-date the rejectVirtualSkillMutation guard).
if (isSkillRowVirtual(skill)) {
// MCP virtual skills support enable/disable — the backend forwards it to
// the underlying MCP server. ACP virtual skills stay read-only: the UI
// hides their toggle, and this short-circuit keeps the toast accurate if
// a programmatic caller still reaches here.
if (isSkillRowVirtual(skill) && !isMcpSkillRow(skill)) {
mcToast.warning(t('skills.virtualReadonlyHint'))
return
}