From b35816f0bb8cc02554f7acb928cd93995e68a429 Mon Sep 17 00:00:00 2001 From: fang-tech Date: Fri, 24 Apr 2026 00:02:31 +0800 Subject: [PATCH 1/5] refactor(skill): preserve rich metadata across parsing --- agentscope-core/pom.xml | 5 + .../io/agentscope/core/skill/AgentSkill.java | 135 +++++- .../core/skill/util/MarkdownSkillParser.java | 408 ++++++------------ .../skill/util/SkillFileSystemHelper.java | 7 +- .../agentscope/core/skill/util/SkillUtil.java | 26 +- .../agentscope/core/skill/AgentSkillTest.java | 112 +++++ .../agentscope/core/skill/SkillUtilTest.java | 2 + .../skill/util/MarkdownSkillParserTest.java | 175 ++++++-- 8 files changed, 514 insertions(+), 356 deletions(-) diff --git a/agentscope-core/pom.xml b/agentscope-core/pom.xml index b5bb9a12b4..f0ffe2d750 100644 --- a/agentscope-core/pom.xml +++ b/agentscope-core/pom.xml @@ -141,6 +141,11 @@ json-schema-validator + + org.yaml + snakeyaml + + io.opentelemetry opentelemetry-api diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkill.java b/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkill.java index ee3027b343..0cdc49d491 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkill.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkill.java @@ -19,6 +19,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -62,8 +63,7 @@ * @see io.agentscope.core.skill.util.MarkdownSkillParser */ public class AgentSkill { - private final String name; - private final String description; + private final Map metadata; private final String skillContent; private final Map resources; private final String source; @@ -104,16 +104,37 @@ public AgentSkill( String skillContent, Map resources, String source) { - if (name == null || name.isEmpty() || description == null || description.isEmpty()) { - throw new IllegalArgumentException( - "The skill must have `name` and `description` fields."); - } + this(createMetadata(name, description), skillContent, resources, source); + } + + /** + * Creates an AgentSkill with explicit metadata. + * + *

The metadata must include non-empty string values for {@code name} and + * {@code description}. The metadata map is copied and stored as immutable. + * + * @param metadata Skill metadata including required {@code name} and {@code description} + * @param skillContent The skill implementation or instructions (must not be null or empty) + * @param resources Supporting resources referenced by the skill (can be null) + * @param source Source identifier for the skill (null defaults to "custom") + * @throws IllegalArgumentException if metadata is invalid or skillContent is null or empty + */ + public AgentSkill( + Map metadata, + String skillContent, + Map resources, + String source) { + String name = getRequiredMetadataString(metadata, "name"); + String description = getRequiredMetadataString(metadata, "description"); if (skillContent == null || skillContent.isEmpty()) { throw new IllegalArgumentException("The skill must have content"); } - this.name = name; - this.description = description; + LinkedHashMap metadataCopy = new LinkedHashMap<>(metadata); + metadataCopy.put("name", name); + metadataCopy.put("description", description); + + this.metadata = Collections.unmodifiableMap(metadataCopy); this.skillContent = skillContent; this.resources = resources != null ? new HashMap<>(resources) : new HashMap<>(); this.source = source != null ? source : "custom"; @@ -125,7 +146,7 @@ public AgentSkill( * @return The skill name (never null) */ public String getName() { - return name; + return (String) metadata.get("name"); } /** @@ -134,7 +155,26 @@ public String getName() { * @return The skill description (never null) */ public String getDescription() { - return description; + return (String) metadata.get("description"); + } + + /** + * Gets the skill metadata. + * + * @return The immutable metadata map (never null, may be empty except required fields) + */ + public Map getMetadata() { + return metadata; + } + + /** + * Gets a metadata value by key. + * + * @param key The metadata key + * @return The metadata value, or null if not found + */ + public Object getMetadataValue(String key) { + return metadata.get(key); } /** @@ -193,7 +233,7 @@ public Set getResourcePaths() { * @return Unique skill identifier (never null) */ public String getSkillId() { - return name + "_" + source; + return getName() + "_" + source; } /** @@ -224,9 +264,9 @@ public static Builder builder() { @Override public String toString() { return "AgentSkill{name='" - + name + + getName() + "', description='" - + description + + getDescription() + "', source='" + source + "'}"; @@ -256,8 +296,7 @@ public String toString() { * } */ public static class Builder { - private String name; - private String description; + private Map metadata; private String skillContent; private Map resources; private String source; @@ -266,6 +305,7 @@ public static class Builder { * Creates an empty builder. */ private Builder() { + this.metadata = new LinkedHashMap<>(); this.resources = new HashMap<>(); } @@ -275,8 +315,7 @@ private Builder() { * @param baseSkill The skill to copy values from */ private Builder(AgentSkill baseSkill) { - this.name = baseSkill.name; - this.description = baseSkill.description; + this.metadata = new LinkedHashMap<>(baseSkill.metadata); this.skillContent = baseSkill.skillContent; this.resources = new HashMap<>(baseSkill.resources); this.source = baseSkill.source; @@ -289,7 +328,7 @@ private Builder(AgentSkill baseSkill) { * @return This builder */ public Builder name(String name) { - this.name = name; + this.metadata.put("name", name); return this; } @@ -300,7 +339,42 @@ public Builder name(String name) { * @return This builder */ public Builder description(String description) { - this.description = description; + this.metadata.put("description", description); + return this; + } + + /** + * Replaces all metadata with a new map. + * + * @param metadata The new metadata map + * @return This builder + */ + public Builder metadata(Map metadata) { + this.metadata = + metadata != null ? new LinkedHashMap<>(metadata) : new LinkedHashMap<>(); + return this; + } + + /** + * Adds or updates a single metadata entry. + * + * @param key The metadata key + * @param value The metadata value + * @return This builder + */ + public Builder putMetadata(String key, Object value) { + this.metadata.put(key, value); + return this; + } + + /** + * Removes a metadata entry. + * + * @param key The metadata key to remove + * @return This builder + */ + public Builder removeMetadata(String key) { + this.metadata.remove(key); return this; } @@ -377,7 +451,28 @@ public Builder source(String source) { * @throws IllegalArgumentException if required fields are missing */ public AgentSkill build() { - return new AgentSkill(name, description, skillContent, resources, source); + return new AgentSkill(metadata, skillContent, resources, source); + } + } + + private static Map createMetadata(String name, String description) { + LinkedHashMap metadata = new LinkedHashMap<>(); + metadata.put("name", name); + metadata.put("description", description); + return metadata; + } + + private static String getRequiredMetadataString(Map metadata, String key) { + if (metadata == null) { + throw new IllegalArgumentException( + "The skill must have `name` and `description` fields."); + } + + Object value = metadata.get(key); + if (!(value instanceof String stringValue) || stringValue.isEmpty()) { + throw new IllegalArgumentException( + "The skill must have `name` and `description` fields."); } + return stringValue; } } diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/util/MarkdownSkillParser.java b/agentscope-core/src/main/java/io/agentscope/core/skill/util/MarkdownSkillParser.java index 939b1b8142..90f780e8d7 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/util/MarkdownSkillParser.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/util/MarkdownSkillParser.java @@ -16,12 +16,22 @@ package io.agentscope.core.skill.util; +import java.lang.reflect.Array; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.yaml.snakeyaml.DumperOptions; +import org.yaml.snakeyaml.LoaderOptions; +import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.constructor.SafeConstructor; +import org.yaml.snakeyaml.representer.Representer; /** * Utility for parsing and generating Markdown files with YAML frontmatter. @@ -47,7 +57,7 @@ *

{@code
  * // Parse markdown with frontmatter
  * ParsedMarkdown parsed = MarkdownSkillParser.parse(markdownContent);
- * Map metadata = parsed.getMetadata();
+ * Map metadata = parsed.getMetadata();
  * String content = parsed.getContent();
  *
  * // Generate markdown with frontmatter
@@ -56,27 +66,23 @@
  */
 public class MarkdownSkillParser {
 
+    private static final int FRONTMATTER_CODE_POINT_LIMIT = 16_384;
+
     private static final Logger logger = LoggerFactory.getLogger(MarkdownSkillParser.class);
 
+    private static final Pattern FRONTMATTER_PATTERN =
+            Pattern.compile(
+                    "^---\\s*[\\r\\n]+(.*?)[\\r\\n]*---(?:\\s*[\\r\\n]+)?(.*)", Pattern.DOTALL);
+
+    private static final LoaderOptions LOADER_OPTIONS = createLoaderOptions();
+
+    private static final DumperOptions DUMPER_OPTIONS = createDumperOptions();
+
     /**
      * Private constructor to prevent instantiation.
      */
     private MarkdownSkillParser() {}
 
-    // Pattern to match frontmatter: starts with ---, ends with ---
-    // Pattern explanation:
-    // ^---          : frontmatter starts with --- at the beginning of the string
-    // \\s*          : optional whitespace after opening ---
-    // [\\r\\n]+     : one or more line breaks (handles \n, \r\n, \r)
-    // (.*?)         : captured group - frontmatter content (non-greedy, can be empty)
-    // [\\r\\n]*     : zero or more line breaks before closing ---
-    // ---           : closing --- delimiter
-    // (?:\\s*[\\r\\n]+)? : optional whitespace and line breaks after closing ---
-    // (.*)          : captured group - remaining content (greedy)
-    private static final Pattern FRONTMATTER_PATTERN =
-            Pattern.compile(
-                    "^---\\s*[\\r\\n]+(.*?)[\\r\\n]*---(?:\\s*[\\r\\n]+)?(.*)", Pattern.DOTALL);
-
     /**
      * Parse markdown content with YAML frontmatter.
      *
@@ -94,7 +100,6 @@ public static ParsedMarkdown parse(String markdown) {
         Matcher matcher = FRONTMATTER_PATTERN.matcher(markdown);
 
         if (!matcher.matches()) {
-            // No frontmatter found, treat entire content as markdown
             return new ParsedMarkdown(Map.of(), markdown);
         }
 
@@ -105,8 +110,7 @@ public static ParsedMarkdown parse(String markdown) {
             return new ParsedMarkdown(Map.of(), markdownContent);
         }
 
-        Map metadata = SimpleYamlParser.parse(yamlContent);
-        return new ParsedMarkdown(metadata, markdownContent);
+        return new ParsedMarkdown(parseYamlMetadata(yamlContent), markdownContent);
     }
 
     /**
@@ -119,19 +123,16 @@ public static ParsedMarkdown parse(String markdown) {
      * @param content Markdown content (can be null or empty)
      * @return Complete markdown with frontmatter
      */
-    public static String generate(Map metadata, String content) {
+    public static String generate(Map metadata, String content) {
         StringBuilder sb = new StringBuilder();
 
-        // Add frontmatter if metadata exists
         if (metadata != null && !metadata.isEmpty()) {
             sb.append("---\n");
-            sb.append(SimpleYamlParser.generate(metadata));
+            sb.append(createDumperYaml().dump(metadata));
             sb.append("---\n");
         }
 
-        // Add content
         if (content != null && !content.isEmpty()) {
-            // Add a blank line between frontmatter and content if frontmatter exists
             if (metadata != null && !metadata.isEmpty()) {
                 sb.append("\n");
             }
@@ -141,290 +142,119 @@ public static String generate(Map metadata, String content) {
         return sb.toString();
     }
 
-    /**
-     * Simple YAML parser for flat key-value structures.
-     * Only supports String:String mappings.
-     */
-    private static class SimpleYamlParser {
-
-        // Pattern to match key: value format
-        // Captures: group(1) = key, group(2) = value (may include quotes)
-        private static final Pattern KEY_VALUE_PATTERN =
-                Pattern.compile("^([a-zA-Z_][a-zA-Z0-9_-]*)\\s*:\\s*(.*)$");
-
-        /**
-         * Parse YAML string into a map of key-value pairs.
-         *
-         * 

This is a simplified parser designed for flat string-to-string mappings. - * Block-style complex YAML structures (such as multi-line lists or indented - * nested objects) are not supported and will be gracefully skipped. - * However, flow-style inline structures (e.g., single-line JSON strings) - * are treated as standard scalar values and will be parsed as raw strings. - * - * @param yaml YAML content to parse - * @return Map of key-value pairs - */ - static Map parse(String yaml) { - Map result = new LinkedHashMap<>(); - - if (yaml == null || yaml.isEmpty()) { - return result; - } - - String[] lines = yaml.split("[\\r\\n]+"); - - for (String line : lines) { - // Skip empty lines - if (line.trim().isEmpty()) { - continue; - } - - // Skip comments - if (line.trim().startsWith("#")) { - continue; - } - - Matcher matcher = KEY_VALUE_PATTERN.matcher(line.trim()); - if (!matcher.matches()) { - logger.debug( - "Skipping unsupported YAML line (expected 'key: value' format): {}", - line); - continue; - } - - String key = matcher.group(1); - String rawValue = matcher.group(2); - - if (isBlockScalarModifier(rawValue)) { - logger.debug( - "Skipping key '{}': block-style values ('{}') are unsupported", - key, - rawValue.trim()); - continue; - } - - result.put(key, parseValue(rawValue)); - } - - return result; + private static Map parseYamlMetadata(String yamlContent) { + if (yamlContent.codePointCount(0, yamlContent.length()) > FRONTMATTER_CODE_POINT_LIMIT) { + logger.debug( + "Skipping YAML frontmatter because it exceeds the code point limit: {}", + FRONTMATTER_CODE_POINT_LIMIT); + return Map.of(); } - /** - * Check if the raw value is a YAML block scalar modifier ('|' or '>'). - * - * @param rawValue The raw string captured after the colon - * @return true if it is a block scalar modifier - */ - private static boolean isBlockScalarModifier(String rawValue) { - if (rawValue == null) { - return false; - } - - String trimmed = rawValue.trim(); - return "|".equals(trimmed) || ">".equals(trimmed); + Object loaded; + try { + loaded = createParserYaml().load(yamlContent); + } catch (RuntimeException e) { + logger.debug("Failed to parse YAML frontmatter, returning empty metadata", e); + return Map.of(); } - /** - * Parse a YAML value, handling quoted strings. - * - * @param rawValue Raw value string from YAML - * @return Parsed value with quotes removed if present - */ - private static String parseValue(String rawValue) { - if (rawValue == null) { - return ""; - } - - String value = rawValue.trim(); - - if (value.isEmpty()) { - return ""; - } - - // Handle double-quoted strings - if (value.startsWith("\"") && value.endsWith("\"") && value.length() >= 2) { - return unescapeString(value.substring(1, value.length() - 1)); - } - - // Handle single-quoted strings - if (value.startsWith("'") && value.endsWith("'") && value.length() >= 2) { - // Single-quoted strings don't process escapes, except '' for ' - return value.substring(1, value.length() - 1).replace("''", "'"); - } - - return value; + if (loaded == null) { + return Map.of(); } - /** - * Unescape a double-quoted YAML string. - * - * @param str String content without surrounding quotes - * @return Unescaped string - */ - private static String unescapeString(String str) { - if (str == null || str.isEmpty()) { - return str; - } - - StringBuilder result = new StringBuilder(); - boolean escape = false; - - for (int i = 0; i < str.length(); i++) { - char c = str.charAt(i); - - if (escape) { - switch (c) { - case 'n': - result.append('\n'); - break; - case 't': - result.append('\t'); - break; - case 'r': - result.append('\r'); - break; - case '\\': - result.append('\\'); - break; - case '"': - result.append('"'); - break; - default: - result.append('\\').append(c); - } - escape = false; - } else if (c == '\\') { - escape = true; - } else { - result.append(c); - } - } - - // Handle trailing backslash - if (escape) { - result.append('\\'); - } - - return result.toString(); + if (!(loaded instanceof Map rawMap)) { + logger.debug( + "Skipping YAML frontmatter because top-level object is not a map: {}", + loaded.getClass()); + return Map.of(); } - /** - * Generate YAML string from a map of key-value pairs. - * - * @param map Map to serialize - * @return YAML string - */ - static String generate(Map map) { - if (map == null || map.isEmpty()) { - return ""; + LinkedHashMap metadata = new LinkedHashMap<>(); + for (Map.Entry entry : rawMap.entrySet()) { + Object key = entry.getKey(); + if (!(key instanceof String stringKey)) { + logger.debug("Skipping YAML metadata entry with non-string key: {}", key); + continue; } - StringBuilder sb = new StringBuilder(); + metadata.put(stringKey, normalizeMetadataValue(entry.getValue())); + } + return metadata; + } - for (Map.Entry entry : map.entrySet()) { - String key = entry.getKey(); - String value = entry.getValue(); + private static LoaderOptions createLoaderOptions() { + LoaderOptions options = new LoaderOptions(); + options.setAllowDuplicateKeys(false); + options.setMaxAliasesForCollections(10); + options.setNestingDepthLimit(10); + options.setCodePointLimit(10_000); + return options; + } - sb.append(key).append(": "); + private static DumperOptions createDumperOptions() { + DumperOptions options = new DumperOptions(); + options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK); + options.setPrettyFlow(true); + options.setSplitLines(false); + return options; + } - if (value == null || value.isEmpty()) { - sb.append(""); - } else if (needsQuoting(value)) { - sb.append(quoteValue(value)); - } else { - sb.append(value); - } + private static Yaml createParserYaml() { + return new Yaml(new SafeConstructor(LOADER_OPTIONS)); + } - sb.append("\n"); - } + private static Yaml createDumperYaml() { + return new Yaml(new Representer(DUMPER_OPTIONS), DUMPER_OPTIONS); + } - return sb.toString(); + /** + * Normalizes YAML parser output into a stable metadata tree. + * + *

Why: {@link SafeConstructor} keeps parsing safe, but it still returns broad container + * types like {@code Map}, arbitrary {@link Collection} implementations, and arrays. + * The rest of the skill pipeline needs a predictable shape so nested metadata can be preserved + * and later rendered as XML without losing structure. + * + *

How: this method recursively rewrites nested values into three forms only: + *

    + *
  • scalar values are kept as-is
  • + *
  • maps become {@code Map}
  • + *
  • collections and arrays become {@code List} + * + */ + private static Object normalizeMetadataValue(Object value) { + if (value == null) { + return null; } - /** - * Check if a value needs to be quoted in YAML. - * - * @param value Value to check - * @return true if quoting is needed - */ - private static boolean needsQuoting(String value) { - if (value.isEmpty()) { - return false; - } - - // Quote if contains special characters - if (value.contains(":") - || value.contains("#") - || value.contains("\n") - || value.contains("\r") - || value.contains("\t")) { - return true; - } - - // Quote if starts/ends with whitespace - if (Character.isWhitespace(value.charAt(0)) - || Character.isWhitespace(value.charAt(value.length() - 1))) { - return true; + if (value instanceof Map rawMap) { + LinkedHashMap normalized = new LinkedHashMap<>(); + for (Map.Entry entry : rawMap.entrySet()) { + Object key = entry.getKey(); + String normalizedKey = key instanceof String ? (String) key : String.valueOf(key); + normalized.put(normalizedKey, normalizeMetadataValue(entry.getValue())); } + return normalized; + } - // Quote if starts with special YAML characters - char first = value.charAt(0); - if (first == '"' - || first == '\'' - || first == '[' - || first == ']' - || first == '{' - || first == '}' - || first == '>' - || first == '|' - || first == '*' - || first == '&' - || first == '!' - || first == '%' - || first == '@' - || first == '`') { - return true; + if (value instanceof Collection collection) { + List normalized = new ArrayList<>(collection.size()); + for (Object item : collection) { + normalized.add(normalizeMetadataValue(item)); } - - return false; + return normalized; } - /** - * Quote a value for YAML output using double quotes. - * - * @param value Value to quote - * @return Quoted and escaped value - */ - private static String quoteValue(String value) { - StringBuilder sb = new StringBuilder(); - sb.append('"'); - - for (int i = 0; i < value.length(); i++) { - char c = value.charAt(i); - switch (c) { - case '"': - sb.append("\\\""); - break; - case '\\': - sb.append("\\\\"); - break; - case '\n': - sb.append("\\n"); - break; - case '\r': - sb.append("\\r"); - break; - case '\t': - sb.append("\\t"); - break; - default: - sb.append(c); - } + if (value.getClass().isArray()) { + int length = Array.getLength(value); + List normalized = new ArrayList<>(length); + for (int i = 0; i < length; i++) { + normalized.add(normalizeMetadataValue(Array.get(value, i))); } - - sb.append('"'); - return sb.toString(); + return normalized; } + + return value; } /** @@ -433,7 +263,7 @@ private static String quoteValue(String value) { *

    Contains both the extracted metadata and the markdown content. */ public static class ParsedMarkdown { - private final Map metadata; + private final Map metadata; private final String content; /** @@ -442,9 +272,11 @@ public static class ParsedMarkdown { * @param metadata YAML metadata (never null, can be empty) * @param content Markdown content (never null, can be empty) */ - public ParsedMarkdown(Map metadata, String content) { + public ParsedMarkdown(Map metadata, String content) { this.metadata = - metadata != null ? new LinkedHashMap<>(metadata) : new LinkedHashMap<>(); + metadata != null + ? Collections.unmodifiableMap(new LinkedHashMap<>(metadata)) + : Collections.emptyMap(); this.content = content != null ? content : ""; } @@ -453,8 +285,8 @@ public ParsedMarkdown(Map metadata, String content) { * * @return Metadata map (never null, can be empty) */ - public Map getMetadata() { - return new LinkedHashMap<>(metadata); + public Map getMetadata() { + return metadata; } /** diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillFileSystemHelper.java b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillFileSystemHelper.java index 39c84ac805..83091bf46b 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillFileSystemHelper.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillFileSystemHelper.java @@ -193,7 +193,7 @@ public static boolean saveSkills(Path baseDir, List skills, boolean Files.createDirectories(skillDir); - Map metadata = new LinkedHashMap<>(); + Map metadata = new LinkedHashMap<>(); metadata.put("name", skill.getName()); metadata.put("description", skill.getDescription()); @@ -451,8 +451,9 @@ private static Optional readSkillName(Path skillDir) { try { String skillMdContent = Files.readString(skillFile, StandardCharsets.UTF_8); ParsedMarkdown parsed = MarkdownSkillParser.parse(skillMdContent); - Map metadata = parsed.getMetadata(); - String name = metadata.get("name"); + Map metadata = parsed.getMetadata(); + Object nameObject = metadata.get("name"); + String name = nameObject != null ? String.valueOf(nameObject) : null; if (name == null || name.isEmpty()) { logger.warn("Missing skill name in SKILL.md: {}", skillFile); return Optional.empty(); diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java index 46b29e0c89..49f7a194bd 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java @@ -26,6 +26,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -87,10 +88,13 @@ public static AgentSkill createFrom(String skillMd, Map resource public static AgentSkill createFrom( String skillMd, Map resources, String source) { ParsedMarkdown parsed = MarkdownSkillParser.parse(skillMd); - Map metadata = parsed.getMetadata(); + Map metadata = parsed.getMetadata(); - String name = metadata.get("name"); - String description = metadata.get("description"); + String name = stringifyRequiredMetadata(metadata, "name"); + String description = stringifyRequiredMetadata(metadata, "description"); + metadata = new LinkedHashMap<>(metadata); + metadata.put("name", name); + metadata.put("description", description); String skillContent = parsed.getContent(); if (name == null || name.isEmpty() || description == null || description.isEmpty()) { @@ -103,7 +107,7 @@ public static AgentSkill createFrom( "The SKILL.md must have content except for the YAML Front Matter."); } - return new AgentSkill(name, description, skillContent, resources, source); + return new AgentSkill(metadata, skillContent, resources, source); } /** @@ -298,4 +302,18 @@ private static String readZipEntryContent(ZipInputStream zipInputStream) throws return outputStream.toString(StandardCharsets.UTF_8); } } + + private static String stringifyRequiredMetadata(Map metadata, String key) { + if (metadata == null) { + return null; + } + + Object value = metadata.get(key); + if (value == null) { + return null; + } + + String stringValue = String.valueOf(value); + return stringValue.isEmpty() ? null : stringValue; + } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillTest.java index c6521f55ff..ecf97c3187 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillTest.java @@ -21,7 +21,10 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Set; import org.junit.jupiter.api.DisplayName; @@ -44,10 +47,47 @@ void testConstructorWithAllParameters() { assertEquals("A test skill", skill.getDescription()); assertEquals("Skill content here", skill.getSkillContent()); assertEquals("custom", skill.getSource()); + assertEquals("test_skill", skill.getMetadata().get("name")); + assertEquals("A test skill", skill.getMetadata().get("description")); assertEquals(2, skill.getResources().size()); assertEquals("{\"key\": \"value\"}", skill.getResources().get("config.json")); } + @Test + @DisplayName("Should constructor with explicit metadata") + void testConstructorWithExplicitMetadata() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "browser_skill"); + metadata.put("description", "Use browser automation"); + metadata.put("version", "1.0.0"); + metadata.put("tags", List.of("web", "automation")); + + AgentSkill skill = + new AgentSkill( + metadata, "Browser skill content", Map.of("file.txt", "content"), null); + + assertEquals("browser_skill", skill.getName()); + assertEquals("Use browser automation", skill.getDescription()); + assertEquals("1.0.0", skill.getMetadataValue("version")); + assertEquals(List.of("web", "automation"), skill.getMetadataValue("tags")); + } + + @Test + @DisplayName("Should preserve metadata order") + void testPreserveMetadataOrder() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "trello"); + metadata.put("description", "Manage Trello boards"); + metadata.put("homepage", "https://developer.atlassian.com/cloud/trello/rest/"); + metadata.put("metadata", Map.of("emoji", "📋")); + + AgentSkill skill = new AgentSkill(metadata, "Skill content", null, null); + + assertEquals( + List.of("name", "description", "homepage", "metadata"), + List.copyOf(skill.getMetadata().keySet())); + } + @Test @DisplayName("Should constructor with custom source") void testConstructorWithCustomSource() { @@ -95,6 +135,30 @@ void testConstructorValidatesDescription() { IllegalArgumentException.class, () -> new AgentSkill("name", "", "content", null)); } + @Test + @DisplayName("Should constructor validates metadata") + void testConstructorValidatesMetadata() { + assertThrows( + IllegalArgumentException.class, + () -> new AgentSkill((Map) null, "content", null, null)); + assertThrows( + IllegalArgumentException.class, + () -> new AgentSkill(Map.of("description", "desc"), "content", null, null)); + assertThrows( + IllegalArgumentException.class, + () -> new AgentSkill(Map.of("name", "name"), "content", null, null)); + assertThrows( + IllegalArgumentException.class, + () -> + new AgentSkill( + Map.of("name", 123, "description", "desc"), "content", null, null)); + assertThrows( + IllegalArgumentException.class, + () -> + new AgentSkill( + Map.of("name", "name", "description", 123), "content", null, null)); + } + @Test @DisplayName("Should constructor validates content") void testConstructorValidatesContent() { @@ -137,6 +201,30 @@ void testResourcesImmutability() { assertEquals(1, skill.getResources().size()); } + @Test + @DisplayName("Should metadata immutability") + void testMetadataImmutability() { + Map originalMetadata = new LinkedHashMap<>(); + List tags = new ArrayList<>(); + tags.add("web"); + originalMetadata.put("name", "name"); + originalMetadata.put("description", "desc"); + originalMetadata.put("tags", tags); + + AgentSkill skill = new AgentSkill(originalMetadata, "content", null, null); + + originalMetadata.put("name", "modified"); + originalMetadata.put("newKey", "newValue"); + + assertEquals("name", skill.getName()); + assertEquals("desc", skill.getDescription()); + assertEquals(List.of("web"), skill.getMetadataValue("tags")); + assertEquals(null, skill.getMetadataValue("newKey")); + assertThrows( + UnsupportedOperationException.class, + () -> skill.getMetadata().put("another", "value")); + } + @Test @DisplayName("Should builder create from scratch") void testBuilderCreateFromScratch() { @@ -153,9 +241,31 @@ void testBuilderCreateFromScratch() { assertEquals("Built with builder", skill.getDescription()); assertEquals("Builder content", skill.getSkillContent()); assertEquals("custom_source", skill.getSource()); + assertEquals("builder_skill", skill.getMetadataValue("name")); + assertEquals("Built with builder", skill.getMetadataValue("description")); assertEquals(1, skill.getResources().size()); } + @Test + @DisplayName("Should builder support metadata") + void testBuilderSupportMetadata() { + AgentSkill skill = + AgentSkill.builder() + .metadata( + Map.of( + "name", "builder_skill", + "description", "Built with metadata", + "category", "tooling")) + .putMetadata("version", "1.2.3") + .skillContent("Builder content") + .build(); + + assertEquals("builder_skill", skill.getName()); + assertEquals("Built with metadata", skill.getDescription()); + assertEquals("tooling", skill.getMetadataValue("category")); + assertEquals("1.2.3", skill.getMetadataValue("version")); + } + @Test @DisplayName("Should builder to builder") void testBuilderToBuilder() { @@ -165,11 +275,13 @@ void testBuilderToBuilder() { AgentSkill modified = original.toBuilder() .description("Modified description") + .putMetadata("version", "2.0") .addResource("new.txt", "new content") .build(); assertEquals("original", modified.getName()); // Unchanged assertEquals("Modified description", modified.getDescription()); // Changed + assertEquals("2.0", modified.getMetadataValue("version")); assertEquals(2, modified.getResources().size()); // Added resource } diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java index 7e2dd03e23..45fd5c8c26 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java @@ -210,6 +210,8 @@ void testCreateFromEdgeCases() { + "Content"; AgentSkill skill4 = SkillUtil.createFrom(extraFieldsSkillMd, null); assertEquals("skill", skill4.getName()); + assertEquals("1.0.0", String.valueOf(skill4.getMetadataValue("version"))); + assertEquals("John", skill4.getMetadataValue("author")); } @Test diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/util/MarkdownSkillParserTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/util/MarkdownSkillParserTest.java index 52597615f9..7e8476fafe 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/util/MarkdownSkillParserTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/util/MarkdownSkillParserTest.java @@ -20,10 +20,13 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import io.agentscope.core.skill.util.MarkdownSkillParser.ParsedMarkdown; import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; @@ -51,7 +54,7 @@ void testParseWithValidFrontmatter() { assertNotNull(parsed); assertTrue(parsed.hasFrontmatter()); - Map metadata = parsed.getMetadata(); + Map metadata = parsed.getMetadata(); assertEquals("test_skill", metadata.get("name")); assertEquals("A test skill", metadata.get("description")); assertEquals("1.0.0", metadata.get("version")); @@ -282,35 +285,30 @@ void testParseUnicodeCharacters() { class ErrorHandlingTests { @Test - @DisplayName("Should gracefully ignore invalid YAML lines instead of throwing exception") + @DisplayName("Should return empty metadata for invalid YAML frontmatter") void testInvalidYaml() { String markdown = "---\nname: test\nthis is not a valid line\n---\nContent"; MarkdownSkillParser.ParsedMarkdown parsed = MarkdownSkillParser.parse(markdown); - Map metadata = parsed.getMetadata(); + Map metadata = parsed.getMetadata(); - assertEquals("test", metadata.get("name")); - assertFalse(metadata.containsKey("this is not a valid line")); + assertTrue(metadata.isEmpty()); assertEquals("Content", parsed.getContent()); } @Test - @DisplayName("Should gracefully ignore list format instead of throwing exception") + @DisplayName("Should return empty metadata for invalid list-style frontmatter") void testListFormat() { String markdown = "---\nname: test_skill\n- item1\n- item2\n---\nContent"; MarkdownSkillParser.ParsedMarkdown parsed = MarkdownSkillParser.parse(markdown); - Map metadata = parsed.getMetadata(); + Map metadata = parsed.getMetadata(); - assertEquals("test_skill", metadata.get("name")); - assertFalse(metadata.containsKey("- item1")); - assertFalse(metadata.containsKey("- item2")); + assertTrue(metadata.isEmpty()); } @Test - @DisplayName( - "Should parse basic scalars and gracefully ignore complex YAML structures like" - + " lists or JSON") + @DisplayName("Should keep flat scalar metadata and preserve complex YAML structures") void testParseAndIgnoreComplexMetadata() { String markdown = """ @@ -329,27 +327,24 @@ void testParseAndIgnoreComplexMetadata() { """; MarkdownSkillParser.ParsedMarkdown parsed = MarkdownSkillParser.parse(markdown); - Map metadata = parsed.getMetadata(); + Map metadata = parsed.getMetadata(); assertEquals("Agent Browser", metadata.get("name")); assertEquals( "A fast Rust-based headless browser automation CLI", metadata.get("description")); assertEquals("Bash(agent-browser:*)", metadata.get("allowed-tools")); - - assertEquals("{\"clawdbot\":{\"emoji\":\"🌐\"}}", metadata.get("metadata")); - - assertEquals("", metadata.get("read_when")); - assertNull(metadata.get("- Automating web interactions")); + assertEquals(Map.of("clawdbot", Map.of("emoji", "🌐")), metadata.get("metadata")); + assertEquals( + List.of("Automating web interactions", "Extracting structured data from pages"), + metadata.get("read_when")); assertTrue(parsed.getContent().contains("# Content")); } @Test - @DisplayName( - "Should gracefully skip keys with block-style modifiers (| or >) instead of" - + " recording them as literal values") - void testSkipBlockStyleModifiers() { + @DisplayName("Should parse block-style scalar values") + void testParseBlockStyleModifiers() { String markdown = """ --- @@ -366,19 +361,29 @@ void testSkipBlockStyleModifiers() { """; MarkdownSkillParser.ParsedMarkdown parsed = MarkdownSkillParser.parse(markdown); - Map metadata = parsed.getMetadata(); + Map metadata = parsed.getMetadata(); assertEquals("test_skill", metadata.get("name")); assertEquals("1.0", metadata.get("version")); + assertEquals( + "This is a multi-line description.\n" + + "It should be ignored by the simple parser.\n", + metadata.get("description")); + assertEquals( + "This is a folded multi-line summary. It should also be ignored.\n", + metadata.get("summary")); + } + + @Test + @DisplayName("Should return empty metadata when frontmatter exceeds size limit") + void testFrontmatterSizeLimit() { + String largeValue = "x".repeat(17_000); + String markdown = "---\nname: " + largeValue + "\ndescription: desc\n---\nContent"; - assertNull( - metadata.get("description"), - "Block scalar modifier '|' should not be parsed as a literal value"); - assertNull( - metadata.get("summary"), - "Block scalar modifier '>' should not be parsed as a literal value"); + ParsedMarkdown parsed = MarkdownSkillParser.parse(markdown); - assertFalse(metadata.containsKey(" This is a multi-line description.")); + assertTrue(parsed.getMetadata().isEmpty()); + assertEquals("Content", parsed.getContent()); } } @@ -389,7 +394,7 @@ class GenerateTests { @Test @DisplayName("Should generate with metadata and content") void testGenerateBasic() { - Map metadata = Map.of("name", "test_skill", "description", "Test"); + Map metadata = Map.of("name", "test_skill", "description", "Test"); String content = "# Skill Content"; String generated = MarkdownSkillParser.generate(metadata, content); @@ -425,7 +430,7 @@ void testGenerateNullOrEmpty() { @Test @DisplayName("Should generate with special characters in content") void testGenerateSpecialContent() { - Map metadata = Map.of("name", "special"); + Map metadata = Map.of("name", "special"); String content = "Content with special chars: @#$%^&*(){}[]|\\:;\"'<>?,./"; String generated = MarkdownSkillParser.generate(metadata, content); @@ -436,7 +441,7 @@ void testGenerateSpecialContent() { @Test @DisplayName("Should generate and quote values with special characters") void testGenerateQuotingSpecialChars() { - Map metadata = + Map metadata = Map.of( "colon", "http://example.com:8080", "hash", "#important", @@ -455,7 +460,7 @@ void testGenerateQuotingSpecialChars() { @Test @DisplayName("Should generate and quote values with whitespace") void testGenerateQuotingWhitespace() { - Map metadata = Map.of("leading", " spaces", "trailing", "spaces "); + Map metadata = Map.of("leading", " spaces", "trailing", "spaces "); String generated = MarkdownSkillParser.generate(metadata, "Content"); ParsedMarkdown parsed = MarkdownSkillParser.parse(generated); @@ -467,7 +472,7 @@ void testGenerateQuotingWhitespace() { @Test @DisplayName("Should generate and quote values starting with YAML special chars") void testGenerateQuotingYAMLChars() { - Map metadata = + Map metadata = Map.of( "quote", "\"starts with quote", "bracket", "[array", @@ -498,7 +503,7 @@ void testGenerateQuotingYAMLChars() { @Test @DisplayName("Should generate with empty value") void testGenerateEmptyValue() { - Map metadata = Map.of("empty", ""); + Map metadata = Map.of("empty", ""); String generated = MarkdownSkillParser.generate(metadata, "Content"); ParsedMarkdown parsed = MarkdownSkillParser.parse(generated); @@ -537,7 +542,7 @@ void testRoundTripBasic() { @Test @DisplayName("Should round trip with special characters") void testRoundTripSpecialCharacters() { - Map original = + Map original = Map.of( "url", "http://example.com:8080", "tag", "#important", @@ -552,6 +557,84 @@ void testRoundTripSpecialCharacters() { assertEquals(original.get("path"), parsed.getMetadata().get("path")); assertEquals(original.get("message"), parsed.getMetadata().get("message")); } + + @Test + @DisplayName("Should preserve metadata order for parse and generate") + void testPreserveMetadataOrder() { + String original = + "---\n" + + "name: trello\n" + + "description: Manage Trello boards\n" + + "homepage: https://developer.atlassian.com/cloud/trello/rest/\n" + + "metadata:\n" + + " clawdbot:\n" + + " emoji: 📋\n" + + " requires:\n" + + " bins:\n" + + " - jq\n" + + " env:\n" + + " - TRELLO_API_KEY\n" + + " - TRELLO_TOKEN\n" + + "---\n" + + "Content"; + + ParsedMarkdown parsed = MarkdownSkillParser.parse(original); + + assertEquals( + List.of("name", "description", "homepage", "metadata"), + List.copyOf(parsed.getMetadata().keySet())); + + @SuppressWarnings("unchecked") + Map nestedMetadata = + (Map) parsed.getMetadata().get("metadata"); + assertEquals(List.of("clawdbot"), List.copyOf(nestedMetadata.keySet())); + + @SuppressWarnings("unchecked") + Map clawdbotMetadata = + (Map) nestedMetadata.get("clawdbot"); + assertEquals(List.of("emoji", "requires"), List.copyOf(clawdbotMetadata.keySet())); + + String generated = + MarkdownSkillParser.generate(parsed.getMetadata(), parsed.getContent()); + int nameIndex = generated.indexOf("name: trello"); + int descriptionIndex = generated.indexOf("description: Manage Trello boards"); + int homepageIndex = + generated.indexOf( + "homepage: https://developer.atlassian.com/cloud/trello/rest/"); + int metadataIndex = generated.indexOf("metadata:"); + + assertTrue(nameIndex < descriptionIndex); + assertTrue(descriptionIndex < homepageIndex); + assertTrue(homepageIndex < metadataIndex); + } + + @Test + @DisplayName("Should keep generated document unchanged after parse and regenerate") + void testParseThenGenerateKeepsDocumentStable() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "trello"); + metadata.put("description", "Manage Trello boards, lists, and cards."); + metadata.put("homepage", "https://developer.atlassian.com/cloud/trello/rest/"); + metadata.put( + "metadata", + Map.of( + "clawdbot", + Map.of( + "emoji", + "📋", + "requires", + Map.of( + "bins", List.of("jq"), + "env", List.of("TRELLO_API_KEY", "TRELLO_TOKEN"))))); + + String original = MarkdownSkillParser.generate(metadata, "# Content\nBody"); + + ParsedMarkdown parsed = MarkdownSkillParser.parse(original); + String regenerated = + MarkdownSkillParser.generate(parsed.getMetadata(), parsed.getContent()); + + assertEquals(original, regenerated); + } } @Nested @@ -561,7 +644,7 @@ class ParsedMarkdownTests { @Test @DisplayName("Should provide correct getters") void testGetters() { - Map metadata = Map.of("key", "value"); + Map metadata = Map.of("key", "value"); ParsedMarkdown parsed = new ParsedMarkdown(metadata, "content"); assertEquals("value", parsed.getMetadata().get("key")); @@ -572,7 +655,7 @@ void testGetters() { @Test @DisplayName("Should maintain immutability") void testImmutability() { - Map originalMetadata = new HashMap<>(); + Map originalMetadata = new HashMap<>(); originalMetadata.put("key", "value"); ParsedMarkdown parsed = new ParsedMarkdown(originalMetadata, "content"); @@ -598,7 +681,7 @@ void testNullInputs() { @Test @DisplayName("Should provide meaningful toString") void testToString() { - Map metadata = Map.of("name", "test"); + Map metadata = Map.of("name", "test"); String content = "This is a very long content that should be truncated in toString"; ParsedMarkdown parsed = new ParsedMarkdown(metadata, content); @@ -608,5 +691,15 @@ void testToString() { assertTrue(toString.contains("metadata")); assertTrue(toString.contains("content")); } + + @Test + @DisplayName("Should keep metadata immutable") + void testMetadataImmutable() { + ParsedMarkdown parsed = new ParsedMarkdown(Map.of("name", "test"), "content"); + + assertThrows( + UnsupportedOperationException.class, + () -> parsed.getMetadata().put("description", "desc")); + } } } From 750b6e502443e58c71018b428010b775db204211 Mon Sep 17 00:00:00 2001 From: fang-tech Date: Fri, 24 Apr 2026 00:51:11 +0800 Subject: [PATCH 2/5] refactor(skill): render skill metadata as xml prompt --- .../core/skill/AgentSkillPromptProvider.java | 136 ++++++++--- .../io/agentscope/core/skill/SkillBox.java | 31 +-- .../skill/AgentSkillPromptProviderTest.java | 214 +++++++++--------- .../agentscope/core/skill/SkillBoxTest.java | 21 ++ docs/en/task/agent-skill.md | 31 ++- docs/zh/task/agent-skill.md | 30 ++- 6 files changed, 297 insertions(+), 166 deletions(-) diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java b/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java index a7e6a66255..cf18317d6a 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java @@ -16,6 +16,10 @@ package io.agentscope.core.skill; import java.nio.file.Path; +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.regex.Pattern; /** * Generates skill system prompts for agents to understand available skills. @@ -30,9 +34,12 @@ * } */ public class AgentSkillPromptProvider { + private static final String INDENT = " "; + private static final Pattern XML_TAG_NAME_PATTERN = Pattern.compile("[A-Za-z_][A-Za-z0-9_.-]*"); + private final SkillRegistry skillRegistry; private final String instruction; - private final String template; + private boolean exposeAllMetadata = true; private boolean codeExecutionEnabled; private String uploadDir; private String codeExecutionInstruction; @@ -54,10 +61,11 @@ public class AgentSkillPromptProvider { 2. Load it: load_skill_through_path(skillId="data-analysis_builtin", path="SKILL.md") 3. Follow the instructions returned by the skill - Template fields explanation: - - : The skill's display name - - : When and how to use this skill - - : Unique identifier for load_skill_through_path tool + Metadata is rendered as XML under each element: + - scalar metadata becomes a simple child element + - nested maps become nested XML elements + - lists become repeated elements + - is always appended for tool loading @@ -99,41 +107,27 @@ public class AgentSkillPromptProvider { """; - // skillName, skillDescription, skillId - public static final String DEFAULT_AGENT_SKILL_TEMPLATE = - """ - - %s - %s - %s - - - """; - /** * Creates a skill prompt provider. * * @param registry The skill registry containing registered skills */ public AgentSkillPromptProvider(SkillRegistry registry) { - this(registry, null, null); + this(registry, null); } /** - * Creates a skill prompt provider with custom instruction and template. + * Creates a skill prompt provider with custom instruction. * * @param registry The skill registry containing registered skills * @param instruction Custom instruction header (null or blank uses default) - * @param template Custom skill template (null or blank uses default) */ - public AgentSkillPromptProvider(SkillRegistry registry, String instruction, String template) { + public AgentSkillPromptProvider(SkillRegistry registry, String instruction) { this.skillRegistry = registry; this.instruction = instruction == null || instruction.isBlank() ? DEFAULT_AGENT_SKILL_INSTRUCTION : instruction; - this.template = - template == null || template.isBlank() ? DEFAULT_AGENT_SKILL_TEMPLATE : template; } /** @@ -144,28 +138,20 @@ public AgentSkillPromptProvider(SkillRegistry registry, String instruction, Stri * @return The skill system prompt, or empty string if no skills exist */ public String getSkillSystemPrompt() { - StringBuilder sb = new StringBuilder(); - - // Check if there are any skills if (skillRegistry.getAllRegisteredSkills().isEmpty()) { return ""; } - // Add instruction header + StringBuilder sb = new StringBuilder(); sb.append(instruction); - // Add each skill for (RegisteredSkill registered : skillRegistry.getAllRegisteredSkills().values()) { AgentSkill skill = skillRegistry.getSkill(registered.getSkillId()); - sb.append( - String.format( - template, skill.getName(), skill.getDescription(), skill.getSkillId())); + appendSkill(sb, skill); } - // Close available_skills tag sb.append(""); - // Conditionally append code execution instructions if (codeExecutionEnabled && uploadDir != null) { String template = codeExecutionInstruction != null @@ -211,4 +197,90 @@ public void setCodeExecutionInstruction(String codeExecutionInstruction) { ? null : codeExecutionInstruction; } + + /** + * Sets whether all metadata fields are exposed to the LLM. + * + *

    When disabled, only {@code name}, {@code description}, and {@code skill-id} + * are rendered into the skill prompt. + * + * @param exposeAllMetadata {@code true} to expose all metadata, {@code false} to expose only + * the core fields + */ + public void setExposeAllMetadata(boolean exposeAllMetadata) { + this.exposeAllMetadata = exposeAllMetadata; + } + + private void appendSkill(StringBuilder sb, AgentSkill skill) { + sb.append("\n"); + for (Map.Entry entry : getPromptMetadata(skill).entrySet()) { + appendXmlNode(sb, entry.getKey(), entry.getValue(), 1); + } + appendXmlNode(sb, "skill-id", skill.getSkillId(), 1); + sb.append("\n\n"); + } + + private Map getPromptMetadata(AgentSkill skill) { + if (exposeAllMetadata) { + return skill.getMetadata(); + } + + LinkedHashMap metadata = new LinkedHashMap<>(); + metadata.put("name", skill.getName()); + metadata.put("description", skill.getDescription()); + return metadata; + } + + private void appendXmlNode(StringBuilder sb, String key, Object value, int indentLevel) { + String indent = INDENT.repeat(indentLevel); + boolean validTagName = isValidXmlTagName(key); + String openTag = validTagName ? "<" + key + ">" : ""; + String closeTag = validTagName ? "" : ""; + + if (isScalarValue(value)) { + sb.append(indent) + .append(openTag) + .append(escapeXml(String.valueOf(value))) + .append(closeTag) + .append("\n"); + return; + } + + sb.append(indent).append(openTag).append("\n"); + if (value instanceof Map mapValue) { + for (Map.Entry entry : mapValue.entrySet()) { + appendXmlNode( + sb, String.valueOf(entry.getKey()), entry.getValue(), indentLevel + 1); + } + } else if (value instanceof Collection collectionValue) { + for (Object item : collectionValue) { + appendXmlNode(sb, "item", item, indentLevel + 1); + } + } else { + sb.append(INDENT.repeat(indentLevel + 1)) + .append(escapeXml(String.valueOf(value))) + .append("\n"); + } + sb.append(indent).append(closeTag).append("\n"); + } + + private boolean isScalarValue(Object value) { + return value == null + || (!(value instanceof Map) && !(value instanceof Collection)); + } + + private boolean isValidXmlTagName(String value) { + return value != null && XML_TAG_NAME_PATTERN.matcher(value).matches(); + } + + private String escapeXml(String value) { + if (value == null) { + return ""; + } + return value.replace("&", "&") + .replace("<", "<") + .replace(">", ">") + .replace("\"", """) + .replace("'", "'"); + } } diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java index 33f6d007f5..0c12f8f9db 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java @@ -55,17 +55,7 @@ public class SkillBox implements StateModule { private boolean autoUploadSkill = true; public SkillBox(Toolkit toolkit) { - this(toolkit, null, null); - } - - /** - * Creates a SkillBox with custom skill prompt instruction and template. - * - * @param instruction Custom instruction header (null or blank uses default) - * @param template Custom skill template (null or blank uses default) - */ - public SkillBox(String instruction, String template) { - this(null, instruction, template); + this(toolkit, null); } /** @@ -73,11 +63,9 @@ public SkillBox(String instruction, String template) { * * @param toolkit The toolkit to bind * @param instruction Custom instruction header (null or blank uses default) - * @param template Custom skill template (null or blank uses default) */ - public SkillBox(Toolkit toolkit, String instruction, String template) { - this.skillPromptProvider = - new AgentSkillPromptProvider(skillRegistry, instruction, template); + public SkillBox(Toolkit toolkit, String instruction) { + this.skillPromptProvider = new AgentSkillPromptProvider(skillRegistry, instruction); this.skillToolFactory = new SkillToolFactory(skillRegistry, toolkit); this.toolkit = toolkit; } @@ -94,6 +82,19 @@ public String getSkillPrompt() { return skillPromptProvider.getSkillSystemPrompt(); } + /** + * Controls whether the skill prompt exposes all metadata fields or only the core fields. + * + *

    When disabled, only {@code name}, {@code description}, and {@code skill-id} + * are included in the skill prompt. + * + * @param exposeAllMetadata {@code true} to expose all metadata, {@code false} to expose only + * the core fields + */ + public void setExposeAllSkillMetadata(boolean exposeAllMetadata) { + skillPromptProvider.setExposeAllMetadata(exposeAllMetadata); + } + /** * Create a fluent builder for registering skills with optional configuration. * diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillPromptProviderTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillPromptProviderTest.java index 389e2d1da9..62faa21445 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillPromptProviderTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillPromptProviderTest.java @@ -21,6 +21,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.nio.file.Path; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -46,88 +49,145 @@ void testNoSkillsReturnsEmpty() { } @Test - @DisplayName("Should generate prompt for single skill") + @DisplayName("Should render metadata as XML for single skill") void testSingleSkill() { AgentSkill skill = new AgentSkill("test_skill", "Test Skill Description", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); + skillRegistry.registerSkill( + "test_skill_custom", skill, new RegisteredSkill("test_skill_custom")); String prompt = provider.getSkillSystemPrompt(); assertTrue(prompt.contains("## Available Skills")); assertTrue(prompt.contains("")); assertTrue(prompt.contains("")); - assertTrue(prompt.contains("test_skill_custom")); assertTrue(prompt.contains("test_skill")); assertTrue(prompt.contains("Test Skill Description")); + assertTrue(prompt.contains("test_skill_custom")); assertTrue(prompt.contains("")); assertTrue(prompt.contains("")); } @Test - @DisplayName("Should generate prompt for multiple skills") - void testMultipleSkills() { - AgentSkill skill1 = new AgentSkill("skill1", "First Skill", "# Content1", null); - RegisteredSkill registered1 = new RegisteredSkill("skill1_custom"); - skillRegistry.registerSkill("skill1_custom", skill1, registered1); + @DisplayName("Should preserve metadata order in XML output") + void testMetadataOrder() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "trello"); + metadata.put("description", "Manage Trello boards"); + metadata.put("homepage", "https://developer.atlassian.com/cloud/trello/rest/"); + AgentSkill skill = new AgentSkill(metadata, "# Content", null, null); + skillRegistry.registerSkill("trello_custom", skill, new RegisteredSkill("trello_custom")); + + String prompt = provider.getSkillSystemPrompt(); - AgentSkill skill2 = new AgentSkill("skill2", "Second Skill", "# Content2", null); - RegisteredSkill registered2 = new RegisteredSkill("skill2_custom"); - skillRegistry.registerSkill("skill2_custom", skill2, registered2); + int nameIndex = prompt.indexOf("trello"); + int descriptionIndex = prompt.indexOf("Manage Trello boards"); + int homepageIndex = + prompt.indexOf( + "https://developer.atlassian.com/cloud/trello/rest/"); + int skillIdIndex = prompt.indexOf("trello_custom"); + + assertTrue(nameIndex < descriptionIndex); + assertTrue(descriptionIndex < homepageIndex); + assertTrue(homepageIndex < skillIdIndex); + } + + @Test + @DisplayName("Should render nested metadata as nested XML") + void testNestedMetadataXml() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "trello"); + metadata.put("description", "Manage Trello boards"); + metadata.put( + "metadata", + Map.of( + "clawdbot", + Map.of( + "emoji", + "📋", + "requires", + Map.of( + "bins", List.of("jq"), + "env", List.of("TRELLO_API_KEY", "TRELLO_TOKEN"))))); + + AgentSkill skill = new AgentSkill(metadata, "# Content", null, null); + skillRegistry.registerSkill("trello_custom", skill, new RegisteredSkill("trello_custom")); String prompt = provider.getSkillSystemPrompt(); - assertTrue(prompt.contains("## Available Skills")); - assertTrue(prompt.contains("skill1_custom")); - assertTrue(prompt.contains("skill1")); - assertTrue(prompt.contains("First Skill")); - assertTrue(prompt.contains("skill2_custom")); - assertTrue(prompt.contains("skill2")); - assertTrue(prompt.contains("Second Skill")); + assertTrue(prompt.contains("")); + assertTrue(prompt.contains("")); + assertTrue(prompt.contains("📋")); + assertTrue(prompt.contains("")); + assertTrue(prompt.contains("")); + assertTrue(prompt.contains("jq")); + assertTrue(prompt.contains("")); + assertTrue(prompt.contains("TRELLO_API_KEY")); + assertTrue(prompt.contains("TRELLO_TOKEN")); } @Test - @DisplayName("Should generate correct prompt format") - void testPromptFormat() { - AgentSkill skill = new AgentSkill("test_skill", "Test Description", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); + @DisplayName("Should expose only name and description when all metadata is disabled") + void testExposeOnlyCoreMetadata() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "trello"); + metadata.put("description", "Manage Trello boards"); + metadata.put("homepage", "https://developer.atlassian.com/cloud/trello/rest/"); + metadata.put("metadata", Map.of("clawdbot", Map.of("emoji", "📋"))); + AgentSkill skill = new AgentSkill(metadata, "# Content", null, null); + skillRegistry.registerSkill("trello_custom", skill, new RegisteredSkill("trello_custom")); + + provider.setExposeAllMetadata(false); String prompt = provider.getSkillSystemPrompt(); - assertTrue(prompt.startsWith("## Available Skills\n")); - assertTrue(prompt.contains("specialized capabilities")); - assertTrue(prompt.contains("load_skill_through_path")); - assertTrue(prompt.contains("")); - assertTrue(prompt.contains("")); - assertTrue(prompt.contains("")); - assertTrue(prompt.contains("")); + assertTrue(prompt.contains("trello")); + assertTrue(prompt.contains("Manage Trello boards")); + assertTrue(prompt.contains("trello_custom")); + assertFalse(prompt.contains("")); + assertFalse(prompt.contains("")); } @Test - @DisplayName("Should handle skills with special characters in description") - void testSpecialCharactersInDescription() { - AgentSkill skill = - new AgentSkill( - "test_skill", - "Description with \"quotes\" and 'apostrophes'", - "# Content", - null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); + @DisplayName("Should escape special characters in XML") + void testXmlEscaping() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "test_skill"); + metadata.put("description", "Description with & \"quotes\" and 'apostrophes'"); + AgentSkill skill = new AgentSkill(metadata, "# Content", null, null); + skillRegistry.registerSkill( + "test_skill_custom", skill, new RegisteredSkill("test_skill_custom")); String prompt = provider.getSkillSystemPrompt(); - assertTrue(prompt.contains("Description with \"quotes\" and 'apostrophes'")); + assertTrue( + prompt.contains( + "Description with <xml> & "quotes" and" + + " 'apostrophes'")); + } + + @Test + @DisplayName("Should fallback to entry tag for invalid metadata keys") + void testInvalidMetadataKeyFallback() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "test_skill"); + metadata.put("description", "desc"); + metadata.put("tool:config", "enabled"); + AgentSkill skill = new AgentSkill(metadata, "# Content", null, null); + skillRegistry.registerSkill( + "test_skill_custom", skill, new RegisteredSkill("test_skill_custom")); + + String prompt = provider.getSkillSystemPrompt(); + + assertTrue(prompt.contains("enabled")); } @Test @DisplayName("Should not include code execution section when not enabled") void testNoCodeExecutionSectionByDefault() { AgentSkill skill = new AgentSkill("test_skill", "Test Skill", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); + skillRegistry.registerSkill( + "test_skill_custom", skill, new RegisteredSkill("test_skill_custom")); String prompt = provider.getSkillSystemPrompt(); @@ -139,11 +199,10 @@ void testNoCodeExecutionSectionByDefault() { @DisplayName("Should not include code execution section when enabled but uploadDir not set") void testNoCodeExecutionSectionWhenEnabledButNoUploadDir() { AgentSkill skill = new AgentSkill("test_skill", "Test Skill", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); + skillRegistry.registerSkill( + "test_skill_custom", skill, new RegisteredSkill("test_skill_custom")); provider.setCodeExecutionEnable(true); - // uploadDir not set String prompt = provider.getSkillSystemPrompt(); @@ -155,8 +214,8 @@ void testNoCodeExecutionSectionWhenEnabledButNoUploadDir() { @DisplayName("Should include code execution section with uploadDir when enabled") void testCodeExecutionSectionIncludedWhenEnabled(@TempDir Path tempDir) { AgentSkill skill = new AgentSkill("test_skill", "Test Skill", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); + skillRegistry.registerSkill( + "test_skill_custom", skill, new RegisteredSkill("test_skill_custom")); provider.setCodeExecutionEnable(true); provider.setUploadDir(tempDir); @@ -169,45 +228,12 @@ void testCodeExecutionSectionIncludedWhenEnabled(@TempDir Path tempDir) { assertTrue(prompt.contains(tempDir.toAbsolutePath().toString())); } - @Test - @DisplayName("Should include skill-id based path pattern in code execution section") - void testCodeExecutionSectionContainsSkillIdPattern(@TempDir Path tempDir) { - AgentSkill skill = new AgentSkill("test_skill", "Test Skill", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); - - provider.setCodeExecutionEnable(true); - provider.setUploadDir(tempDir); - - String prompt = provider.getSkillSystemPrompt(); - String uploadDirStr = tempDir.toAbsolutePath().toString(); - - assertTrue(prompt.contains(uploadDirStr + "//scripts/")); - assertTrue(prompt.contains(uploadDirStr + "//assets/")); - } - - @Test - @DisplayName("Should include absolute path instruction in code execution section") - void testCodeExecutionSectionMentionsAbsolutePaths(@TempDir Path tempDir) { - AgentSkill skill = new AgentSkill("test_skill", "Test Skill", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); - - provider.setCodeExecutionEnable(true); - provider.setUploadDir(tempDir); - - String prompt = provider.getSkillSystemPrompt(); - - assertTrue(prompt.contains("absolute paths")); - assertTrue(prompt.contains("existing script")); - } - @Test @DisplayName("Code execution section should appear after ") void testCodeExecutionSectionAppearsAfterAvailableSkills(@TempDir Path tempDir) { AgentSkill skill = new AgentSkill("test_skill", "Test Skill", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); + skillRegistry.registerSkill( + "test_skill_custom", skill, new RegisteredSkill("test_skill_custom")); provider.setCodeExecutionEnable(true); provider.setUploadDir(tempDir); @@ -223,8 +249,8 @@ void testCodeExecutionSectionAppearsAfterAvailableSkills(@TempDir Path tempDir) @DisplayName("Should use custom code execution instruction when set") void testCustomCodeExecutionInstruction(@TempDir Path tempDir) { AgentSkill skill = new AgentSkill("test_skill", "Test Skill", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); + skillRegistry.registerSkill( + "test_skill_custom", skill, new RegisteredSkill("test_skill_custom")); provider.setCodeExecutionEnable(true); provider.setUploadDir(tempDir); @@ -236,20 +262,4 @@ void testCustomCodeExecutionInstruction(@TempDir Path tempDir) { assertTrue(prompt.contains("Skills dir: " + tempDir.toAbsolutePath())); assertFalse(prompt.contains("## Code Execution")); } - - @Test - @DisplayName("Should fall back to default instruction when null is set") - void testNullCodeExecutionInstructionUsesDefault(@TempDir Path tempDir) { - AgentSkill skill = new AgentSkill("test_skill", "Test Skill", "# Content", null); - RegisteredSkill registered = new RegisteredSkill("test_skill_custom"); - skillRegistry.registerSkill("test_skill_custom", skill, registered); - - provider.setCodeExecutionEnable(true); - provider.setUploadDir(tempDir); - provider.setCodeExecutionInstruction(null); - - String prompt = provider.getSkillSystemPrompt(); - - assertTrue(prompt.contains("## Code Execution")); - } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxTest.java index cc97eb46d2..f87e3f24ae 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillBoxTest.java @@ -1067,6 +1067,27 @@ void testCodeExecutionSectionOrderInPrompt() { assertTrue(codeExecutionStart >= 0); assertTrue(availableSkillsEnd < codeExecutionStart); } + + @Test + @DisplayName("Should expose only core skill metadata when configured") + void testExposeOnlyCoreSkillMetadata() { + Map metadata = new java.util.LinkedHashMap<>(); + metadata.put("name", "trello"); + metadata.put("description", "Manage Trello boards"); + metadata.put("homepage", "https://developer.atlassian.com/cloud/trello/rest/"); + metadata.put("metadata", Map.of("clawdbot", Map.of("emoji", "📋"))); + AgentSkill skill = new AgentSkill(metadata, "# Content", null, null); + skillBox.registerSkill(skill); + skillBox.setExposeAllSkillMetadata(false); + + String prompt = skillBox.getSkillPrompt(); + + assertTrue(prompt.contains("trello")); + assertTrue(prompt.contains("Manage Trello boards")); + assertTrue(prompt.contains("trello_custom")); + assertFalse(prompt.contains("")); + assertFalse(prompt.contains("")); + } } /** diff --git a/docs/en/task/agent-skill.md b/docs/en/task/agent-skill.md index cdce7c4a54..f005bbe2e6 100644 --- a/docs/en/task/agent-skill.md +++ b/docs/en/task/agent-skill.md @@ -59,6 +59,12 @@ skill-name/ --- name: skill-name # Required: Skill name (lowercase letters, numbers, underscores) description: This skill should be used when... # Required: Trigger description, explaining when to use +homepage: https://example.com/docs # Optional: Additional metadata exposed to the agent prompt +metadata: + clawdbot: + requires: + env: + - API_KEY --- # Skill Name @@ -79,6 +85,13 @@ description: This skill should be used when... # Required: Trigger description, - `name` - Skill name (lowercase letters, numbers, underscores) - `description` - Skill functionality and usage scenarios, helps AI determine when to use +**Metadata Notes:** + +- Any additional YAML frontmatter fields are preserved as skill metadata, not limited to predefined fields +- Nested maps and lists are supported and keep their structure and insertion order +- Frontmatter is parsed with SnakeYAML `SafeConstructor`; only top-level YAML objects of type `Map` are accepted +- Invalid frontmatter or frontmatter exceeding the parser limit is ignored and treated as empty metadata + ## Quick Start ### 1. Create a Skill @@ -89,6 +102,7 @@ description: This skill should be used when... # Required: Trigger description, AgentSkill skill = AgentSkill.builder() .name("data_analysis") .description("Use when analyzing data...") + .putMetadata("homepage", "https://example.com/docs") .skillContent("# Data Analysis\n...") .addResource("references/formulas.md", "# Common Formulas\n...") .source("custom") @@ -341,31 +355,31 @@ try (NacosSkillRepository repository = new NacosSkillRepository(aiService, "name ### Feature 4: Custom Skill Prompts -When SkillBox injects a system prompt into the Agent, it generates a description entry for each registered Skill so the LLM can decide when to load which Skill. The two components of this prompt can be customized via the constructor: +When SkillBox injects a system prompt into the Agent, it generates one XML `` entry per registered Skill so the LLM can decide when to load which Skill. Metadata is rendered directly from `AgentSkill.getMetadata()`, and `` is always appended for tool loading. - **`instruction`**: The prompt header, explaining how to use Skills (how to load them, path conventions, etc.). Defaults to a built-in `load_skill_through_path` usage guide -- **`template`**: The format template for each Skill entry, containing three `%s` placeholders corresponding to `name`, `description`, and `skillId` in order +- **XML metadata rendering**: Scalar metadata becomes a child element, nested maps become nested XML, and lists become repeated `` elements +- **Metadata exposure control**: `skillBox.setExposeAllSkillMetadata(false)` limits the prompt to `name`, `description`, and `skill-id`; the default is to expose all metadata fields When code execution is enabled, the section appended after `` can also be customized via `.codeExecutionInstruction()`: - **`codeExecutionInstruction`**: Template for the code execution section; every `%s` placeholder will be replaced with the `uploadDir` absolute path. Passing `null` or blank uses the built-in default. -Passing `null` or a blank string for any of these uses the built-in default. +Passing `null` or a blank string for `instruction` or `codeExecutionInstruction` uses the built-in default. **Example**: ```java -// Customize instruction and template +// Customize the instruction header String customInstruction = """ ## Available Skills When a task matches a skill, load it with load_skill_through_path. """; -String customTemplate = """ - - **%s**: %s (id: %s) - """; +SkillBox skillBox = new SkillBox(toolkit, customInstruction); -SkillBox skillBox = new SkillBox(toolkit, customInstruction, customTemplate); +// Optionally expose only core metadata fields in the prompt +skillBox.setExposeAllSkillMetadata(false); // Customize the code execution section (takes effect when code execution is enabled) skillBox.codeExecution() @@ -391,4 +405,3 @@ skillBox.codeExecution() - [Claude Agent Skills Official Documentation](https://platform.claude.com/docs/zh-CN/agents-and-tools/agent-skills/overview) - Complete concept and architecture introduction - [Tool Usage Guide](./tool.md) - Tool system usage methods - [Agent Configuration](./agent.md) - Agent configuration and usage - diff --git a/docs/zh/task/agent-skill.md b/docs/zh/task/agent-skill.md index c7ff76b722..7c9c45ec26 100644 --- a/docs/zh/task/agent-skill.md +++ b/docs/zh/task/agent-skill.md @@ -59,6 +59,12 @@ skill-name/ --- name: skill-name # 必需: 技能名称(小写字母、数字、下划线) description: This skill should be used when... # 必需: 触发描述,说明何时使用 +homepage: https://example.com/docs # 可选: 额外 metadata,会暴露到智能体提示词中 +metadata: + clawdbot: + requires: + env: + - API_KEY --- # 技能名称 @@ -79,6 +85,13 @@ description: This skill should be used when... # 必需: 触发描述,说明何 - `name` - 技能的名字(小写字母、数字、下划线) - `description` - 技能功能和使用场景描述,帮助 AI 判断何时使用 +**Metadata 说明:** + +- YAML frontmatter 中除 `name`、`description` 外的字段都会作为 Skill metadata 保留,不再局限于固定字段 +- 支持嵌套 `Map/List`,并保留原有层级结构和插入顺序 +- frontmatter 使用 SnakeYAML `SafeConstructor` 解析,只接受顶层为 `Map` 的 YAML 对象 +- 非法 frontmatter 或超过解析器限制的 frontmatter 会被忽略,并按空 metadata 处理 + ## 快速开始 ### 1. 创建 Skill @@ -89,6 +102,7 @@ description: This skill should be used when... # 必需: 触发描述,说明何 AgentSkill skill = AgentSkill.builder() .name("data_analysis") .description("Use when analyzing data...") + .putMetadata("homepage", "https://example.com/docs") .skillContent("# Data Analysis\n...") .addResource("references/formulas.md", "# 常用公式\n...") .source("custom") @@ -335,31 +349,31 @@ try (NacosSkillRepository repository = new NacosSkillRepository(aiService, "name ### 功能 4: 自定义 Skill 提示词 -SkillBox 在注入给 Agent 的系统提示词中,会为每个已注册的 Skill 生成描述信息,供 LLM 判断何时加载哪个 Skill。通过构造函数可自定义提示词的两个组成部分: +SkillBox 在注入给 Agent 的系统提示词中,会为每个已注册的 Skill 生成一个 XML `` 条目,供 LLM 判断何时加载哪个 Skill。metadata 直接来自 `AgentSkill.getMetadata()`,并始终追加 `` 作为工具加载标识。 - **`instruction`**: 提示词头部,说明 Skill 的使用方式(如何加载、路径约定等)。默认包含 `load_skill_through_path` 的调用说明 -- **`template`**: 每个 Skill 条目的格式模板,包含三个 `%s` 占位符,依次对应 `name`、`description`、`skillId` +- **XML metadata 渲染**: 标量会渲染为子节点,嵌套 `Map` 会递归渲染为嵌套 XML,列表会渲染为重复的 `` 节点 +- **metadata 暴露控制**: `skillBox.setExposeAllSkillMetadata(false)` 可将提示词限制为只暴露 `name`、`description` 和 `skill-id`;默认暴露全部 metadata 开启代码执行后,还可通过 `.codeExecutionInstruction()` 自定义追加在 `` 之后的代码执行说明段落: - **`codeExecutionInstruction`**: 代码执行说明模板,所有 `%s` 占位符都会被替换为 `uploadDir` 的绝对路径。传 `null` 或空字符串时使用内置默认值 -三者传 `null` 或空字符串时均使用内置默认值。 +`instruction` 和 `codeExecutionInstruction` 传 `null` 或空字符串时均使用内置默认值。 **示例代码**: ```java -// 自定义 instruction 和 template +// 自定义 instruction 头部 String customInstruction = """ ## 可用技能 当任务匹配某个技能时,使用 load_skill_through_path 加载它。 """; -String customTemplate = """ - - **%s**: %s (id: %s) - """; +SkillBox skillBox = new SkillBox(toolkit, customInstruction); -SkillBox skillBox = new SkillBox(toolkit, customInstruction, customTemplate); +// 可选: 仅向 prompt 暴露核心 metadata +skillBox.setExposeAllSkillMetadata(false); // 自定义代码执行说明(开启代码执行后生效) skillBox.codeExecution() From d217125ea215eb1478a4c7015c7455080c3aa71c Mon Sep 17 00:00:00 2001 From: fang-tech Date: Fri, 24 Apr 2026 00:55:04 +0800 Subject: [PATCH 3/5] fix(skill): persist filesystem metadata round-trip --- .../skill/util/SkillFileSystemHelper.java | 7 +-- .../FileSystemSkillRepositoryTest.java | 31 ++++++++++++ .../skill/util/SkillFileSystemHelperTest.java | 48 +++++++++++++++++++ 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillFileSystemHelper.java b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillFileSystemHelper.java index 83091bf46b..daf0b8f56f 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillFileSystemHelper.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillFileSystemHelper.java @@ -28,7 +28,6 @@ import java.util.Base64; import java.util.Comparator; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -193,12 +192,8 @@ public static boolean saveSkills(Path baseDir, List skills, boolean Files.createDirectories(skillDir); - Map metadata = new LinkedHashMap<>(); - metadata.put("name", skill.getName()); - metadata.put("description", skill.getDescription()); - String skillMdContent = - MarkdownSkillParser.generate(metadata, skill.getSkillContent()); + MarkdownSkillParser.generate(skill.getMetadata(), skill.getSkillContent()); Path skillFile = skillDir.resolve(SKILL_FILE_NAME); Files.writeString(skillFile, skillMdContent, StandardCharsets.UTF_8); diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/repository/FileSystemSkillRepositoryTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/repository/FileSystemSkillRepositoryTest.java index bee6919a76..d0ae03f65d 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/repository/FileSystemSkillRepositoryTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/repository/FileSystemSkillRepositoryTest.java @@ -27,6 +27,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import org.junit.jupiter.api.AfterEach; @@ -267,6 +268,36 @@ void testSave_WithResources() { assertEquals("{\"key\": \"value\"}", loaded.getResources().get("config.json")); } + @Test + @DisplayName("Should round trip full metadata through filesystem repository") + void testSave_RoundTripFullMetadata() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "repo-metadata-skill"); + metadata.put("description", "Repository Metadata Skill"); + metadata.put("homepage", "https://example.com/repo"); + metadata.put( + "metadata", + Map.of( + "clawdbot", + Map.of("emoji", "📋", "requires", Map.of("env", List.of("API_KEY"))))); + AgentSkill skill = new AgentSkill(metadata, "Repository content", null, null); + + assertTrue(repository.save(List.of(skill), false)); + + AgentSkill loaded = repository.getSkill("repo-metadata-skill"); + assertEquals(List.copyOf(metadata.keySet()), List.copyOf(loaded.getMetadata().keySet())); + assertEquals("https://example.com/repo", loaded.getMetadataValue("homepage")); + + @SuppressWarnings("unchecked") + Map loadedMetadata = + (Map) loaded.getMetadataValue("metadata"); + @SuppressWarnings("unchecked") + Map clawdbot = (Map) loadedMetadata.get("clawdbot"); + + assertEquals("📋", clawdbot.get("emoji")); + assertEquals(Map.of("env", List.of("API_KEY")), clawdbot.get("requires")); + } + @Test @DisplayName("Should fail to save existing skill without force") void testSave_ExistingWithoutForce() { diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/util/SkillFileSystemHelperTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/util/SkillFileSystemHelperTest.java index 2282c367e8..21c4da5e0c 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/util/SkillFileSystemHelperTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/util/SkillFileSystemHelperTest.java @@ -29,6 +29,7 @@ import java.nio.file.Path; import java.util.Base64; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import org.junit.jupiter.api.BeforeEach; @@ -128,6 +129,53 @@ void testSaveSkills_NewSkill() { assertEquals(1, loaded.getResources().size()); } + @Test + @DisplayName("Should preserve full metadata when saving and loading") + void testSaveSkills_PreservesFullMetadata() throws IOException { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "metadata-skill"); + metadata.put("description", "Metadata Skill"); + metadata.put("homepage", "https://example.com/docs"); + metadata.put( + "metadata", + Map.of( + "clawdbot", + Map.of( + "requires", + Map.of( + "env", List.of("API_KEY", "API_SECRET"), + "bins", List.of("jq"))))); + AgentSkill skill = + new AgentSkill(metadata, "Content", Map.of("references/doc.md", "Doc"), null); + + boolean result = SkillFileSystemHelper.saveSkills(skillsBaseDir, List.of(skill), false); + assertTrue(result); + + String savedSkillMd = + Files.readString( + skillsBaseDir.resolve("metadata-skill").resolve("SKILL.md"), + StandardCharsets.UTF_8); + assertTrue(savedSkillMd.contains("homepage: https://example.com/docs")); + assertTrue(savedSkillMd.contains("metadata:")); + assertTrue(savedSkillMd.contains("clawdbot:")); + + AgentSkill loaded = + SkillFileSystemHelper.loadSkill(skillsBaseDir, "metadata-skill", "source"); + assertEquals(List.copyOf(metadata.keySet()), List.copyOf(loaded.getMetadata().keySet())); + assertEquals("https://example.com/docs", loaded.getMetadataValue("homepage")); + + @SuppressWarnings("unchecked") + Map loadedMetadata = + (Map) loaded.getMetadataValue("metadata"); + @SuppressWarnings("unchecked") + Map clawdbot = (Map) loadedMetadata.get("clawdbot"); + @SuppressWarnings("unchecked") + Map requires = (Map) clawdbot.get("requires"); + + assertEquals(List.of("API_KEY", "API_SECRET"), requires.get("env")); + assertEquals(List.of("jq"), requires.get("bins")); + } + @Test @DisplayName("Should return false when saving empty list") void testSaveSkills_EmptyList() { From 1513f319108f62e62830f1f40e256526978f4954 Mon Sep 17 00:00:00 2001 From: fang-tech Date: Fri, 24 Apr 2026 01:18:06 +0800 Subject: [PATCH 4/5] refactor(skill): preserve rich metadata across skill storage and prompts --- .../mysql/MysqlSkillRepository.java | 253 +++++++++++++++--- .../mysql/MysqlSkillRepositoryTest.java | 139 +++++++++- 2 files changed, 338 insertions(+), 54 deletions(-) diff --git a/agentscope-extensions/agentscope-extensions-skill-mysql-repository/src/main/java/io/agentscope/core/skill/repository/mysql/MysqlSkillRepository.java b/agentscope-extensions/agentscope-extensions-skill-mysql-repository/src/main/java/io/agentscope/core/skill/repository/mysql/MysqlSkillRepository.java index f8dd46ab4e..1dab06d0fc 100644 --- a/agentscope-extensions/agentscope-extensions-skill-mysql-repository/src/main/java/io/agentscope/core/skill/repository/mysql/MysqlSkillRepository.java +++ b/agentscope-extensions/agentscope-extensions-skill-mysql-repository/src/main/java/io/agentscope/core/skill/repository/mysql/MysqlSkillRepository.java @@ -15,9 +15,11 @@ */ package io.agentscope.core.skill.repository.mysql; +import com.fasterxml.jackson.core.type.TypeReference; import io.agentscope.core.skill.AgentSkill; import io.agentscope.core.skill.repository.AgentSkillRepository; import io.agentscope.core.skill.repository.AgentSkillRepositoryInfo; +import io.agentscope.core.util.JsonUtils; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -25,6 +27,7 @@ import java.sql.Statement; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.regex.Pattern; @@ -35,18 +38,16 @@ /** * MySQL database-based implementation of AgentSkillRepository. * - *

    - * This implementation stores skills in MySQL database tables with the following - * structure: + *

    This implementation stores skills in MySQL database tables with the following structure: * *

      - *
    • Skills table: stores skill metadata (id, name, description, content, source) - *
    • Resources table: stores skill resources (id, resource_path, - * resource_content) + *
    • Skills table: stores core lookup fields ({@code name}, {@code description}), skill + * content, source, and optionally the full metadata tree in {@code metadata_json} + *
    • Resources table: stores skill resources ({@code id}, {@code resource_path}, + * {@code resource_content}) *
    * - *

    - * Table Schema (auto-created if createIfNotExist=true): + *

    Table schema for newly created tables ({@code createIfNotExist=true}): * *

      * CREATE TABLE IF NOT EXISTS agentscope_skills (
    @@ -55,6 +56,7 @@
      *     description TEXT NOT NULL,
      *     skill_content LONGTEXT NOT NULL,
      *     source VARCHAR(255) NOT NULL,
    + *     metadata_json LONGTEXT NULL,
      *     created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
      *     updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
      * ) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;
    @@ -70,15 +72,25 @@
      * ) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;
      * 
    * - *

    - * Features: + *

    Compatibility behavior: + * + *

      + *
    • New tables created by this repository include {@code metadata_json} + *
    • Existing tables are not auto-migrated with {@code ALTER TABLE} + *
    • When {@code metadata_json} exists, full skill metadata is persisted and restored + *
    • When {@code metadata_json} does not exist, the repository falls back to the legacy + * schema and only round-trips {@code name} and {@code description} + *
    + * + *

    Features: * *

      - *
    • Automatic table creation when createIfNotExist=true - *
    • Full CRUD operations for skills and their resources - *
    • SQL injection prevention through parameterized queries - *
    • Transaction support for atomic operations - *
    • UTF-8 (utf8mb4) character set support for internationalization + *
    • Automatic database/table creation when {@code createIfNotExist=true} + *
    • Runtime compatibility detection for legacy and new schemas + *
    • Full CRUD operations for skills and their resources + *
    • SQL injection prevention through parameterized queries + *
    • Transaction support for atomic operations + *
    • UTF-8 (utf8mb4) character set support for internationalization *
    * *

    @@ -140,6 +152,7 @@ public class MysqlSkillRepository implements AgentSkillRepository { private final String databaseName; private final String skillsTableName; private final String resourcesTableName; + private final boolean metadataJsonColumnSupported; private boolean writeable; /** @@ -150,8 +163,9 @@ public class MysqlSkillRepository implements AgentSkillRepository { * names ({@code agentscope_skills} and {@code agentscope_skill_resources}). * * @param dataSource DataSource for database connections - * @param createIfNotExist If true, auto-create database and tables; if false, - * require existing + * @param createIfNotExist If true, auto-create the database and tables for new deployments; if + * false, require existing schema. Existing tables are not auto-migrated + * to add {@code metadata_json} * @param writeable Whether the repository supports write operations * @throws IllegalArgumentException if dataSource is null * @throws IllegalStateException if createIfNotExist is false and @@ -173,10 +187,10 @@ public MysqlSkillRepository( * options. * *

    - * If {@code createIfNotExist} is true, the database and tables will be created - * automatically - * if they don't exist. If false and the database or tables don't exist, an - * {@link IllegalStateException} will be thrown. + * If {@code createIfNotExist} is true, the database and tables will be created automatically if + * they don't exist. If false and the database or tables don't exist, an + * {@link IllegalStateException} will be thrown. Existing tables are validated as-is and are not + * auto-migrated to add {@code metadata_json}. * *

    * This constructor is private. Use {@link #builder(DataSource)} to create instances @@ -189,8 +203,9 @@ public MysqlSkillRepository( * empty) * @param resourcesTableName Custom resources table name (uses default if null * or empty) - * @param createIfNotExist If true, auto-create database and tables; if false, - * require existing + * @param createIfNotExist If true, auto-create the database and tables for new deployments; + * if false, require existing schema. Existing tables are not + * auto-migrated to add {@code metadata_json} * @param writeable Whether the repository supports write operations * @throws IllegalArgumentException if dataSource is null or identifiers are * invalid @@ -240,6 +255,8 @@ private MysqlSkillRepository( verifyTablesExist(); } + this.metadataJsonColumnSupported = detectMetadataJsonColumnSupport(); + logger.info( "MysqlSkillRepository initialized with database: {}, skills table: {}," + " resources table: {}", @@ -272,6 +289,9 @@ private void createDatabaseIfNotExist() { /** * Create the skills and resources tables if they don't exist. + * + *

    Newly created skills tables include the optional {@code metadata_json} column so complete + * skill metadata can be persisted without changing the legacy lookup columns. */ private void createTablesIfNotExist() { // Create skills table with id as primary key and name as unique @@ -281,6 +301,7 @@ private void createTablesIfNotExist() { + " (id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY," + " name VARCHAR(255) NOT NULL UNIQUE, description TEXT NOT NULL," + " skill_content LONGTEXT NOT NULL, source VARCHAR(255) NOT NULL," + + " metadata_json LONGTEXT NULL," + " created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP" + " DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP) DEFAULT" + " CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci"; @@ -390,12 +411,41 @@ private String getFullTableName(String tableName) { return databaseName + "." + tableName; } + /** + * Detect whether the current skills table supports the optional {@code metadata_json} column. + * + *

    This capability is cached at repository construction time and drives the read/write + * compatibility path: full metadata round-trip when present, legacy fallback when absent. + */ + private boolean detectMetadataJsonColumnSupport() { + String checkSql = + "SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ?" + + " AND COLUMN_NAME = ? LIMIT 1"; + + try (Connection conn = dataSource.getConnection(); + PreparedStatement stmt = conn.prepareStatement(checkSql)) { + stmt.setString(1, databaseName); + stmt.setString(2, skillsTableName); + stmt.setString(3, "metadata_json"); + try (ResultSet rs = stmt.executeQuery()) { + return rs.next(); + } + } catch (SQLException e) { + logger.warn( + "Failed to detect metadata_json column support, falling back to legacy schema", + e); + return false; + } + } + @Override public AgentSkill getSkill(String name) { validateSkillName(name); String selectSkillSql = - "SELECT id, name, description, skill_content, source FROM " + "SELECT id, name, description, skill_content, source" + + (metadataJsonColumnSupported ? ", metadata_json" : "") + + " FROM " + getFullTableName(skillsTableName) + " WHERE name = ?"; @@ -410,6 +460,7 @@ public AgentSkill getSkill(String name) { String description; String skillContent; String source; + String metadataJson = null; try (PreparedStatement stmt = conn.prepareStatement(selectSkillSql)) { stmt.setString(1, name); @@ -421,6 +472,9 @@ public AgentSkill getSkill(String name) { description = rs.getString("description"); skillContent = rs.getString("skill_content"); source = rs.getString("source"); + if (metadataJsonColumnSupported) { + metadataJson = rs.getString("metadata_json"); + } } } @@ -437,7 +491,7 @@ public AgentSkill getSkill(String name) { } } - return new AgentSkill(name, description, skillContent, resources, source); + return buildSkill(name, description, skillContent, source, metadataJson, resources); } catch (SQLException e) { throw new RuntimeException("Failed to load skill: " + name, e); @@ -469,7 +523,9 @@ public List getAllSkillNames() { @Override public List getAllSkills() { String selectAllSkillsSql = - "SELECT id, name, description, skill_content, source FROM " + "SELECT id, name, description, skill_content, source" + + (metadataJsonColumnSupported ? ", metadata_json" : "") + + " FROM " + getFullTableName(skillsTableName) + " ORDER BY name"; @@ -479,7 +535,7 @@ public List getAllSkills() { try (Connection conn = dataSource.getConnection()) { // Load all skills in one query, use id as key for mapping resources - Map skillBuilders = new HashMap<>(); + Map skillRecords = new HashMap<>(); try (PreparedStatement stmt = conn.prepareStatement(selectAllSkillsSql); ResultSet rs = stmt.executeQuery()) { @@ -489,14 +545,13 @@ public List getAllSkills() { String description = rs.getString("description"); String skillContent = rs.getString("skill_content"); String source = rs.getString("source"); + String metadataJson = + metadataJsonColumnSupported ? rs.getString("metadata_json") : null; - AgentSkill.Builder builder = - AgentSkill.builder() - .name(name) - .description(description) - .skillContent(skillContent) - .source(source); - skillBuilders.put(skillId, builder); + skillRecords.put( + skillId, + new LoadedSkillRecord( + name, description, skillContent, source, metadataJson)); } } @@ -508,9 +563,9 @@ public List getAllSkills() { String resourcePath = rs.getString("resource_path"); String resourceContent = rs.getString("resource_content"); - AgentSkill.Builder builder = skillBuilders.get(skillId); - if (builder != null) { - builder.addResource(resourcePath, resourceContent); + LoadedSkillRecord record = skillRecords.get(skillId); + if (record != null) { + record.resources.put(resourcePath, resourceContent); } else { logger.warn("Found orphaned resource for non-existent id: {}", skillId); } @@ -518,10 +573,17 @@ public List getAllSkills() { } // Build all skills - List skills = new ArrayList<>(skillBuilders.size()); - for (AgentSkill.Builder builder : skillBuilders.values()) { + List skills = new ArrayList<>(skillRecords.size()); + for (LoadedSkillRecord record : skillRecords.values()) { try { - skills.add(builder.build()); + skills.add( + buildSkill( + record.name, + record.description, + record.skillContent, + record.source, + record.metadataJson, + record.resources)); } catch (Exception e) { logger.warn("Failed to build skill: {}", e.getMessage(), e); } @@ -631,7 +693,11 @@ private long insertSkill(Connection conn, AgentSkill skill) throws SQLException String insertSql = "INSERT INTO " + getFullTableName(skillsTableName) - + " (name, description, skill_content, source) VALUES (?, ?, ?, ?)"; + + (metadataJsonColumnSupported + ? " (name, description, skill_content, source, metadata_json)" + + " VALUES (?, ?, ?, ?, ?)" + : " (name, description, skill_content, source) VALUES (?, ?, ?," + + " ?)"); try (PreparedStatement stmt = conn.prepareStatement(insertSql, Statement.RETURN_GENERATED_KEYS)) { @@ -639,6 +705,16 @@ private long insertSkill(Connection conn, AgentSkill skill) throws SQLException stmt.setString(2, skill.getDescription()); stmt.setString(3, skill.getSkillContent()); stmt.setString(4, skill.getSource()); + if (metadataJsonColumnSupported) { + stmt.setString(5, serializeMetadata(skill.getMetadata())); + } else if (hasExtendedMetadata(skill.getMetadata())) { + logger.warn( + "metadata_json column not found in {}.{}; extended metadata for skill '{}'" + + " will not be persisted", + databaseName, + skillsTableName, + skill.getName()); + } stmt.executeUpdate(); try (ResultSet generatedKeys = stmt.getGeneratedKeys()) { @@ -875,6 +951,101 @@ public DataSource getDataSource() { return dataSource; } + /** + * Exposes whether the connected skills table supports {@code metadata_json}. + * + *

    Package-private for tests. + */ + boolean isMetadataJsonColumnSupported() { + return metadataJsonColumnSupported; + } + + /** + * Build an {@link AgentSkill} from SQL row data, restoring full metadata when available and + * otherwise falling back to legacy core metadata. + */ + private AgentSkill buildSkill( + String name, + String description, + String skillContent, + String source, + String metadataJson, + Map resources) { + Map metadata = deserializeMetadata(metadataJson, name, description); + return new AgentSkill(metadata, skillContent, resources, source); + } + + /** + * Deserialize {@code metadata_json} when present, then overlay the authoritative SQL columns + * for {@code name} and {@code description}. + */ + private Map deserializeMetadata( + String metadataJson, String name, String description) { + LinkedHashMap metadata = new LinkedHashMap<>(); + if (metadataJson != null && !metadataJson.isBlank()) { + try { + Map parsed = + JsonUtils.getJsonCodec() + .fromJson( + metadataJson, new TypeReference>() {}); + if (parsed != null) { + metadata.putAll(parsed); + } + } catch (RuntimeException e) { + logger.warn( + "Failed to deserialize metadata_json for skill '{}', falling back to core" + + " metadata", + name, + e); + } + } + metadata.put("name", name); + metadata.put("description", description); + return metadata; + } + + /** Serialize the complete skill metadata tree for storage in {@code metadata_json}. */ + private String serializeMetadata(Map metadata) { + return JsonUtils.getJsonCodec().toJson(metadata); + } + + /** + * Check whether metadata contains fields beyond the legacy core columns. + * + *

    This is used only to emit a downgrade warning when writing to a legacy schema. + */ + private boolean hasExtendedMetadata(Map metadata) { + if (metadata == null || metadata.isEmpty()) { + return false; + } + return metadata.size() > 2 + || metadata.keySet().stream() + .anyMatch(key -> !"name".equals(key) && !"description".equals(key)); + } + + /** Temporary holder used while stitching skills and resources from separate result sets. */ + private static final class LoadedSkillRecord { + private final String name; + private final String description; + private final String skillContent; + private final String source; + private final String metadataJson; + private final Map resources = new HashMap<>(); + + private LoadedSkillRecord( + String name, + String description, + String skillContent, + String source, + String metadataJson) { + this.name = name; + this.description = description; + this.skillContent = skillContent; + this.source = source; + this.metadataJson = metadataJson; + } + } + /** * Clear all skills from the database (for testing or cleanup). * diff --git a/agentscope-extensions/agentscope-extensions-skill-mysql-repository/src/test/java/io/agentscope/core/skill/repository/mysql/MysqlSkillRepositoryTest.java b/agentscope-extensions/agentscope-extensions-skill-mysql-repository/src/test/java/io/agentscope/core/skill/repository/mysql/MysqlSkillRepositoryTest.java index ac354c0811..52f2fab1cb 100644 --- a/agentscope-extensions/agentscope-extensions-skill-mysql-repository/src/test/java/io/agentscope/core/skill/repository/mysql/MysqlSkillRepositoryTest.java +++ b/agentscope-extensions/agentscope-extensions-skill-mysql-repository/src/test/java/io/agentscope/core/skill/repository/mysql/MysqlSkillRepositoryTest.java @@ -23,6 +23,8 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -32,6 +34,7 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import javax.sql.DataSource; @@ -46,15 +49,13 @@ /** * Unit tests for MysqlSkillRepository. * - *

    - * These tests use mocked DataSource and Connection to verify the behavior of + *

    These tests use mocked DataSource and Connection to verify the behavior of * MysqlSkillRepository without requiring an actual MySQL database. * - *

    - * Test categories: + *

    Test categories: *

      - *
    • Constructor tests - validate initialization and parameter handling - *
    • CRUD operation tests - verify skill save, get, delete operations + *
    • Constructor tests - validate initialization, schema creation, and compatibility detection + *
    • CRUD operation tests - verify legacy fallback and full metadata round-trip behavior *
    • SQL injection prevention tests - ensure security validations work *
    • Repository info tests - verify metadata reporting *
    @@ -81,6 +82,8 @@ void setUp() throws SQLException { when(mockConnection.prepareStatement(anyString())).thenReturn(mockStatement); // Also mock prepareStatement with RETURN_GENERATED_KEYS for insertSkill when(mockConnection.prepareStatement(anyString(), anyInt())).thenReturn(mockStatement); + when(mockStatement.executeQuery()).thenReturn(mockResultSet); + when(mockResultSet.next()).thenReturn(false); // Mock getGeneratedKeys for insertSkill when(mockStatement.getGeneratedKeys()).thenReturn(mockGeneratedKeysResultSet); when(mockGeneratedKeysResultSet.next()).thenReturn(true); @@ -121,6 +124,20 @@ void testConstructorWithCreateIfNotExistTrue() throws SQLException { assertEquals("agentscope_skill_resources", repo.getResourcesTableName()); assertEquals(mockDataSource, repo.getDataSource()); assertTrue(repo.isWriteable()); + assertTrue(repo.isMetadataJsonColumnSupported() == false); + verify(mockConnection, atLeast(1)).prepareStatement(anyString()); + } + + @Test + @DisplayName("Should create metadata_json column for new tables") + void testConstructorCreatesMetadataJsonColumn() throws SQLException { + when(mockStatement.execute()).thenReturn(true); + + new MysqlSkillRepository(mockDataSource, true, true); + + verify(mockConnection) + .prepareStatement( + org.mockito.ArgumentMatchers.contains("metadata_json LONGTEXT NULL")); } @Test @@ -597,22 +614,70 @@ void setUp() throws SQLException { @Test @DisplayName("Should get skill successfully") void testGetSkill() throws SQLException { - // Setup mock for skill query - when(mockStatement.executeQuery()).thenReturn(mockResultSet); - // First query: skill exists, second query: no resources - when(mockResultSet.next()).thenReturn(true, false); + when(mockResultSet.next()).thenReturn(true, true, false); when(mockResultSet.getString("name")).thenReturn("test-skill"); when(mockResultSet.getString("description")).thenReturn("Test description"); when(mockResultSet.getString("skill_content")).thenReturn("Test content"); when(mockResultSet.getString("source")).thenReturn("mysql_test"); + when(mockResultSet.getString("metadata_json")) + .thenReturn( + "{\"name\":\"test-skill\",\"description\":\"Test description\"," + + "\"homepage\":\"https://example.com\"}"); + + MysqlSkillRepository metadataRepo = + new MysqlSkillRepository(mockDataSource, true, true); - AgentSkill skill = repo.getSkill("test-skill"); + AgentSkill skill = metadataRepo.getSkill("test-skill"); assertNotNull(skill); assertEquals("test-skill", skill.getName()); assertEquals("Test description", skill.getDescription()); assertEquals("Test content", skill.getSkillContent()); assertEquals("mysql_test", skill.getSource()); + assertEquals("https://example.com", skill.getMetadataValue("homepage")); + } + + @Test + @DisplayName("Should fall back to core metadata when metadata_json column is absent") + void testGetSkillLegacySchemaFallback() throws SQLException { + when(mockResultSet.next()).thenReturn(false, true, false); + when(mockResultSet.getString("name")).thenReturn("test-skill"); + when(mockResultSet.getString("description")).thenReturn("Test description"); + when(mockResultSet.getString("skill_content")).thenReturn("Test content"); + when(mockResultSet.getString("source")).thenReturn("mysql_test"); + + MysqlSkillRepository legacyRepo = new MysqlSkillRepository(mockDataSource, true, true); + AgentSkill skill = legacyRepo.getSkill("test-skill"); + + assertEquals(List.of("name", "description"), List.copyOf(skill.getMetadata().keySet())); + assertEquals("test-skill", skill.getName()); + assertEquals("Test description", skill.getDescription()); + } + + @Test + @DisplayName("Should get all skills with metadata_json when column exists") + void testGetAllSkillsWithMetadataJson() throws SQLException { + when(mockResultSet.next()).thenReturn(true, true, false, true, false, false); + when(mockResultSet.getLong("id")).thenReturn(1L, 1L); + when(mockResultSet.getString("name")).thenReturn("skill1"); + when(mockResultSet.getString("description")).thenReturn("Desc 1"); + when(mockResultSet.getString("skill_content")).thenReturn("Content 1"); + when(mockResultSet.getString("source")).thenReturn("mysql_test"); + when(mockResultSet.getString("metadata_json")) + .thenReturn( + "{\"name\":\"skill1\",\"description\":\"Desc" + + " 1\",\"homepage\":\"https://example.com/1\"}"); + when(mockResultSet.getString("resource_path")).thenReturn("readme.md"); + when(mockResultSet.getString("resource_content")).thenReturn("hello"); + + MysqlSkillRepository metadataRepo = + new MysqlSkillRepository(mockDataSource, true, true); + + List skills = metadataRepo.getAllSkills(); + + assertEquals(1, skills.size()); + assertEquals("https://example.com/1", skills.get(0).getMetadataValue("homepage")); + assertEquals("hello", skills.get(0).getResources().get("readme.md")); } @Test @@ -695,11 +760,58 @@ void testSaveSkillWithResources() throws SQLException { verify(mockStatement, atLeast(1)).executeBatch(); } + @Test + @DisplayName("Should save metadata_json when column exists") + void testSaveSkillWithMetadataJson() throws SQLException { + when(mockStatement.executeUpdate()).thenReturn(1); + when(mockResultSet.next()).thenReturn(true, false); + + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "new-skill"); + metadata.put("description", "Description"); + metadata.put("homepage", "https://example.com"); + AgentSkill skill = new AgentSkill(metadata, "Content", Map.of(), "test"); + + MysqlSkillRepository metadataRepo = + new MysqlSkillRepository(mockDataSource, true, true); + + boolean saved = metadataRepo.save(List.of(skill), false); + + assertTrue(saved); + verify(mockStatement) + .setString( + eq(5), + org.mockito.ArgumentMatchers.contains( + "\"homepage\":\"https://example.com\"")); + } + + @Test + @DisplayName("Should skip metadata_json when legacy schema is used") + void testSaveSkillLegacySchemaFallback() throws SQLException { + when(mockStatement.execute()).thenReturn(true); + when(mockStatement.executeUpdate()).thenReturn(1); + when(mockResultSet.next()).thenReturn(false, false); + + MysqlSkillRepository legacyRepo = new MysqlSkillRepository(mockDataSource, true, true); + + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "legacy-skill"); + metadata.put("description", "Description"); + metadata.put("homepage", "https://example.com"); + AgentSkill skill = new AgentSkill(metadata, "Content", Map.of(), "test"); + + boolean saved = legacyRepo.save(List.of(skill), false); + + assertTrue(saved); + verify(mockStatement, never()).setString(eq(5), anyString()); + } + @Test @DisplayName("Should throw exception when skill exists and force=false") void testSaveSkillExistsNoForce() throws SQLException { when(mockStatement.executeQuery()).thenReturn(mockResultSet); - when(mockResultSet.next()).thenReturn(true); // skill exists + when(mockResultSet.next()) + .thenReturn(true, true); // metadata column exists, skill exists AgentSkill skill = new AgentSkill("existing-skill", "Description", "Content", Map.of(), "test"); @@ -718,7 +830,8 @@ void testSaveSkillExistsNoForce() throws SQLException { void testSaveSkillWithForce() throws SQLException { when(mockStatement.executeUpdate()).thenReturn(1); when(mockStatement.executeQuery()).thenReturn(mockResultSet); - when(mockResultSet.next()).thenReturn(true, false); // skill exists, then deleted + when(mockResultSet.next()) + .thenReturn(true, true, false); // column exists, skill exists, then deleted AgentSkill skill = new AgentSkill( From b48f76d55f4afeac7fdfcedd2b1d7df58e8ceaac Mon Sep 17 00:00:00 2001 From: fang-tech Date: Fri, 24 Apr 2026 11:14:34 +0800 Subject: [PATCH 5/5] fix(skill): tighten metadata prompt and validation behavior --- .../core/skill/AgentSkillPromptProvider.java | 10 +++++-- .../io/agentscope/core/skill/SkillBox.java | 2 +- .../core/skill/util/MarkdownSkillParser.java | 2 +- .../agentscope/core/skill/util/SkillUtil.java | 5 ++-- .../skill/AgentSkillPromptProviderTest.java | 17 +++++++++++ .../agentscope/core/skill/SkillUtilTest.java | 9 ++---- .../skill/util/MarkdownSkillParserTest.java | 28 +++++++++++++++++++ 7 files changed, 60 insertions(+), 13 deletions(-) diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java b/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java index cf18317d6a..5353d7c742 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/AgentSkillPromptProvider.java @@ -214,6 +214,9 @@ public void setExposeAllMetadata(boolean exposeAllMetadata) { private void appendSkill(StringBuilder sb, AgentSkill skill) { sb.append("\n"); for (Map.Entry entry : getPromptMetadata(skill).entrySet()) { + if (entry.getValue() == null) { + continue; + } appendXmlNode(sb, entry.getKey(), entry.getValue(), 1); } appendXmlNode(sb, "skill-id", skill.getSkillId(), 1); @@ -232,6 +235,10 @@ private Map getPromptMetadata(AgentSkill skill) { } private void appendXmlNode(StringBuilder sb, String key, Object value, int indentLevel) { + if (value == null) { + return; + } + String indent = INDENT.repeat(indentLevel); boolean validTagName = isValidXmlTagName(key); String openTag = validTagName ? "<" + key + ">" : ""; @@ -265,8 +272,7 @@ private void appendXmlNode(StringBuilder sb, String key, Object value, int inden } private boolean isScalarValue(Object value) { - return value == null - || (!(value instanceof Map) && !(value instanceof Collection)); + return !(value instanceof Map) && !(value instanceof Collection); } private boolean isValidXmlTagName(String value) { diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java index 0c12f8f9db..da391666f5 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/SkillBox.java @@ -59,7 +59,7 @@ public SkillBox(Toolkit toolkit) { } /** - * Creates a SkillBox with a toolkit and custom skill prompt instruction and template. + * Creates a SkillBox with a toolkit and custom skill prompt instruction. * * @param toolkit The toolkit to bind * @param instruction Custom instruction header (null or blank uses default) diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/util/MarkdownSkillParser.java b/agentscope-core/src/main/java/io/agentscope/core/skill/util/MarkdownSkillParser.java index 90f780e8d7..b00bd088d8 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/util/MarkdownSkillParser.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/util/MarkdownSkillParser.java @@ -187,7 +187,7 @@ private static LoaderOptions createLoaderOptions() { options.setAllowDuplicateKeys(false); options.setMaxAliasesForCollections(10); options.setNestingDepthLimit(10); - options.setCodePointLimit(10_000); + options.setCodePointLimit(FRONTMATTER_CODE_POINT_LIMIT); return options; } diff --git a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java index 49f7a194bd..aa6d551216 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java +++ b/agentscope-core/src/main/java/io/agentscope/core/skill/util/SkillUtil.java @@ -309,11 +309,10 @@ private static String stringifyRequiredMetadata(Map metadata, St } Object value = metadata.get(key); - if (value == null) { + if (!(value instanceof String stringValue)) { return null; } - String stringValue = String.valueOf(value); - return stringValue.isEmpty() ? null : stringValue; + return stringValue.isBlank() ? null : stringValue; } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillPromptProviderTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillPromptProviderTest.java index 62faa21445..ffc3e0bd60 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillPromptProviderTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/AgentSkillPromptProviderTest.java @@ -148,6 +148,23 @@ void testExposeOnlyCoreMetadata() { assertFalse(prompt.contains("")); } + @Test + @DisplayName("Should omit null metadata values from XML output") + void testOmitNullMetadataValues() { + Map metadata = new LinkedHashMap<>(); + metadata.put("name", "trello"); + metadata.put("description", "Manage Trello boards"); + metadata.put("homepage", null); + AgentSkill skill = new AgentSkill(metadata, "# Content", null, null); + skillRegistry.registerSkill("trello_custom", skill, new RegisteredSkill("trello_custom")); + + String prompt = provider.getSkillSystemPrompt(); + + assertFalse(prompt.contains("null")); + assertFalse(prompt.contains("")); + assertFalse(prompt.contains("")); + } + @Test @DisplayName("Should escape special characters in XML") void testXmlEscaping() { diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java index 45fd5c8c26..e585157348 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/SkillUtilTest.java @@ -215,14 +215,11 @@ void testCreateFromEdgeCases() { } @Test - @DisplayName("Should create from with numeric metadata") - void testCreateFromWithNumericMetadata() { + @DisplayName("Should reject numeric values for required metadata") + void testCreateFromRejectsNumericRequiredMetadata() { String skillMd = "---\nname: 123\ndescription: 456\n---\nContent"; - AgentSkill skill = SkillUtil.createFrom(skillMd, null); - - assertEquals("123", skill.getName()); - assertEquals("456", skill.getDescription()); + assertThrows(IllegalArgumentException.class, () -> SkillUtil.createFrom(skillMd, null)); } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/skill/util/MarkdownSkillParserTest.java b/agentscope-core/src/test/java/io/agentscope/core/skill/util/MarkdownSkillParserTest.java index 7e8476fafe..dd748d87a2 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/skill/util/MarkdownSkillParserTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/skill/util/MarkdownSkillParserTest.java @@ -608,6 +608,34 @@ void testPreserveMetadataOrder() { assertTrue(homepageIndex < metadataIndex); } + @Test + @DisplayName("Should reject non-string required metadata values") + void testRejectNonStringRequiredMetadataValues() { + String skillMd = + "---\n" + + "name:\n" + + " - a\n" + + " - b\n" + + "description: valid description\n" + + "---\n" + + "Content"; + + assertThrows(IllegalArgumentException.class, () -> SkillUtil.createFrom(skillMd, null)); + } + + @Test + @DisplayName("Should align parser code point limit with frontmatter limit") + void testFrontmatterAtConfiguredCodePointLimit() { + String value = "a".repeat(16_360); + String markdown = + "---\n" + "name: test\n" + "description: " + value + "\n---\n" + "Content"; + + ParsedMarkdown parsed = MarkdownSkillParser.parse(markdown); + + assertEquals("test", parsed.getMetadata().get("name")); + assertEquals(value, parsed.getMetadata().get("description")); + } + @Test @DisplayName("Should keep generated document unchanged after parse and regenerate") void testParseThenGenerateKeepsDocumentStable() {