From 4f3276b9e639056c2da8be7a3d944a438a813032 Mon Sep 17 00:00:00 2001 From: matevip Date: Thu, 30 Apr 2026 18:04:11 +0800 Subject: [PATCH] fix(skill): mark docs-only vs scripts+docs in Available Skills prompt (issue #49) --- .../skill/runtime/SkillRuntimeService.java | 57 +++++++++++-------- 1 file changed, 33 insertions(+), 24 deletions(-) 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 3e9d6496..09045ce4 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,32 +162,33 @@ 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. + // Issue #46 + #49 prompt rewrite. Stop-gap until RFC-090 lands the + // proper `type` enum and inlines `type: prompt` skill bodies into the + // system prompt directly. Today every skill is announced via this + // catalog and the LLM has to pull SKILL.md on demand. Two failure + // modes have been observed: + // #46 — LLM treated the skill name as a tool ("tool_use{name= + // RedisOps}"), so we lead with an explicit "NOT callable" + // warning and surface the actual tool names readSkillFile / + // runSkillScript. + // #49 — On a docs-only skill (SKILL.md, no scripts/ dir), the LLM + // tried to invoke a non-existent scripts/ file and never + // followed SKILL.md's text guidance. The previous wording + // "usually that means calling runSkillScript(...)" actively + // pushed it that way. Fix: tell the model the two shapes + // exist, mark each row's shape, and forbid invoking scripts + // on docs-only skills. StringBuilder sb = new StringBuilder(); 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("To use a skill:\n\n"); + sb.append("1. ALWAYS first 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"); + sb.append("2. Then follow what SKILL.md says. Skills come in two shapes — check the **Shape** column below:\n"); + sb.append(" - **docs only** — no `scripts/` directory exists. SKILL.md is the entire instruction set; follow its text guidance directly. Do NOT call `runSkillScript` on these — the script does not exist and the call will fail.\n"); + sb.append(" - **scripts + docs** — `scripts/` is present. SKILL.md will name the script to run; invoke it with `runSkillScript(skillName=\"\", scriptPath=\"scripts/\")`.\n"); + sb.append(" Either shape may also expose supplementary docs via `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. @@ -198,14 +199,22 @@ public class SkillRuntimeService { 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"); + sb.append("| Skill name | Shape | Description |\n"); + sb.append("|------------|-------|-------------|\n"); for (ResolvedSkill skill : activeSkills) { + // `scripts` is populated by SkillDirectoryScanner — empty map both + // for "scripts/ directory absent" and "directory present but empty". + // Either way, runSkillScript has nothing to call, so we report the + // skill as docs-only. (Database-fallback skills also land here + // because SkillPackageResolver.resolveFromDatabase sets it to + // Map.of().) RFC-090's `type` field will replace this heuristic. + boolean hasScripts = skill.getScripts() != null && !skill.getScripts().isEmpty(); + String shape = hasScripts ? "scripts + docs" : "docs only"; sb.append("| `").append(skill.getName()).append("`"); if (skill.getIcon() != null && !skill.getIcon().isBlank()) { sb.append(" ").append(skill.getIcon()); } - sb.append(" | "); + sb.append(" | ").append(shape).append(" | "); if (skill.getDescription() != null && !skill.getDescription().isBlank()) { String desc = skill.getDescription(); if (desc.length() > 200) {