diff --git a/mateclaw-server/src/main/java/vip/mate/skill/acp/AcpSkillBridge.java b/mateclaw-server/src/main/java/vip/mate/skill/acp/AcpSkillBridge.java index 204f22a6..448289e9 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/acp/AcpSkillBridge.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/acp/AcpSkillBridge.java @@ -329,6 +329,7 @@ public class AcpSkillBridge { s.setSecurityScanStatus("PASSED"); // ACP endpoints are user-configured external CLIs, not skill scripts s.setConfigJson(buildConfigJson(ep)); s.setManifestJson(serializeManifest(buildManifest(ep))); + s.setSkillContent(buildSkillContent(ep)); return s; } @@ -363,7 +364,7 @@ public class AcpSkillBridge { .id(virtualIdFor(ep)) .name(slugForEndpoint(ep)) .description(buildDescription(ep)) - .content("") // no SKILL.md + .content(buildSkillContent(ep)) .source("acp") .skillDir(null) .configuredSkillDir(null) @@ -442,6 +443,72 @@ public class AcpSkillBridge { .build(); } + /** + * Synthesize a SKILL.md body for an ACP-derived virtual skill. + * + *

ACP endpoints carry no hand-authored SKILL.md — they wrap an + * external coding-agent CLI rather than a skill package. Without a + * synthesized body, an agent that calls + * {@code readSkillFile(skillName=..., filePath="SKILL.md")} gets + * nothing beyond the one-line description and cannot tell how to + * drive the endpoint. + * + *

This builds a markdown brief from the live endpoint row: what + * the endpoint is, the single wrapper tool it exposes, that tool's + * arguments, and usage notes — so the LLM can call + * {@code acp__prompt} correctly on the first attempt. + */ + private String buildSkillContent(AcpEndpointEntity ep) { + String slug = slugForEndpoint(ep); + String toolName = "acp_" + slug + "_prompt"; + StringBuilder sb = new StringBuilder(); + + sb.append("# ").append(displayName(ep)).append("\n\n"); + sb.append(buildDescription(ep)).append("\n\n"); + + sb.append("## Overview\n\n"); + sb.append("This skill delegates work to the **").append(ep.getName()) + .append("** ACP (Agent Communication Protocol) coding agent. ") + .append("The agent runs as an external CLI process spawned on demand: ") + .append("send it a single natural-language instruction and it returns ") + .append("its final reply.\n\n"); + + sb.append("## Tools\n\n"); + sb.append("### `").append(toolName).append("`\n\n"); + sb.append("Delegate a prompt to the '").append(ep.getName()) + .append("' coding agent and receive its final reply.\n\n"); + sb.append("Parameters:\n\n"); + sb.append("- `prompt` (string, required) — the instruction or question to send.\n"); + sb.append("- `cwd` (string, optional) — working directory; defaults to the ") + .append("endpoint's workspace base path when omitted.\n\n"); + + sb.append("## Usage notes\n\n"); + sb.append("- Call `").append(toolName).append("` with one self-contained instruction. ") + .append("The endpoint runs autonomously and returns only its final answer, ") + .append("not intermediate steps.\n"); + sb.append("- Omit `cwd` unless the task needs a specific directory — the server ") + .append("resolves the endpoint's bound workspace path.\n"); + if (Boolean.TRUE.equals(ep.getTrusted())) { + sb.append("- This endpoint is trusted: the agent's own tool calls are accepted ") + .append("without re-prompting for approval.\n"); + } else { + sb.append("- This endpoint is not trusted: the agent's tool calls may require ") + .append("human approval before they run.\n"); + } + String status = nullSafe(ep.getLastStatus()); + if ("OK".equalsIgnoreCase(status)) { + sb.append("- Last connection test: OK.\n"); + } else if ("ERROR".equalsIgnoreCase(status) + || (ep.getLastError() != null && !ep.getLastError().isBlank())) { + sb.append("- Last connection test failed: ").append(nullSafe(ep.getLastError())) + .append(". The CLI may not be installed or reachable.\n"); + } else { + sb.append("- Not yet tested — the CLI is spawned on the first call.\n"); + } + + return sb.toString(); + } + // ==================== Helpers ==================== private List safeListEnabled() { diff --git a/mateclaw-server/src/test/java/vip/mate/skill/acp/AcpSkillBridgeContentTest.java b/mateclaw-server/src/test/java/vip/mate/skill/acp/AcpSkillBridgeContentTest.java new file mode 100644 index 00000000..b192796e --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/skill/acp/AcpSkillBridgeContentTest.java @@ -0,0 +1,186 @@ +package vip.mate.skill.acp; + +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.acp.model.AcpEndpointEntity; +import vip.mate.acp.service.AcpDelegationService; +import vip.mate.acp.service.AcpEndpointService; +import vip.mate.skill.model.SkillEntity; +import vip.mate.skill.runtime.model.ResolvedSkill; +import vip.mate.tool.ToolRegistry; + +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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Asserts the bridge synthesizes a non-empty SKILL.md body for every + * ACP-derived virtual skill and wires it onto both the {@link ResolvedSkill} + * {@code content} field and the virtual {@link SkillEntity} {@code skillContent}. + * + *

Without a body, an agent calling + * {@code readSkillFile(filePath="SKILL.md")} on an ACP endpoint gets + * "Error: SKILL.md content not available" and has nothing beyond the + * one-line description to work from. + */ +class AcpSkillBridgeContentTest { + + private AcpEndpointService endpointService; + private AcpSkillBridge bridge; + + @BeforeEach + void setUp() { + endpointService = mock(AcpEndpointService.class); + AcpDelegationService delegationService = mock(AcpDelegationService.class); + ToolRegistry toolRegistry = mock(ToolRegistry.class); + bridge = new AcpSkillBridge(endpointService, delegationService, new ObjectMapper(), toolRegistry); + } + + @Test + @DisplayName("ResolvedSkill carries a synthesized SKILL.md body, not an empty string") + void resolvedSkillHasNonEmptyContent() { + AcpEndpointEntity ep = newEndpoint(7L, "codex"); + ep.setDescription("OpenAI-compatible coding agent"); + when(endpointService.listEnabled()).thenReturn(List.of(ep)); + + ResolvedSkill resolved = bridge.listAcpDerivedResolvedSkills().get(0); + + assertNotNull(resolved.getContent()); + assertFalse(resolved.getContent().isBlank(), "SKILL.md body must not be blank"); + String body = resolved.getContent(); + assertTrue(body.contains("# codex"), "expected a markdown title, got: " + body); + assertTrue(body.contains("OpenAI-compatible coding agent"), + "expected the endpoint description in the body, got: " + body); + assertTrue(body.contains("acp_codex_prompt"), + "expected the wrapper tool name in the body, got: " + body); + assertTrue(body.contains("`prompt`") && body.contains("`cwd`"), + "expected both wrapper parameters documented, got: " + body); + assertTrue(body.contains("## Usage notes"), + "expected a usage notes section, got: " + body); + } + + @Test + @DisplayName("virtual SkillEntity skillContent matches the ResolvedSkill content") + void skillEntityContentMatchesResolved() { + AcpEndpointEntity ep = newEndpoint(7L, "codex"); + when(endpointService.listEnabled()).thenReturn(List.of(ep)); + + SkillEntity entity = bridge.listAcpDerivedSkillEntities().get(0); + ResolvedSkill resolved = bridge.listAcpDerivedResolvedSkills().get(0); + + assertNotNull(entity.getSkillContent()); + assertFalse(entity.getSkillContent().isBlank(), "skillContent must not be blank"); + assertEquals(resolved.getContent(), entity.getSkillContent(), + "entity skillContent and resolved content must be the same synthesized body"); + } + + @Test + @DisplayName("trusted endpoints document the no-approval behavior") + void trustedEndpointPhrasing() { + AcpEndpointEntity ep = newEndpoint(7L, "codex"); + ep.setTrusted(true); + when(endpointService.listEnabled()).thenReturn(List.of(ep)); + + String body = bridge.listAcpDerivedResolvedSkills().get(0).getContent(); + + assertTrue(body.contains("trusted: the agent's own tool calls are accepted"), + "expected trusted phrasing, got: " + body); + } + + @Test + @DisplayName("untrusted endpoints document that tool calls may need approval") + void untrustedEndpointPhrasing() { + AcpEndpointEntity ep = newEndpoint(7L, "codex"); + ep.setTrusted(false); + when(endpointService.listEnabled()).thenReturn(List.of(ep)); + + String body = bridge.listAcpDerivedResolvedSkills().get(0).getContent(); + + assertTrue(body.contains("not trusted: the agent's tool calls may require"), + "expected untrusted phrasing, got: " + body); + } + + @Test + @DisplayName("status hint reflects a failed connection test") + void erroredEndpointStatusHint() { + AcpEndpointEntity ep = newEndpoint(7L, "codex"); + ep.setLastStatus("ERROR"); + ep.setLastError("command not found: codex"); + when(endpointService.listEnabled()).thenReturn(List.of(ep)); + + String body = bridge.listAcpDerivedResolvedSkills().get(0).getContent(); + + assertTrue(body.contains("Last connection test failed: command not found: codex"), + "expected the error surfaced in the body, got: " + body); + } + + @Test + @DisplayName("status hint reflects an OK connection test") + void okEndpointStatusHint() { + AcpEndpointEntity ep = newEndpoint(7L, "codex"); + ep.setLastStatus("OK"); + when(endpointService.listEnabled()).thenReturn(List.of(ep)); + + String body = bridge.listAcpDerivedResolvedSkills().get(0).getContent(); + + assertTrue(body.contains("Last connection test: OK."), + "expected OK status in the body, got: " + body); + } + + @Test + @DisplayName("untested endpoints note the CLI is spawned on first call") + void untestedEndpointStatusHint() { + AcpEndpointEntity ep = newEndpoint(7L, "codex"); + ep.setLastStatus("UNKNOWN"); + when(endpointService.listEnabled()).thenReturn(List.of(ep)); + + String body = bridge.listAcpDerivedResolvedSkills().get(0).getContent(); + + assertTrue(body.contains("Not yet tested"), + "expected an untested hint, got: " + body); + } + + @Test + @DisplayName("CJK-only endpoint names slug to a stable id-based wrapper tool name") + void cjkOnlyNameUsesIdSlugInContent() { + AcpEndpointEntity ep = newEndpoint(42L, "代码助手"); + when(endpointService.listEnabled()).thenReturn(List.of(ep)); + + String body = bridge.listAcpDerivedResolvedSkills().get(0).getContent(); + + assertTrue(body.contains("acp_acp-42_prompt"), + "all-CJK name should fall back to an id-based wrapper name, got: " + body); + } + + @Test + @DisplayName("findResolvedById and findEntityById return entries with the synthesized body") + void lookupByVirtualIdCarriesContent() { + AcpEndpointEntity ep = newEndpoint(7L, "codex"); + when(endpointService.get(7L)).thenReturn(ep); + + long virtualId = AcpSkillBridge.virtualIdFor(ep); + ResolvedSkill resolved = bridge.findResolvedById(virtualId); + SkillEntity entity = bridge.findEntityById(virtualId); + + assertNotNull(resolved); + assertNotNull(entity); + assertFalse(resolved.getContent().isBlank(), "resolved content must not be blank"); + assertFalse(entity.getSkillContent().isBlank(), "entity skillContent must not be blank"); + } + + private static AcpEndpointEntity newEndpoint(long id, String name) { + AcpEndpointEntity ep = new AcpEndpointEntity(); + ep.setId(id); + ep.setName(name); + ep.setEnabled(true); + ep.setCommand("codex"); + return ep; + } +}