diff --git a/mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java b/mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java index cc22af1d..ea3f3b4b 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/AgentGraphBuilder.java @@ -310,6 +310,10 @@ public class AgentGraphBuilder { primaryModelConfig != null ? primaryModelConfig.getProvider() : null, providerPool); ToolExecutionExecutor executor = new ToolExecutionExecutor(toolSet, toolGuardService, approvalService, streamTracker, toolTimeoutProperties, toolResultStorage, toolConcurrencyRegistry); + // Issue #46: enable skill-aware "Tool not found" hint so when the + // LLM mis-calls a skill name as a tool, the response tells it + // the right invocation pattern instead of a dead-end error. + executor.setSkillRuntimeService(skillRuntimeService); PlanGenerationNode planGenerationNode = new PlanGenerationNode(chatModel, planningService, streamingHelper, conversationWindowManager, toolSet); StepExecutionNode stepExecutionNode = new StepExecutionNode(chatModel, toolSet, executor, planningService, streamTracker, reasoningEffort, streamingHelper, conversationWindowManager); PlanSummaryNode planSummaryNode = new PlanSummaryNode(chatModel, planningService, streamingHelper); @@ -437,6 +441,10 @@ public class AgentGraphBuilder { primaryModelConfig != null ? primaryModelConfig.getProvider() : null, providerPool); ToolExecutionExecutor executor = new ToolExecutionExecutor(toolSet, toolGuardService, approvalService, streamTracker, toolTimeoutProperties, toolResultStorage, toolConcurrencyRegistry); + // Issue #46: enable skill-aware "Tool not found" hint so when the + // LLM mis-calls a skill name as a tool, the response tells it + // the right invocation pattern instead of a dead-end error. + executor.setSkillRuntimeService(skillRuntimeService); // PR-1.2 (RFC-049 L1-B): propagate the bound model's capability so ReasoningNode // can gate the ThinkingLevelHolder override explicitly, rather than inferring // capability from reasoningEffort == null. diff --git a/mateclaw-server/src/main/java/vip/mate/agent/BaseAgent.java b/mateclaw-server/src/main/java/vip/mate/agent/BaseAgent.java index 3d159ab4..cac7ce46 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/BaseAgent.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/BaseAgent.java @@ -228,17 +228,36 @@ public abstract class BaseAgent { } } - // Tail guard: a few providers reject prompts whose history ends with an - // assistant message. Anthropic Claude returns 400 "does not support - // assistant message prefill"; DeepSeek thinking mode requires the - // last assistant turn's reasoning_content (which we may not have). - // The trailing-user-dedup above can already produce an assistant tail - // when the immediately-prior turn was an error / placeholder that got - // dropped by stage 1 / 1.5 of sanitizeForLlm. Strip remaining assistant - // tails defensively — the current user message is fed in separately as - // the final prompt by the caller, so dropping these assistant entries - // never loses information the LLM needs. - while (!messages.isEmpty() && messages.get(messages.size() - 1) instanceof AssistantMessage) { + // Tail guard — orphan-user strip (issue #47). + // + // Invariant: every caller of buildConversationHistory appends the + // current user message AFTER this history (BaseAgent.buildClient + // via .user(), StateGraphReActAgent / StateGraphPlanExecuteAgent + // via messages.add(buildCurrentUserMessage)). So the final prompt is + // [system, ...history, current_user] + // and is *always* terminated by a user message. That means a trailing + // assistant in history is FINE for every provider we support — it + // produces the correct [..., user, assistant, current_user] alternation + // (OpenAI, Anthropic, DeepSeek regular/thinking, Gemini, Qwen, …). + // + // The actual hazard is the opposite: a trailing USER in history. + // That happens when the immediately-prior turn's assistant message + // was dropped by Stage 1 (approval placeholder) or Stage 1.5 (errored + // turn / "[错误] " row), or never persisted at all (turn interrupted + // before doOnComplete saved the assistant). In that case the history + // ends with an orphan unanswered user, and appending the current user + // produces TWO consecutive user messages. Most providers concatenate + // those and answer both — leaking the orphan question's answer + // alongside the current answer. (This was the symptom reported in + // issue #47, originally caused by a tail guard that stripped trailing + // ASSISTANT messages instead of trailing USER ones — a direction- + // reversed version of this loop.) + // + // Stripping orphan users is safe: the user re-asked or asked a new + // question; the orphan turn produced no answer the model can build + // on. We lose a small amount of conversational context in exchange + // for clean alternation across every provider. + while (!messages.isEmpty() && messages.get(messages.size() - 1) instanceof UserMessage) { messages.remove(messages.size() - 1); } return messages; diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java index 0ed7cd4e..a926c0ae 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java @@ -123,6 +123,18 @@ public class ToolExecutionExecutor { private final ToolResultStorage resultStorage; /** RFC-008 Phase 4 metadata-driven concurrency classifier; nullable for legacy constructors. */ private final vip.mate.tool.ToolConcurrencyRegistry concurrencyRegistry; + /** + * Issue #46: when {@code toolCallbackMap} misses a name that the LLM + * called, we check whether it matches an active skill so we can + * return a precise hint instead of bare "Tool not found". Nullable — + * legacy constructors and tests may leave this unset, in which case + * the safety net falls through to the original error string. + */ + private vip.mate.skill.runtime.SkillRuntimeService skillRuntimeService; + + public void setSkillRuntimeService(vip.mate.skill.runtime.SkillRuntimeService s) { + this.skillRuntimeService = s; + } public ToolExecutionExecutor(AgentToolSet toolSet, ToolGuardService toolGuardService, ApprovalWorkflowService approvalService, ChatStreamTracker streamTracker) { @@ -326,10 +338,11 @@ public class ToolExecutionExecutor { } ToolCallback callback = toolCallbackMap.get(toolName); if (callback == null) { - log.warn("[ToolExecutor] Tool not found: {}", toolName); - events.add(GraphEventPublisher.toolComplete(toolCall.id(), toolName, "Tool not found: " + toolName, false)); + String msg = skillAwareNotFoundMessage(toolName); + log.warn("[ToolExecutor] {}", msg); + events.add(GraphEventPublisher.toolComplete(toolCall.id(), toolName, msg, false)); allResponses.add(new ToolResponseMessage.ToolResponse( - toolCall.id(), toolName, "Tool not found: " + toolName)); + toolCall.id(), toolName, msg)); continue; } @@ -402,9 +415,10 @@ public class ToolExecutionExecutor { ToolCallback callback = toolCallbackMap.get(toolName); if (callback == null) { - log.warn("[ToolExecutor] Pre-approved tool not found: {}", toolName); - events.add(GraphEventPublisher.toolComplete(toolCall.id(), toolName, "Tool not found: " + toolName, false)); - return new ToolResponseMessage.ToolResponse(toolCall.id(), toolName, "Tool not found: " + toolName); + String msg = skillAwareNotFoundMessage(toolName); + log.warn("[ToolExecutor] Pre-approved {}", msg); + events.add(GraphEventPublisher.toolComplete(toolCall.id(), toolName, msg, false)); + return new ToolResponseMessage.ToolResponse(toolCall.id(), toolName, msg); } try { @@ -817,6 +831,38 @@ public class ToolExecutionExecutor { return approvalResponse; } + /** + * Issue #46 — when a tool callback miss happens, check whether the + * unrecognized name actually matches an active skill. If it does, return + * a precise hint telling the LLM the right invocation pattern instead + * of bare "Tool not found: X". Without this, an LLM that called e.g. + * {@code RedisOps} as a tool gets no recovery signal and either gives + * up or falls back to shell guessing. + * + *

Case-insensitive match because LLMs sometimes change the case of + * skill names mid-conversation. + */ + private String skillAwareNotFoundMessage(String toolName) { + if (skillRuntimeService != null && toolName != null && !toolName.isBlank()) { + try { + boolean isSkill = skillRuntimeService.getActiveSkills().stream() + .anyMatch(s -> s.getName() != null && s.getName().equalsIgnoreCase(toolName)); + if (isSkill) { + return String.format( + "'%s' is a Skill, not a Tool — calling it as a tool fails. " + + "To use it, FIRST call readSkillFile(skillName=\"%s\", filePath=\"SKILL.md\") " + + "to read its instructions, THEN follow what SKILL.md tells you " + + "(typically runSkillScript with a scripts/ path).", + toolName, toolName); + } + } catch (Exception e) { + // Don't let a hint-side failure mask the original error. + log.debug("[ToolExecutor] skill-aware hint check failed: {}", e.getMessage()); + } + } + return "Tool not found: " + toolName; + } + // ==================== 内部数据类 ==================== private record PreparedToolCall( diff --git a/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillRuntimeService.java b/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillRuntimeService.java index 47cfe4c2..3e9d6496 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillRuntimeService.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/runtime/SkillRuntimeService.java @@ -162,30 +162,62 @@ public class SkillRuntimeService { return ""; } + // Issue #46 prompt rewrite. Three deliberate choices vs. the old + // version, all driven by observed LLM mis-calls (e.g. calling + // "RedisOps" directly as a tool): + // 1. Lead with an explicit warning that skills are NOT callable. + // Primacy bias — putting it first is what makes it stick. + // 2. Use the actual camelCase tool names (readSkillFile / + // runSkillScript). The old text said `read_skill_file` / + // `run_skill_script`, which don't exist in the tool registry, + // so even an LLM trying to comply couldn't find them. + // 3. Concrete worked example: "to use RedisOps, START with + // readSkillFile(...)". Abstract instructions reliably lose + // to a worked example in tool-use prompting. + // 4. Render the listing as a markdown table with the call pattern + // explicit, instead of `- **Name** — desc` which looks + // identical to a tool list and primes the model to call + // the name directly. StringBuilder sb = new StringBuilder(); - sb.append("\n\n## Available Skills\n"); - sb.append("以下技能已启用，你可以通过 skill runtime tools 使用它们：\n\n"); + sb.append("\n\n## Available Skills\n\n"); + sb.append("⚠️ **Skills are documentation packages, NOT directly callable tools.**\n"); + sb.append("Calling a skill name as a tool (e.g. tool_use{name=\"RedisOps\"}) will fail with \"Tool not found\". "); + sb.append("To use a skill, follow this two-step pattern:\n\n"); + sb.append("1. Read its SKILL.md to learn how it works:\n"); + sb.append(" `readSkillFile(skillName=\"\", filePath=\"SKILL.md\")`\n"); + sb.append("2. Follow what SKILL.md tells you — usually that means calling:\n"); + sb.append(" `runSkillScript(skillName=\"\", scriptPath=\"scripts/\")` or\n"); + sb.append(" `readSkillFile(skillName=\"\", filePath=\"references/\")`\n\n"); + // Concrete example anchored to the first enabled skill so the LLM + // sees a real name it just read in the listing below. + String exampleName = activeSkills.get(0).getName(); + sb.append("Concrete example — to use the `").append(exampleName).append("` skill, START with:\n"); + sb.append(" `readSkillFile(skillName=\"").append(exampleName).append("\", filePath=\"SKILL.md\")`\n\n"); + + sb.append("### Enabled skills\n"); + sb.append("Pass these names as the `skillName=` argument to `readSkillFile` / `runSkillScript`. "); + sb.append("Do **not** call them as tools.\n\n"); + sb.append("| Skill name | Description |\n"); + sb.append("|------------|-------------|\n"); for (ResolvedSkill skill : activeSkills) { - sb.append("- **").append(skill.getName()).append("**"); + sb.append("| `").append(skill.getName()).append("`"); if (skill.getIcon() != null && !skill.getIcon().isBlank()) { sb.append(" ").append(skill.getIcon()); } + sb.append(" | "); if (skill.getDescription() != null && !skill.getDescription().isBlank()) { String desc = skill.getDescription(); if (desc.length() > 200) { desc = desc.substring(0, 200) + "..."; } - sb.append(" — ").append(desc); + // Escape pipe and newline so a multi-line description doesn't + // break the table layout. + sb.append(desc.replace("|", "\\|").replace("\n", " ")); } - sb.append("\n"); + sb.append(" |\n"); } - sb.append("\n### 如何使用技能\n"); - sb.append("1. 使用 `read_skill_file` 工具读取技能内部文件（SKILL.md / references / scripts）\n"); - sb.append("2. 使用 `run_skill_script` 工具执行技能脚本\n"); - sb.append("3. 所有路径相对技能根目录解析，必须以 references/ 或 scripts/ 开头\n"); - return sb.toString(); } } diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java index abc0c89e..d1da943d 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java @@ -148,9 +148,16 @@ public class SkillFileTool { } @Tool(description = """ - 列出所有当前可用的技能（Skills），包括名称、图标和描述。 - 使用此工具查看系统中有哪些已启用且可运行的技能。 - 注意：这里列出的是技能（Skills），不是 Agent。如需列出可用 Agent，请使用 listAvailableAgents。 + List all currently available Skills (documentation packages). + + IMPORTANT: Skills are NOT directly callable as tools. Each name + returned here is a `skillName` argument, not a tool name. To use + a skill, call `readSkillFile(skillName="", filePath="SKILL.md")` + first to read its instructions, then follow what SKILL.md tells you. + Calling a skill name as a tool will fail with "Tool not found". + + Note: this returns Skills (vendor-installable docs), not Agents. + For Agents, use `listAvailableAgents`. Returns: A formatted list of active skills with name, icon, and description. """) @@ -160,27 +167,35 @@ public class SkillFileTool { List activeSkills = runtimeService.getActiveSkills(); if (activeSkills.isEmpty()) { - return "当前没有可用的技能（Skills）。"; + return "No skills are currently available."; } - StringBuilder sb = new StringBuilder("可用技能（Skills）列表：\n\n"); + // Issue #46: render as a table with the call pattern stated up front, + // instead of a `- **Name** — desc` list that primes the LLM to call + // the names directly as tools. + StringBuilder sb = new StringBuilder(); + sb.append("⚠️ These are Skills (documentation packages), NOT directly callable tools.\n"); + sb.append("To use any of them, call:\n"); + sb.append(" readSkillFile(skillName=\"\", filePath=\"SKILL.md\")\n"); + sb.append("then follow what SKILL.md tells you (typically `runSkillScript`).\n\n"); + sb.append("| Skill name | Description |\n"); + sb.append("|------------|-------------|\n"); for (ResolvedSkill skill : activeSkills) { - sb.append("- **").append(skill.getName()).append("**"); + sb.append("| `").append(skill.getName()).append("`"); if (skill.getIcon() != null && !skill.getIcon().isBlank()) { sb.append(" ").append(skill.getIcon()); } + sb.append(" | "); if (skill.getDescription() != null && !skill.getDescription().isBlank()) { String desc = skill.getDescription(); if (desc.length() > 200) { desc = desc.substring(0, 200) + "..."; } - sb.append(" — ").append(desc); + sb.append(desc.replace("|", "\\|").replace("\n", " ")); } - sb.append("\n"); + sb.append(" |\n"); } - - sb.append("\n共 ").append(activeSkills.size()).append(" 个可用技能。"); - sb.append("\n\n使用 `readSkillFile` 读取技能详情，使用 `runSkillScript` 执行技能脚本。"); + sb.append("\nTotal: ").append(activeSkills.size()).append(" skill(s)."); return sb.toString(); }