From 240c75229edf1157fb7bacdbcd555996b33ab6ef Mon Sep 17 00:00:00 2001 From: MaryamZi Date: Thu, 12 Mar 2026 10:01:10 +0530 Subject: [PATCH 1/9] Support Agent Skills - Take I --- ballerina-interpreter/Dependencies.toml | 10 +- ballerina-interpreter/README.md | 10 + ballerina-interpreter/agent.bal | 40 +++- ballerina-interpreter/main.bal | 9 +- ballerina-interpreter/parser.bal | 10 +- ballerina-interpreter/skills.bal | 265 +++++++++++++++++++++ ballerina-interpreter/tests/agent_test.bal | 4 +- ballerina-interpreter/types.bal | 16 ++ templates/agent_template.afm.md | 7 + 9 files changed, 348 insertions(+), 23 deletions(-) create mode 100644 ballerina-interpreter/skills.bal diff --git a/ballerina-interpreter/Dependencies.toml b/ballerina-interpreter/Dependencies.toml index 9850b6f..37caeb7 100644 --- a/ballerina-interpreter/Dependencies.toml +++ b/ballerina-interpreter/Dependencies.toml @@ -5,7 +5,7 @@ [ballerina] dependencies-toml-version = "2" -distribution-version = "2201.12.7" +distribution-version = "2201.12.10" [[package]] org = "ballerina" @@ -90,7 +90,7 @@ dependencies = [ [[package]] org = "ballerina" name = "data.xmldata" -version = "1.5.2" +version = "1.6.1" dependencies = [ {org = "ballerina", name = "jballerina.java"}, {org = "ballerina", name = "lang.object"} @@ -117,6 +117,9 @@ dependencies = [ {org = "ballerina", name = "os"}, {org = "ballerina", name = "time"} ] +modules = [ + {org = "ballerina", packageName = "file", moduleName = "file"} +] [[package]] org = "ballerina" @@ -468,7 +471,7 @@ modules = [ [[package]] org = "ballerinax" name = "ai.anthropic" -version = "1.3.0" +version = "1.3.1" dependencies = [ {org = "ballerina", name = "ai"}, {org = "ballerina", name = "constraint"}, @@ -530,6 +533,7 @@ version = "0.1.0" dependencies = [ {org = "ballerina", name = "ai"}, {org = "ballerina", name = "data.yaml"}, + {org = "ballerina", name = "file"}, {org = "ballerina", name = "http"}, {org = "ballerina", name = "io"}, {org = "ballerina", name = "jballerina.java"}, diff --git a/ballerina-interpreter/README.md b/ballerina-interpreter/README.md index c76ca18..1937029 100644 --- a/ballerina-interpreter/README.md +++ b/ballerina-interpreter/README.md @@ -48,8 +48,17 @@ docker run -v /path/to/agent.afm.md:/app/agent.afm.md \ -e afmFilePath=/app/agent.afm.md \ -p 8085:8085 \ afm-ballerina-interpreter + +# Run with skills (mount the skills directory so the agent can discover them) +docker run -v /path/to/agent.afm.md:/app/agent.afm.md \ + -v /path/to/skills:/app/skills \ + -e afmFilePath=/app/agent.afm.md \ + -p 8085:8085 \ + afm-ballerina-interpreter ``` +When using local skills, the `path` in the AFM file should be relative to the AFM file's location. For example, if the AFM file is at `/app/agent.afm.md` and skills are mounted at `/app/skills`, use `path: "./skills"` in the AFM file. + ## Testing ```bash @@ -68,6 +77,7 @@ ballerina-interpreter/ ├── interface_web_chat.bal # Web chat HTTP API ├── interface_web_ui.bal # Web chat UI ├── interface_webhook.bal # Webhook/WebSub handler +├── skills.bal # Agent Skills discovery & toolkit ├── modules/ │ └── everit.validator/ # JSON Schema validation ├── tests/ # Test files diff --git a/ballerina-interpreter/agent.bal b/ballerina-interpreter/agent.bal index bad401d..61ffb38 100644 --- a/ballerina-interpreter/agent.bal +++ b/ballerina-interpreter/agent.bal @@ -23,10 +23,10 @@ import ballerina/http; import ballerinax/ai.anthropic; import ballerinax/ai.openai; -function createAgent(AFMRecord afmRecord) returns ai:Agent|error { +function createAgent(AFMRecord afmRecord, string afmFileDir) returns ai:Agent|error { AFMRecord {metadata, role, instructions} = afmRecord; - ai:McpToolKit[] mcpToolkits = []; + ai:McpToolKit[] mcpToolKits = []; MCPServer[]? mcpServers = metadata?.tools?.mcp; if mcpServers is MCPServer[] { foreach MCPServer mcpConn in mcpServers { @@ -36,7 +36,7 @@ function createAgent(AFMRecord afmRecord) returns ai:Agent|error { } string[]? filteredTools = getFilteredTools(mcpConn.tool_filter); - mcpToolkits.push(check new ai:McpToolKit( + mcpToolKits.push(check new ai:McpToolKit( transport.url, permittedTools = filteredTools, auth = check mapToHttpClientAuth(transport.authentication) @@ -44,14 +44,27 @@ function createAgent(AFMRecord afmRecord) returns ai:Agent|error { } } + [string, SkillsToolKit]? catalog = check extractSkillCatalog(metadata, afmFileDir); + + string effectiveInstructions; + (ai:BaseToolKit)[] toolKits; + + if catalog is () { + effectiveInstructions = instructions; + toolKits = mcpToolKits; + } else { + effectiveInstructions = string `${instructions}\n${catalog[0]}`; + toolKits = [...mcpToolKits, catalog[1]]; + } + ai:ModelProvider model = check getModel(metadata?.model); - + ai:AgentConfiguration agentConfig = { systemPrompt: { - role, - instructions + role, + instructions: effectiveInstructions }, - tools: mcpToolkits, + tools: toolKits, model }; @@ -85,9 +98,9 @@ function getModel(Model? model) returns ai:ModelProvider|error { return error("This implementation requires the 'provider' of the model to be specified"); } - provider = provider.toLowerAscii(); + string providerLower = provider.toLowerAscii(); - if provider == "wso2" { + if providerLower == "wso2" { return new ai:Wso2ModelProvider( model.url ?: "https://dev-tools.wso2.com/ballerina-copilot/v2.0", check getToken(model.authentication) @@ -99,7 +112,7 @@ function getModel(Model? model) returns ai:ModelProvider|error { return error("This implementation requires the 'name' of the model to be specified"); } - match provider { + match providerLower { "openai" => { return new openai:ModelProvider( check getApiKey(model.authentication), @@ -115,12 +128,13 @@ function getModel(Model? model) returns ai:ModelProvider|error { ); } } - return error(string `Model provider: ${provider} not yet supported`); + return error(string `Model provider: ${provider} not yet supported`); } const DEFAULT_SESSION_ID = "sessionId"; -function runAgent(ai:Agent agent, json payload, map? inputSchema = (), map? outputSchema = (), string sessionId = DEFAULT_SESSION_ID) +function runAgent(ai:Agent agent, json payload, map? inputSchema = (), + map? outputSchema = (), string sessionId = DEFAULT_SESSION_ID) returns json|InputError|AgentError { error? validateJsonSchemaResult = validateJsonSchema(inputSchema, payload); if validateJsonSchemaResult is error { @@ -275,7 +289,7 @@ isolated function validateJsonSchema(map? jsonSchemaVal, json sampleJson) validator:JSONObject jsonObject = validator:newJSONObject7(sampleJson.toJsonString()); error? validationResult = trap schema.validate(jsonObject); if validationResult is error { - return error("JSON validation failed: " + validationResult.message()); + return error(string `JSON validation failed: ${validationResult.message()}`); } return (); } diff --git a/ballerina-interpreter/main.bal b/ballerina-interpreter/main.bal index 1cf8b0e..ebadc8f 100644 --- a/ballerina-interpreter/main.bal +++ b/ballerina-interpreter/main.bal @@ -15,6 +15,7 @@ // under the License. import ballerina/ai; +import ballerina/file; import ballerina/http; import ballerina/io; import ballerina/log; @@ -41,14 +42,14 @@ public function main(string? filePath = ()) returns error? { fileToUse = filePath; } - string content = check io:fileReadString(fileToUse); + string afmFileDir = check file:parentPath(check file:getAbsolutePath(fileToUse)); AFMRecord afm = check parseAfm(content); - check runAgentFromAFM(afm, port); + check runAgentFromAFM(afm, port, afmFileDir); } -function runAgentFromAFM(AFMRecord afm, int port) returns error? { +function runAgentFromAFM(AFMRecord afm, int port, string afmFileDir) returns error? { AgentMetadata metadata = afm.metadata; Interface[] agentInterfaces = metadata.interfaces ?: [{}]; @@ -56,7 +57,7 @@ function runAgentFromAFM(AFMRecord afm, int port) returns error? { var [consoleChatInterface, webChatInterface, webhookInterface] = check validateAndExtractInterfaces(agentInterfaces); - ai:Agent agent = check createAgent(afm); + ai:Agent agent = check createAgent(afm, afmFileDir); // Start all service-based interfaces first (non-blocking) http:Listener? httpListener = (); diff --git a/ballerina-interpreter/parser.bal b/ballerina-interpreter/parser.bal index 3420f39..c046604 100644 --- a/ballerina-interpreter/parser.bal +++ b/ballerina-interpreter/parser.bal @@ -161,7 +161,7 @@ function validateHttpVariables(AFMRecord afmRecord) returns error? { return error("http: variables are only supported in webhook prompt fields, found in instructions section"); } - AgentMetadata {authors, provider, model, interfaces, tools, max_iterations: _, ...rest} = afmRecord.metadata; + AgentMetadata {authors, provider, model, interfaces, tools, skills, max_iterations: _, ...rest} = afmRecord.metadata; string[] erroredKeys = []; @@ -285,6 +285,14 @@ function validateHttpVariables(AFMRecord afmRecord) returns error? { } } + if skills is SkillSource[] { + foreach SkillSource skillSource in skills { + if containsHttpVariable(skillSource.path) { + erroredKeys.push("skills.path"); + } + } + } + if erroredKeys.length() > 0 { return error(string `http: variables are only supported in webhook prompt fields, found in metadata fields: ${string:'join(", ", ...erroredKeys)}`); } diff --git a/ballerina-interpreter/skills.bal b/ballerina-interpreter/skills.bal new file mode 100644 index 0000000..3fd6971 --- /dev/null +++ b/ballerina-interpreter/skills.bal @@ -0,0 +1,265 @@ +// Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/ai; +import ballerina/data.yaml; +import ballerina/file; +import ballerina/io; +import ballerina/log; + +const SKILL_FILE = "SKILL.md"; +const REFERENCES_DIR = "references"; +const ASSETS_DIR = "assets"; + +function extractSkillCatalog(AgentMetadata metadata, string afmFileDir) returns [string, SkillsToolKit]|error? { + SkillSource[]? skillSources = metadata?.skills; + if skillSources is () || skillSources.length() == 0 { + return (); + } + map skills = check discoverSkills(skillSources, afmFileDir); + log:printDebug(string `Loaded ${skills.length()} skill(s): ${string:'join(", ", ...skills.keys())}`); + string? skillCatalog = buildSkillCatalog(skills); + if skillCatalog is () { + return (); // No catalog if no skills, even if skill sources were defined + } + return [skillCatalog, new SkillsToolKit(skills)]; +} + +type SkillFrontmatter record { + string name; + string description; +}; + +function discoverSkills(SkillSource[] sources, string afmFileDir) returns map|error { + map skills = {}; + + foreach SkillSource 'source in sources { + string resolvedPath = check file:joinPath(afmFileDir, 'source.path); + map localSkills = check discoverLocalSkills(resolvedPath); + foreach [string, SkillInfo] [name, info] in localSkills.entries() { + if skills.hasKey(name) { + log:printWarn(string `Skill '${name}' already discovered, skipping duplicate from ${'source.path}`); + continue; + } + skills[name] = info; + } + } + + return skills; +} + +function discoverLocalSkills(string path) returns map|error { + map skills = {}; + string absPath = check file:getAbsolutePath(path); + + string skillMdPath = check file:joinPath(absPath, SKILL_FILE); + if check file:test(skillMdPath, file:EXISTS) { + SkillInfo info = check parseSkillMd(skillMdPath, absPath); + return {[info.name]: info}; + } + + file:MetaData[] entries = check file:readDir(absPath); + foreach file:MetaData entry in entries { + if !entry.dir { + continue; + } + string subSkillMd = check file:joinPath(entry.absPath, SKILL_FILE); + if !check file:test(subSkillMd, file:EXISTS) { + continue; + } + + SkillInfo|error info = parseSkillMd(subSkillMd, entry.absPath); + if info is error { + log:printError(string `Failed to parse skill at ${entry.absPath}`, 'error = info); + continue; + } + if skills.hasKey(info.name) { + log:printWarn(string `Skill '${info.name}' already discovered, skipping duplicate at ${entry.absPath}`); + continue; + } + skills[info.name] = info; + } + + return skills; +} + +function parseSkillMd(string skillMdPath, string basePath) returns SkillInfo|error { + string content = check io:fileReadString(skillMdPath); + return parseSkillMdContent(content, basePath, listLocalResources(basePath)); +} + +function parseSkillMdContent(string content, string basePath, string[] resources) returns SkillInfo|error { + string[] lines = splitLines(content); + int length = lines.length(); + + if length == 0 || lines[0].trim() != FRONTMATTER_DELIMITER { + return error("SKILL.md must start with YAML frontmatter (---)"); + } + + int i = 1; + while i < length && lines[i].trim() != FRONTMATTER_DELIMITER { + i += 1; + } + + if i >= length { + return error("SKILL.md frontmatter is not closed (missing ---)"); + } + + string yamlContent = string:'join("\n", ...lines.slice(1, i)); + map intermediate = check yaml:parseString(yamlContent); + SkillFrontmatter frontmatter = check intermediate.fromJsonWithType(); + + if frontmatter.name.trim() == "" { + return error("SKILL.md 'name' field is required and must not be empty"); + } + if frontmatter.description.trim() == "" { + return error("SKILL.md 'description' field is required and must not be empty"); + } + + string body = string:'join("\n", ...lines.slice(i + 1)).trim(); + + return { + name: frontmatter.name, + description: frontmatter.description, + body, + basePath, + resources + }; +} + +function listLocalResources(string basePath) returns string[] { + string[] resources = []; + foreach string dir in [REFERENCES_DIR, ASSETS_DIR] { + do { + string dirPath = check file:joinPath(basePath, dir); + if !check file:test(dirPath, file:EXISTS) { + continue; + } + file:MetaData[] entries = check file:readDir(dirPath); + foreach file:MetaData entry in entries { + if !entry.dir { + resources.push(check file:joinPath(dir, getFileName(entry.absPath))); + } + } + } on fail error e { + log:printWarn(string `Failed to read directory ${dir}`, 'error = e); + } + } + return resources; +} + +function getFileName(string path) returns string { + int? lastSlash = path.lastIndexOf("/") ?: path.lastIndexOf("\\"); + return lastSlash is int ? path.substring(lastSlash + 1) : path; +} + +function buildSkillCatalog(map skills) returns string? { + if skills.length() == 0 { + return (); + } + + return string ` +## Available Skills + +The following skills provide specialized instructions for specific tasks. +When a task matches a skill's description, call the activate_skill tool +with the skill's name to load its full instructions. + +${xml ` +${from SkillInfo skill in skills select xml ` + + ${skill.name} + ${skill.description} + +`} +`.toString()} +`; +} + +isolated class SkillsToolKit { + *ai:BaseToolKit; + + private final map & readonly skills; + + isolated function init(map skills) { + self.skills = skills.cloneReadOnly(); + } + + # Activates a skill by name and returns its full instructions along with available resources. + # Call this when a task matches one of the available skills' descriptions. + # + # + name - the name of the skill to activate (must match a name from the available skills catalog) + # + return - the skill's full instructions and list of available resource files + @ai:AgentTool + isolated function activate_skill(string name) returns string|error { + lock { + if !self.skills.hasKey(name) { + return error(string `Skill '${name}' not found. Available skills: ${ + string:'join(", ", ...self.skills.keys())}`); + } + + SkillInfo info = self.skills.get(name); + return xml ` + +${info.body} +${info.resources.length() > 0 ? + xml ` + +${from string res in info.resources select xml `${res}`} + +Use the read_skill_resource tool to read any of these files if needed. +` : xml ``} + +`.toString(); + } + } + + # Reads a resource file from a skill's references/ or assets/ directory. + # Only files listed in skill_resources after activating a skill can be read. + # + # + skillName - the name of the skill that owns the resource + # + resourcePath - relative path to the resource file (e.g., "references/REFERENCE.md" or "assets/template.json") + # + return - the content of the resource file + @ai:AgentTool + isolated function read_skill_resource(string skillName, string resourcePath) returns string|error { + lock { + if !self.skills.hasKey(skillName) { + return error(string `Skill '${skillName}' not found`); + } + + if !resourcePath.startsWith(string `${REFERENCES_DIR}/`) && + !resourcePath.startsWith(string `${ASSETS_DIR}/`) { + return error(string `Resource path must start with '${REFERENCES_DIR}/' or '${ASSETS_DIR}/'`); + } + + if resourcePath.indexOf("../") != () || resourcePath.indexOf("..\\") != () { + return error("Path traversal is not allowed in resource paths"); + } + + SkillInfo info = self.skills.get(skillName); + if info.resources.indexOf(resourcePath) is () { + return error(string `Resource '${resourcePath}' not found in skill '${ + skillName}'. Available: ${string:'join(", ", ...info.resources)}`); + } + + string fullPath = check file:joinPath(info.basePath, resourcePath); + return io:fileReadString(fullPath); + } + } + + public isolated function getTools() returns ai:ToolConfig[] => + ai:getToolConfigs([self.activate_skill, self.read_skill_resource]); +} diff --git a/ballerina-interpreter/tests/agent_test.bal b/ballerina-interpreter/tests/agent_test.bal index f1cadd1..c117be2 100644 --- a/ballerina-interpreter/tests/agent_test.bal +++ b/ballerina-interpreter/tests/agent_test.bal @@ -369,7 +369,7 @@ function testArrayOutputSchemaEndToEnd() returns error? { AFMRecord afm = check parseAfm(content); int testPort = 9195; - future _ = start runAgentFromAFM(afm, testPort); + future _ = start runAgentFromAFM(afm, testPort, "tests"); // Wait for agent to start runtime:sleep(2); @@ -396,7 +396,7 @@ function testArrayOutputSchemaInvalidResponse() returns error? { AFMRecord afm = check parseAfm(content); int testPort = 9196; - future _ = start runAgentFromAFM(afm, testPort); + future _ = start runAgentFromAFM(afm, testPort, "tests"); // Wait for agent to start runtime:sleep(2); diff --git a/ballerina-interpreter/types.bal b/ballerina-interpreter/types.bal index 0ff5079..77ced96 100644 --- a/ballerina-interpreter/types.bal +++ b/ballerina-interpreter/types.bal @@ -65,6 +65,21 @@ type Tools record {| MCPServer[] mcp?; |}; +type LocalSkillSource record {| + "local" 'type = "local"; + string path; +|}; + +type SkillSource LocalSkillSource; + +type SkillInfo record {| + string name; + string description; + string body; + string basePath; + string[] resources; +|}; + type Parameter record {| string name; string 'type; @@ -154,6 +169,7 @@ type AgentMetadata record {| Model model?; Interface[] interfaces?; Tools tools?; + SkillSource[] skills?; int max_iterations?; |}; diff --git a/templates/agent_template.afm.md b/templates/agent_template.afm.md index 515a9a1..b31b8ce 100644 --- a/templates/agent_template.afm.md +++ b/templates/agent_template.afm.md @@ -155,6 +155,13 @@ tools: allow: - "query" - "search" +# ============================================================================ +# SKILLS - OPTIONAL (Agent Skills format: https://agentskills.io) +# ============================================================================ +skills: + # Local skills directory or (a directory that may contain multiple skill subdirectories) + - type: "local" + path: "./skills" --- # Role From c5ab98ec3e9b2e554fb1023c578a5a864d3a802f Mon Sep 17 00:00:00 2001 From: MaryamZi Date: Thu, 12 Mar 2026 17:40:35 +0530 Subject: [PATCH 2/9] Fix up code and add tests --- ballerina-interpreter/agent.bal | 2 +- ballerina-interpreter/parser.bal | 79 +-- ballerina-interpreter/skills.bal | 105 ++- .../duplicate_names/error_translator/SKILL.md | 8 + .../error_translator_dup/SKILL.md | 8 + .../tests/skills/empty_dir/.gitkeep | 0 .../tests/skills/invalid_skill/SKILL.md | 6 + .../skills/multi_skills/pr_summary/SKILL.md | 12 + .../multi_skills/security_review/SKILL.md | 15 + .../security_review/assets/template.json | 1 + .../security_review/references/REFERENCE.md | 11 + .../tests/skills/no_frontmatter/SKILL.md | 2 + .../partial_invalid/invalid_skill/SKILL.md | 6 + .../partial_invalid/valid_skill/SKILL.md | 6 + .../tests/skills/single_skill/SKILL.md | 12 + .../skills/source1/release_notes/SKILL.md | 8 + .../skills/source2/release_notes/SKILL.md | 8 + ballerina-interpreter/tests/skills_test.bal | 599 ++++++++++++++++++ 18 files changed, 782 insertions(+), 106 deletions(-) create mode 100644 ballerina-interpreter/tests/skills/duplicate_names/error_translator/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/duplicate_names/error_translator_dup/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/empty_dir/.gitkeep create mode 100644 ballerina-interpreter/tests/skills/invalid_skill/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/multi_skills/pr_summary/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/multi_skills/security_review/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/multi_skills/security_review/assets/template.json create mode 100644 ballerina-interpreter/tests/skills/multi_skills/security_review/references/REFERENCE.md create mode 100644 ballerina-interpreter/tests/skills/no_frontmatter/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/partial_invalid/invalid_skill/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/partial_invalid/valid_skill/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/single_skill/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/source1/release_notes/SKILL.md create mode 100644 ballerina-interpreter/tests/skills/source2/release_notes/SKILL.md create mode 100644 ballerina-interpreter/tests/skills_test.bal diff --git a/ballerina-interpreter/agent.bal b/ballerina-interpreter/agent.bal index 61ffb38..e6f8a55 100644 --- a/ballerina-interpreter/agent.bal +++ b/ballerina-interpreter/agent.bal @@ -53,7 +53,7 @@ function createAgent(AFMRecord afmRecord, string afmFileDir) returns ai:Agent|er effectiveInstructions = instructions; toolKits = mcpToolKits; } else { - effectiveInstructions = string `${instructions}\n${catalog[0]}`; + effectiveInstructions = string `${instructions}\n\n${catalog[0]}`; toolKits = [...mcpToolKits, catalog[1]]; } diff --git a/ballerina-interpreter/parser.bal b/ballerina-interpreter/parser.bal index c046604..d12296e 100644 --- a/ballerina-interpreter/parser.bal +++ b/ballerina-interpreter/parser.bal @@ -19,40 +19,19 @@ import ballerina/os; function parseAfm(string content) returns AFMRecord|error { string resolvedContent = check resolveVariables(content); - - string[] lines = splitLines(resolvedContent); - int length = lines.length(); - - AgentMetadata? metadata = (); - int bodyStart = 0; - - // Extract and parse YAML frontmatter - if length > 0 && lines[0].trim() == FRONTMATTER_DELIMITER { - int i = 1; - while i < length && lines[i].trim() != FRONTMATTER_DELIMITER { - i += 1; - } - - if i < length { - string[] fmLines = []; - foreach int j in 1 ..< i { - fmLines.push(lines[j]); - } - string yamlContent = string:'join("\n", ...fmLines); - map intermediate = check yaml:parseString(yamlContent); - metadata = check intermediate.fromJsonWithType(); - bodyStart = i + 1; - } - } - + + [map, string] [frontmatterMap, body] = check extractFrontmatter(resolvedContent); + AgentMetadata metadata = check frontmatterMap.fromJsonWithType(); + // Extract Role and Instructions sections + string[] bodyLines = splitLines(body); string role = ""; string instructions = ""; boolean inRole = false; boolean inInstructions = false; - - foreach int k in bodyStart ..< length { - string line = lines[k]; + + foreach int k in 0 ..< bodyLines.length() { + string line = bodyLines[k]; string trimmed = line.trim(); if trimmed.startsWith("# ") { @@ -70,7 +49,7 @@ function parseAfm(string content) returns AFMRecord|error { } AFMRecord afmRecord = { - metadata: check metadata.ensureType(), + metadata, role: role.trim(), instructions: instructions.trim() }; @@ -406,19 +385,43 @@ function toolFilterContainsHttpVariable(ToolFilter? filter) returns boolean { return false; } +// Extracts YAML frontmatter and the remaining body from a document delimited by `---`. +// Returns the parsed YAML as a map and the body text after the closing delimiter. +function extractFrontmatter(string content) returns [map, string]|error { + string[] lines = splitLines(content); + int length = lines.length(); + + if length == 0 || lines[0].trim() != FRONTMATTER_DELIMITER { + return error("Document must start with YAML frontmatter (---)"); + } + + int i = 1; + while i < length && lines[i].trim() != FRONTMATTER_DELIMITER { + i += 1; + } + + if i >= length { + return error("Frontmatter is not closed (missing ---)"); + } + + string yamlContent = string:'join("\n", ...lines.slice(1, i)); + map frontmatter = check yaml:parseString(yamlContent); + string body = string:'join("\n", ...lines.slice(i + 1)); + return [frontmatter, body]; +} + function splitLines(string content) returns string[] { string[] result = []; - string remaining = content; + int length = content.length(); + int 'start = 0; - while true { - int? idx = remaining.indexOf("\n"); + while 'start < length { + int? idx = content.indexOf("\n", 'start); if idx is int { - result.push(remaining.substring(0, idx)); - remaining = remaining.substring(idx + 1); + result.push(content.substring('start, idx)); + 'start = idx + 1; } else { - if remaining.length() > 0 { - result.push(remaining); - } + result.push(content.substring('start)); break; } } diff --git a/ballerina-interpreter/skills.bal b/ballerina-interpreter/skills.bal index 3fd6971..065b820 100644 --- a/ballerina-interpreter/skills.bal +++ b/ballerina-interpreter/skills.bal @@ -15,7 +15,6 @@ // under the License. import ballerina/ai; -import ballerina/data.yaml; import ballerina/file; import ballerina/io; import ballerina/log; @@ -102,25 +101,8 @@ function parseSkillMd(string skillMdPath, string basePath) returns SkillInfo|err } function parseSkillMdContent(string content, string basePath, string[] resources) returns SkillInfo|error { - string[] lines = splitLines(content); - int length = lines.length(); - - if length == 0 || lines[0].trim() != FRONTMATTER_DELIMITER { - return error("SKILL.md must start with YAML frontmatter (---)"); - } - - int i = 1; - while i < length && lines[i].trim() != FRONTMATTER_DELIMITER { - i += 1; - } - - if i >= length { - return error("SKILL.md frontmatter is not closed (missing ---)"); - } - - string yamlContent = string:'join("\n", ...lines.slice(1, i)); - map intermediate = check yaml:parseString(yamlContent); - SkillFrontmatter frontmatter = check intermediate.fromJsonWithType(); + [map, string] [frontmatterMap, body] = check extractFrontmatter(content); + SkillFrontmatter frontmatter = check frontmatterMap.fromJsonWithType(); if frontmatter.name.trim() == "" { return error("SKILL.md 'name' field is required and must not be empty"); @@ -129,12 +111,10 @@ function parseSkillMdContent(string content, string basePath, string[] resources return error("SKILL.md 'description' field is required and must not be empty"); } - string body = string:'join("\n", ...lines.slice(i + 1)).trim(); - return { name: frontmatter.name, description: frontmatter.description, - body, + body: body.trim(), basePath, resources }; @@ -145,27 +125,19 @@ function listLocalResources(string basePath) returns string[] { foreach string dir in [REFERENCES_DIR, ASSETS_DIR] { do { string dirPath = check file:joinPath(basePath, dir); - if !check file:test(dirPath, file:EXISTS) { - continue; - } file:MetaData[] entries = check file:readDir(dirPath); foreach file:MetaData entry in entries { if !entry.dir { - resources.push(check file:joinPath(dir, getFileName(entry.absPath))); + resources.push(check file:joinPath(dir, check file:basename(entry.absPath))); } } } on fail error e { - log:printWarn(string `Failed to read directory ${dir}`, 'error = e); + log:printDebug(string `Failed to read directory ${dir}`, 'error = e); } } return resources; } -function getFileName(string path) returns string { - int? lastSlash = path.lastIndexOf("/") ?: path.lastIndexOf("\\"); - return lastSlash is int ? path.substring(lastSlash + 1) : path; -} - function buildSkillCatalog(map skills) returns string? { if skills.length() == 0 { return (); @@ -205,26 +177,27 @@ isolated class SkillsToolKit { # + return - the skill's full instructions and list of available resource files @ai:AgentTool isolated function activate_skill(string name) returns string|error { - lock { - if !self.skills.hasKey(name) { - return error(string `Skill '${name}' not found. Available skills: ${ - string:'join(", ", ...self.skills.keys())}`); - } + if !self.skills.hasKey(name) { + return error(string `Skill '${name}' not found. Available skills: ${ + string:'join(", ", ...self.skills.keys())}`); + } - SkillInfo info = self.skills.get(name); - return xml ` - -${info.body} -${info.resources.length() > 0 ? - xml ` + SkillInfo info = self.skills.get(name); + // Using string templates instead of XML literals to avoid escaping skill body content + // (e.g., angle brackets in code examples would be escaped to </> by XML) + string resourcesSection = info.resources.length() > 0 ? + string ` -${from string res in info.resources select xml `${res}`} +${string:'join("\n", ...from string res in info.resources select string `${res}`)} Use the read_skill_resource tool to read any of these files if needed. -` : xml ``} +` : ""; + return string ` + +${info.body} +${resourcesSection} -`.toString(); - } +`; } # Reads a resource file from a skill's references/ or assets/ directory. @@ -235,29 +208,27 @@ Use the read_skill_resource tool to read any of these files if needed. # + return - the content of the resource file @ai:AgentTool isolated function read_skill_resource(string skillName, string resourcePath) returns string|error { - lock { - if !self.skills.hasKey(skillName) { - return error(string `Skill '${skillName}' not found`); - } - - if !resourcePath.startsWith(string `${REFERENCES_DIR}/`) && - !resourcePath.startsWith(string `${ASSETS_DIR}/`) { - return error(string `Resource path must start with '${REFERENCES_DIR}/' or '${ASSETS_DIR}/'`); - } + if !self.skills.hasKey(skillName) { + return error(string `Skill '${skillName}' not found`); + } - if resourcePath.indexOf("../") != () || resourcePath.indexOf("..\\") != () { - return error("Path traversal is not allowed in resource paths"); - } + string[] segments = check file:splitPath(resourcePath); + if segments.length() < 2 || (segments[0] != REFERENCES_DIR && segments[0] != ASSETS_DIR) { + return error(string `Resource path must start with '${REFERENCES_DIR}/' or '${ASSETS_DIR}/'`); + } - SkillInfo info = self.skills.get(skillName); - if info.resources.indexOf(resourcePath) is () { - return error(string `Resource '${resourcePath}' not found in skill '${ - skillName}'. Available: ${string:'join(", ", ...info.resources)}`); - } + if resourcePath.indexOf("..") != () { + return error("Path traversal is not allowed in resource paths"); + } - string fullPath = check file:joinPath(info.basePath, resourcePath); - return io:fileReadString(fullPath); + SkillInfo info = self.skills.get(skillName); + if info.resources.indexOf(resourcePath) is () { + return error(string `Resource '${resourcePath}' not found in skill '${ + skillName}'. Available: ${string:'join(", ", ...info.resources)}`); } + + string fullPath = check file:joinPath(info.basePath, resourcePath); + return io:fileReadString(fullPath); } public isolated function getTools() returns ai:ToolConfig[] => diff --git a/ballerina-interpreter/tests/skills/duplicate_names/error_translator/SKILL.md b/ballerina-interpreter/tests/skills/duplicate_names/error_translator/SKILL.md new file mode 100644 index 0000000..81cc440 --- /dev/null +++ b/ballerina-interpreter/tests/skills/duplicate_names/error_translator/SKILL.md @@ -0,0 +1,8 @@ +--- +name: duplicate-skill +description: Translate error messages to user-friendly language +--- + +This is the first occurrence of the error-translator skill. + +Rewrite technical error messages so non-technical users can understand the problem and know what to do next. diff --git a/ballerina-interpreter/tests/skills/duplicate_names/error_translator_dup/SKILL.md b/ballerina-interpreter/tests/skills/duplicate_names/error_translator_dup/SKILL.md new file mode 100644 index 0000000..c7a94cd --- /dev/null +++ b/ballerina-interpreter/tests/skills/duplicate_names/error_translator_dup/SKILL.md @@ -0,0 +1,8 @@ +--- +name: duplicate-skill +description: Translate error messages (duplicate — should be skipped) +--- + +This is the duplicate occurrence of the error-translator skill. + +This is an intentional duplicate to test that the first occurrence wins. diff --git a/ballerina-interpreter/tests/skills/empty_dir/.gitkeep b/ballerina-interpreter/tests/skills/empty_dir/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/ballerina-interpreter/tests/skills/invalid_skill/SKILL.md b/ballerina-interpreter/tests/skills/invalid_skill/SKILL.md new file mode 100644 index 0000000..d46488d --- /dev/null +++ b/ballerina-interpreter/tests/skills/invalid_skill/SKILL.md @@ -0,0 +1,6 @@ +--- +name: "" +description: "" +--- + +This skill has empty required fields. diff --git a/ballerina-interpreter/tests/skills/multi_skills/pr_summary/SKILL.md b/ballerina-interpreter/tests/skills/multi_skills/pr_summary/SKILL.md new file mode 100644 index 0000000..b995c13 --- /dev/null +++ b/ballerina-interpreter/tests/skills/multi_skills/pr_summary/SKILL.md @@ -0,0 +1,12 @@ +--- +name: pr-summary +description: Summarize pull requests into concise changelog entries +--- + +Summarize each merged pull request into a single changelog line. + +When a PR is merged, produce a one-line changelog entry in the format: + +- ``: (#) + +Where type is one of: feat, fix, refactor, docs, test, chore. diff --git a/ballerina-interpreter/tests/skills/multi_skills/security_review/SKILL.md b/ballerina-interpreter/tests/skills/multi_skills/security_review/SKILL.md new file mode 100644 index 0000000..569e4be --- /dev/null +++ b/ballerina-interpreter/tests/skills/multi_skills/security_review/SKILL.md @@ -0,0 +1,15 @@ +--- +name: security-review +description: Review code for security vulnerabilities using OWASP guidelines +--- + +Perform a security-focused code review using OWASP guidelines. + +When reviewing code, check for: + +1. Injection flaws (SQL, command, path traversal) +2. Broken authentication or session management +3. Sensitive data exposure in logs or responses + +Consult the reference document for the full OWASP checklist. +Use the response template in assets/ to structure your findings. diff --git a/ballerina-interpreter/tests/skills/multi_skills/security_review/assets/template.json b/ballerina-interpreter/tests/skills/multi_skills/security_review/assets/template.json new file mode 100644 index 0000000..c55aa34 --- /dev/null +++ b/ballerina-interpreter/tests/skills/multi_skills/security_review/assets/template.json @@ -0,0 +1 @@ +{"template": "security-review", "version": 1, "sections": ["summary", "findings", "severity", "remediation"]} diff --git a/ballerina-interpreter/tests/skills/multi_skills/security_review/references/REFERENCE.md b/ballerina-interpreter/tests/skills/multi_skills/security_review/references/REFERENCE.md new file mode 100644 index 0000000..e2f3b02 --- /dev/null +++ b/ballerina-interpreter/tests/skills/multi_skills/security_review/references/REFERENCE.md @@ -0,0 +1,11 @@ +# OWASP Top 10 Quick Reference + +This is a reference file for security-review. + +| # | Category | +|---|---| +| A01 | Broken Access Control | +| A02 | Cryptographic Failures | +| A03 | Injection | +| A04 | Insecure Design | +| A05 | Security Misconfiguration | diff --git a/ballerina-interpreter/tests/skills/no_frontmatter/SKILL.md b/ballerina-interpreter/tests/skills/no_frontmatter/SKILL.md new file mode 100644 index 0000000..deafd5b --- /dev/null +++ b/ballerina-interpreter/tests/skills/no_frontmatter/SKILL.md @@ -0,0 +1,2 @@ +This file has no frontmatter at all. +Just plain content without any --- delimiters. diff --git a/ballerina-interpreter/tests/skills/partial_invalid/invalid_skill/SKILL.md b/ballerina-interpreter/tests/skills/partial_invalid/invalid_skill/SKILL.md new file mode 100644 index 0000000..b6992b1 --- /dev/null +++ b/ballerina-interpreter/tests/skills/partial_invalid/invalid_skill/SKILL.md @@ -0,0 +1,6 @@ +--- +name: "" +description: Invalid because name is empty +--- + +This skill has an empty name and should cause a parse error diff --git a/ballerina-interpreter/tests/skills/partial_invalid/valid_skill/SKILL.md b/ballerina-interpreter/tests/skills/partial_invalid/valid_skill/SKILL.md new file mode 100644 index 0000000..650cf63 --- /dev/null +++ b/ballerina-interpreter/tests/skills/partial_invalid/valid_skill/SKILL.md @@ -0,0 +1,6 @@ +--- +name: valid-skill +description: Convert JSON payloads to Markdown tables +--- + +Given a JSON array of objects, produce a Markdown table with column headers derived from the object keys. diff --git a/ballerina-interpreter/tests/skills/single_skill/SKILL.md b/ballerina-interpreter/tests/skills/single_skill/SKILL.md new file mode 100644 index 0000000..9a700da --- /dev/null +++ b/ballerina-interpreter/tests/skills/single_skill/SKILL.md @@ -0,0 +1,12 @@ +--- +name: test-gen +description: Generate and run unit tests for code changes +--- + +When asked to test code, follow these steps: + +1. Identify the functions or modules that changed. +2. Write unit tests covering the happy path and key edge cases. +3. Run the tests and report results. + +Keep tests focused — one assertion per behavior. This is the body of the test skill. diff --git a/ballerina-interpreter/tests/skills/source1/release_notes/SKILL.md b/ballerina-interpreter/tests/skills/source1/release_notes/SKILL.md new file mode 100644 index 0000000..f00d7d8 --- /dev/null +++ b/ballerina-interpreter/tests/skills/source1/release_notes/SKILL.md @@ -0,0 +1,8 @@ +--- +name: same-skill +description: Draft release notes from recent commits +--- + +This is from source1. + +Scan the commit log since the last tag and produce categorized release notes (Added, Changed, Fixed, Removed). diff --git a/ballerina-interpreter/tests/skills/source2/release_notes/SKILL.md b/ballerina-interpreter/tests/skills/source2/release_notes/SKILL.md new file mode 100644 index 0000000..4154a9b --- /dev/null +++ b/ballerina-interpreter/tests/skills/source2/release_notes/SKILL.md @@ -0,0 +1,8 @@ +--- +name: same-skill +description: Draft release notes (duplicate — should be skipped) +--- + +This is from source2. + +Intentional cross-source duplicate to verify first-source-wins behavior. diff --git a/ballerina-interpreter/tests/skills_test.bal b/ballerina-interpreter/tests/skills_test.bal new file mode 100644 index 0000000..bf1b877 --- /dev/null +++ b/ballerina-interpreter/tests/skills_test.bal @@ -0,0 +1,599 @@ +// Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/file; +import ballerina/test; + +// ============================================ +// parseSkillMdContent — pure function tests +// ============================================ + +@test:Config +function testParseSkillMdContentValid() returns error? { + string content = string `--- +name: lint-fix +description: Auto-fix linting errors across the project +--- + +Run the configured linter and apply safe auto-fixes.`; + SkillInfo info = check parseSkillMdContent(content, "/base/path", ["references/doc.md"]); + test:assertEquals(info.name, "lint-fix"); + test:assertEquals(info.description, "Auto-fix linting errors across the project"); + test:assertEquals(info.body, "Run the configured linter and apply safe auto-fixes."); + test:assertEquals(info.basePath, "/base/path"); + test:assertEquals(info.resources, ["references/doc.md"]); +} + +@test:Config +function testParseSkillMdContentMissingFrontmatterOpener() { + string content = "No frontmatter here\nJust text."; + SkillInfo|error result = parseSkillMdContent(content, "/base", []); + if result is SkillInfo { + test:assertFail("Expected error for missing frontmatter"); + } + test:assertTrue(result.message().includes("must start with YAML frontmatter")); +} + +@test:Config +function testParseSkillMdContentUnclosedFrontmatter() { + string content = string `--- +name: deploy-check +description: Validate deployment readiness`; + SkillInfo|error result = parseSkillMdContent(content, "/base", []); + if result is SkillInfo { + test:assertFail("Expected error for unclosed frontmatter"); + } + test:assertTrue(result.message().includes("not closed")); +} + +@test:Config +function testParseSkillMdContentEmptyName() { + string content = string `--- +name: "" +description: Scaffold REST endpoints from OpenAPI specs +--- + +Body.`; + SkillInfo|error result = parseSkillMdContent(content, "/base", []); + if result is SkillInfo { + test:assertFail("Expected error for empty name"); + } + test:assertTrue(result.message().includes("'name' field is required")); +} + +@test:Config +function testParseSkillMdContentEmptyDescription() { + string content = string `--- +name: api-scaffold +description: "" +--- + +Body.`; + SkillInfo|error result = parseSkillMdContent(content, "/base", []); + if result is SkillInfo { + test:assertFail("Expected error for empty description"); + } + test:assertTrue(result.message().includes("'description' field is required")); +} + +@test:Config +function testParseSkillMdContentBodyWithResources() returns error? { + string content = string `--- +name: db-migrate +description: Generate and validate database migration scripts +--- + +Read the schema diff and produce migration SQL.`; + string[] resources = ["references/README.md", "assets/config.json"]; + SkillInfo info = check parseSkillMdContent(content, "/skills/db-migrate", resources); + test:assertEquals(info.resources, resources); + test:assertEquals(info.body, "Read the schema diff and produce migration SQL."); +} + +// ============================================ +// buildSkillCatalog — pure function tests +// ============================================ + +@test:Config +function testBuildSkillCatalogEmpty() { + map skills = {}; + string? catalog = buildSkillCatalog(skills); + test:assertTrue(catalog is ()); +} + +@test:Config +function testBuildSkillCatalogSingleSkill() { + map skills = { + "commit-msg": { + name: "commit-msg", + description: "Draft conventional commit messages from staged diffs", + body: "", + basePath: "", + resources: [] + } + }; + string? catalog = buildSkillCatalog(skills); + if catalog is () { + test:assertFail("Expected non-nil catalog"); + } + test:assertTrue(catalog.includes("commit-msg")); + test:assertTrue(catalog.includes("Draft conventional commit messages from staged diffs")); + test:assertTrue(catalog.includes("Available Skills")); +} + +@test:Config +function testBuildSkillCatalogMultipleSkills() { + map skills = { + "changelog": { + name: "changelog", + description: "Generate changelog entries from merged PRs", + body: "", + basePath: "", + resources: [] + }, + "dep-audit": { + name: "dep-audit", + description: "Audit dependencies for known vulnerabilities", + body: "", + basePath: "", + resources: [] + } + }; + string? catalog = buildSkillCatalog(skills); + if catalog is () { + test:assertFail("Expected non-nil catalog"); + } + test:assertTrue(catalog.includes("changelog")); + test:assertTrue(catalog.includes("dep-audit")); + test:assertTrue(catalog.includes("Generate changelog entries from merged PRs")); + test:assertTrue(catalog.includes("Audit dependencies for known vulnerabilities")); +} + +// ============================================ +// SkillsToolKit class tests +// ============================================ + +@test:Config +function testActivateSkillFound() returns error? { + map skills = { + "doc-gen": { + name: "doc-gen", + description: "Generate API documentation from source annotations", + body: "Scan exported symbols and produce Markdown API docs.", + basePath: "/skills/doc-gen", + resources: [] + } + }; + SkillsToolKit toolkit = new (skills); + string result = check toolkit.activate_skill("doc-gen"); + test:assertTrue(result.includes("Scan exported symbols and produce Markdown API docs.")); + test:assertTrue(result.includes("doc-gen")); +} + +@test:Config +function testActivateSkillFoundWithResources() returns error? { + map skills = { + "perf-profile": { + name: "perf-profile", + description: "Profile hot paths and suggest optimizations", + body: "Instrument the critical path and collect flame-graph data.", + basePath: "/skills/perf-profile", + resources: ["references/guide.md", "assets/schema.json"] + } + }; + SkillsToolKit toolkit = new (skills); + string result = check toolkit.activate_skill("perf-profile"); + test:assertTrue(result.includes("Instrument the critical path and collect flame-graph data.")); + test:assertTrue(result.includes("skill_resources")); + test:assertTrue(result.includes("references/guide.md")); + test:assertTrue(result.includes("assets/schema.json")); + test:assertTrue(result.includes("read_skill_resource")); +} + +@test:Config +function testActivateSkillNotFound() { + map skills = { + "format-sql": { + name: "format-sql", + description: "Format SQL queries to follow team style guide", + body: "Parse and reformat SQL using the project conventions.", + basePath: "/skills/format-sql", + resources: [] + } + }; + SkillsToolKit toolkit = new (skills); + string|error result = toolkit.activate_skill("nonexistent"); + if result is string { + test:assertFail("Expected error for nonexistent skill"); + } + test:assertTrue(result.message().includes("not found")); + test:assertTrue(result.message().includes("format-sql")); +} + +@test:Config +function testReadSkillResourceInvalidSkillName() { + map skills = {}; + SkillsToolKit toolkit = new (skills); + string|error result = toolkit.read_skill_resource("unknown", "references/file.md"); + if result is string { + test:assertFail("Expected error for unknown skill"); + } + test:assertTrue(result.message().includes("not found")); +} + +@test:Config +function testReadSkillResourceInvalidPathPrefix() { + map skills = { + "env-check": { + name: "env-check", + description: "Validate environment variables before deploy", + body: "Check required env vars.", + basePath: "/skills/env-check", + resources: ["references/env-spec.md"] + } + }; + SkillsToolKit toolkit = new (skills); + string|error result = toolkit.read_skill_resource("env-check", "other/file.txt"); + if result is string { + test:assertFail("Expected error for invalid path prefix"); + } + test:assertTrue(result.message().includes("must start with")); +} + +@test:Config +function testReadSkillResourcePathTraversal() { + map skills = { + "env-check": { + name: "env-check", + description: "Validate environment variables before deploy", + body: "Check required env vars.", + basePath: "/skills/env-check", + resources: ["references/env-spec.md"] + } + }; + SkillsToolKit toolkit = new (skills); + string|error result = toolkit.read_skill_resource("env-check", "references/../../../etc/passwd"); + if result is string { + test:assertFail("Expected error for path traversal"); + } + test:assertTrue(result.message().includes("traversal")); +} + +@test:Config +function testReadSkillResourceNotListed() { + map skills = { + "env-check": { + name: "env-check", + description: "Validate environment variables before deploy", + body: "Check required env vars.", + basePath: "/skills/env-check", + resources: ["references/env-spec.md"] + } + }; + SkillsToolKit toolkit = new (skills); + string|error result = toolkit.read_skill_resource("env-check", "references/other.md"); + if result is string { + test:assertFail("Expected error for unlisted resource"); + } + test:assertTrue(result.message().includes("not found in skill")); +} + +// ============================================ +// Filesystem-dependent tests (using fixtures) +// ============================================ + +@test:Config +function testDiscoverLocalSkillsSingleSkillDir() returns error? { + string absPath = check file:getAbsolutePath("tests/skills/single_skill"); + map skills = check discoverLocalSkills(absPath); + test:assertEquals(skills.length(), 1); + test:assertTrue(skills.hasKey("test-gen")); + SkillInfo info = skills.get("test-gen"); + test:assertEquals(info.description, "Generate and run unit tests for code changes"); + test:assertTrue(info.body.includes("body of the test skill")); +} + +@test:Config +function testDiscoverLocalSkillsMultiSkillDir() returns error? { + string absPath = check file:getAbsolutePath("tests/skills/multi_skills"); + map skills = check discoverLocalSkills(absPath); + test:assertEquals(skills.length(), 2); + test:assertTrue(skills.hasKey("pr-summary")); + test:assertTrue(skills.hasKey("security-review")); +} + +@test:Config +function testDiscoverLocalSkillsInvalidSkill() returns error? { + string absPath = check file:getAbsolutePath("tests/skills/invalid_skill"); + map|error result = discoverLocalSkills(absPath); + // The skill has empty name, so parseSkillMdContent returns error. + // Since it's a direct SKILL.md (not subdirectory), the error propagates. + test:assertTrue(result is error); +} + +@test:Config +function testDiscoverSkillsMultipleSources() returns error? { + string testDir = check file:getAbsolutePath("tests/skills"); + SkillSource[] sources = [ + {path: "single_skill"}, + {path: "multi_skills"} + ]; + map skills = check discoverSkills(sources, testDir); + test:assertEquals(skills.length(), 3); + test:assertTrue(skills.hasKey("test-gen")); + test:assertTrue(skills.hasKey("pr-summary")); + test:assertTrue(skills.hasKey("security-review")); +} + +@test:Config +function testParseSkillMdValidFile() returns error? { + string absPath = check file:getAbsolutePath("tests/skills/single_skill"); + string skillMdPath = check file:joinPath(absPath, "SKILL.md"); + SkillInfo info = check parseSkillMd(skillMdPath, absPath); + test:assertEquals(info.name, "test-gen"); + test:assertEquals(info.description, "Generate and run unit tests for code changes"); + test:assertTrue(info.body.includes("body of the test skill")); +} + +@test:Config +function testListLocalResourcesWithReferencesAndAssets() returns error? { + string absPath = check file:getAbsolutePath("tests/skills/multi_skills/security_review"); + string[] resources = listLocalResources(absPath); + test:assertEquals(resources.length(), 2); + // Check that both references and assets are found + boolean hasReference = false; + boolean hasAsset = false; + foreach string res in resources { + if res.includes("references/") && res.includes("REFERENCE.md") { + hasReference = true; + } + if res.includes("assets/") && res.includes("template.json") { + hasAsset = true; + } + } + test:assertTrue(hasReference, "Should find references/REFERENCE.md"); + test:assertTrue(hasAsset, "Should find assets/template.json"); +} + +@test:Config +function testListLocalResourcesNoResources() returns error? { + string absPath = check file:getAbsolutePath("tests/skills/single_skill"); + string[] resources = listLocalResources(absPath); + test:assertEquals(resources.length(), 0); +} + +@test:Config +function testExtractSkillCatalogNullSkills() returns error? { + AgentMetadata metadata = {}; + string|[string, SkillsToolKit]|error? result = extractSkillCatalog(metadata, "."); + test:assertTrue(result is ()); +} + +@test:Config +function testExtractSkillCatalogEmptySkillsArray() returns error? { + AgentMetadata metadata = {skills: []}; + string|[string, SkillsToolKit]|error? result = extractSkillCatalog(metadata, "."); + test:assertTrue(result is ()); +} + +// ============================================ +// Progressive disclosure E2E test +// ============================================ +// Walks the full chain using real fixtures: +// discover → catalog (names only) → activate (body revealed) → read resource (content revealed) + +@test:Config +function testProgressiveDisclosureEndToEnd() returns error? { + string testDir = check file:getAbsolutePath("tests/skills"); + + // Step 1: Discover skills from multi_skills fixture (has resources on security_review) + map skills = check discoverLocalSkills( + check file:joinPath(testDir, "multi_skills")); + test:assertEquals(skills.length(), 2); + + // Step 2: Build catalog — should contain ONLY names and descriptions, NOT bodies + string? catalog = buildSkillCatalog(skills); + if catalog is () { + test:assertFail("Catalog should be non-nil for discovered skills"); + } + + // Catalog MUST include names and descriptions + test:assertTrue(catalog.includes("pr-summary")); + test:assertTrue(catalog.includes("Summarize pull requests")); + test:assertTrue(catalog.includes("security-review")); + test:assertTrue(catalog.includes("Review code for security vulnerabilities")); + + // Catalog MUST NOT include skill bodies (progressive disclosure) + test:assertFalse(catalog.includes("Summarize each merged pull request"), + "Catalog must not leak pr-summary body"); + test:assertFalse(catalog.includes("Perform a security-focused code review"), + "Catalog must not leak security-review body"); + + // Step 3: Activate skill — body is now revealed + SkillsToolKit toolkit = new (skills); + string activateResult = check toolkit.activate_skill("security-review"); + + // Activation MUST include the full body + test:assertTrue(activateResult.includes("Perform a security-focused code review"), + "activate_skill must return the skill body"); + + // Activation MUST list available resources + test:assertTrue(activateResult.includes("skill_resources"), + "activate_skill must include skill_resources section"); + test:assertTrue(activateResult.includes("references/REFERENCE.md")); + test:assertTrue(activateResult.includes("assets/template.json")); + + // Step 4: Read a resource — file content is now revealed + string refContent = check toolkit.read_skill_resource("security-review", "references/REFERENCE.md"); + test:assertTrue(refContent.includes("OWASP Top 10 Quick Reference"), + "read_skill_resource must return actual file content"); + test:assertTrue(refContent.includes("reference file for security-review")); + + string assetContent = check toolkit.read_skill_resource("security-review", "assets/template.json"); + test:assertTrue(assetContent.includes("security-review"), + "read_skill_resource must return asset file content"); + + // Step 5: Verify non-resource skill has no resources disclosed + string alphaResult = check toolkit.activate_skill("pr-summary"); + test:assertTrue(alphaResult.includes("Summarize each merged pull request")); + test:assertFalse(alphaResult.includes("skill_resources"), + "pr-summary has no resources, so no skill_resources section"); +} + +@test:Config +function testProgressiveDisclosureViaExtractSkillCatalog() returns error? { + string testDir = check file:getAbsolutePath("tests/skills"); + AgentMetadata metadata = { + skills: [ + {path: "single_skill"}, + {path: "multi_skills"} + ] + }; + + // extractSkillCatalog returns [catalog, toolkit] — same entry point used by createAgent + var result = check extractSkillCatalog(metadata, testDir); + if result is () { + test:assertFail("Should return catalog and toolkit for valid skills"); + } + var [catalogStr, toolkit] = result; + + // Catalog has all 3 skills' names + test:assertTrue(catalogStr.includes("test-gen")); + test:assertTrue(catalogStr.includes("pr-summary")); + test:assertTrue(catalogStr.includes("security-review")); + + // Catalog does NOT have any body content + test:assertFalse(catalogStr.includes("body of the test skill"), + "Catalog must not contain test-gen body"); + test:assertFalse(catalogStr.includes("Summarize each merged pull request"), + "Catalog must not contain pr-summary body"); + test:assertFalse(catalogStr.includes("Perform a security-focused code review"), + "Catalog must not contain security-review body"); + + // Activating reveals the body + string body = check toolkit.activate_skill("test-gen"); + test:assertTrue(body.includes("body of the test skill"), + "activate_skill must reveal the full body"); +} + +// ============================================ +// Duplicate detection tests +// ============================================ + +@test:Config +function testDiscoverLocalSkillsDuplicateSubdirectories() returns error? { + // Test that duplicate skill names in subdirectories are detected + // First occurrence is kept, second is logged and skipped + string absPath = check file:getAbsolutePath("tests/skills/duplicate_names"); + map skills = check discoverLocalSkills(absPath); + + // Only one skill should be discovered despite two subdirectories with same name + test:assertEquals(skills.length(), 1); + test:assertTrue(skills.hasKey("duplicate-skill")); + + // First occurrence (error_translator) should be kept + SkillInfo info = skills.get("duplicate-skill"); + test:assertEquals(info.description, "Translate error messages to user-friendly language"); + test:assertTrue(info.body.includes("first occurrence")); +} + +@test:Config +function testDiscoverSkillsDuplicateFromMultipleSources() returns error? { + // Test that duplicate skill names across multiple sources are detected + // First source's skill is kept, subsequent duplicates are skipped (covers lines 54-55) + string testDir = check file:getAbsolutePath("tests/skills"); + SkillSource[] sources = [ + {path: "source1"}, // has "same-skill" + {path: "source2"} // also has "same-skill" (should be skipped) + ]; + + map skills = check discoverSkills(sources, testDir); + + // Should have only one skill (first source wins) + test:assertEquals(skills.length(), 1); + test:assertTrue(skills.hasKey("same-skill")); + + // Verify it's from source1 (first occurrence) + SkillInfo info = skills.get("same-skill"); + test:assertEquals(info.description, "Draft release notes from recent commits"); + test:assertTrue(info.body.includes("source1")); +} + +// ============================================ +// Error handling in subdirectory discovery +// ============================================ + +@test:Config +function testDiscoverLocalSkillsSkipsInvalidSubdir() returns error? { + // Test that invalid skills in subdirectories are skipped with error logging + // Valid skills in other subdirectories are still discovered + string absPath = check file:getAbsolutePath("tests/skills/partial_invalid"); + map skills = check discoverLocalSkills(absPath); + + // Should discover only the valid skill + test:assertEquals(skills.length(), 1); + test:assertTrue(skills.hasKey("valid-skill")); + test:assertFalse(skills.hasKey("")); // Invalid skill has empty name + + SkillInfo validInfo = skills.get("valid-skill"); + test:assertEquals(validInfo.description, "Convert JSON payloads to Markdown tables"); +} + +// ============================================ +// SkillsToolKit getTools test +// ============================================ + +@test:Config +function testSkillsToolKitGetTools() returns error? { + // Test that getTools() returns the correct tool configuration + map skills = { + "refactor": { + name: "refactor", + description: "Extract repeated logic into reusable functions", + body: "Identify duplication and extract helper functions.", + basePath: "/skills/refactor", + resources: [] + } + }; + + SkillsToolKit toolkit = new (skills); + var tools = toolkit.getTools(); + + // Should return 2 tools: activate_skill and read_skill_resource + test:assertEquals(tools.length(), 2); + + // Verify both tools are present by checking the function references + // The tools array contains ToolConfig records with the function metadata + test:assertTrue(tools.length() == 2, "Should have exactly 2 tools"); +} + +// ============================================ +// Empty skills directory test (line 36) +// ============================================ + +@test:Config +function testExtractSkillCatalogEmptyDirectory() returns error? { + AgentMetadata metadata = { + skills: [ + {path: "tests/skills/empty_dir"} + ] + }; + + [string, SkillsToolKit]|error? result = extractSkillCatalog(metadata, "."); + + // When no valid skills are found, should return () + test:assertTrue(result is (), "Should return () when no skills are found"); +} From 5a025fabec8cd9102ac6e2254074eea55c22e66d Mon Sep 17 00:00:00 2001 From: MaryamZi Date: Fri, 13 Mar 2026 14:33:21 +0530 Subject: [PATCH 3/9] Refactor code --- ballerina-interpreter/skills.bal | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ballerina-interpreter/skills.bal b/ballerina-interpreter/skills.bal index 065b820..3908b8d 100644 --- a/ballerina-interpreter/skills.bal +++ b/ballerina-interpreter/skills.bal @@ -61,7 +61,6 @@ function discoverSkills(SkillSource[] sources, string afmFileDir) returns map|error { - map skills = {}; string absPath = check file:getAbsolutePath(path); string skillMdPath = check file:joinPath(absPath, SKILL_FILE); @@ -70,6 +69,7 @@ function discoverLocalSkills(string path) returns map|error { return {[info.name]: info}; } + map skills = {}; file:MetaData[] entries = check file:readDir(absPath); foreach file:MetaData entry in entries { if !entry.dir { @@ -217,7 +217,7 @@ ${resourcesSection} return error(string `Resource path must start with '${REFERENCES_DIR}/' or '${ASSETS_DIR}/'`); } - if resourcePath.indexOf("..") != () { + if segments.indexOf("..") != () { return error("Path traversal is not allowed in resource paths"); } From f5e2bb0ea376b872b57df7997610e631e35bacfa Mon Sep 17 00:00:00 2001 From: MaryamZi Date: Fri, 13 Mar 2026 16:12:44 +0530 Subject: [PATCH 4/9] Address review suggestions --- ballerina-interpreter/skills.bal | 15 +++++++++++++-- ballerina-interpreter/tests/skills_test.bal | 4 ---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/ballerina-interpreter/skills.bal b/ballerina-interpreter/skills.bal index 3908b8d..98a67ad 100644 --- a/ballerina-interpreter/skills.bal +++ b/ballerina-interpreter/skills.bal @@ -44,9 +44,17 @@ type SkillFrontmatter record { function discoverSkills(SkillSource[] sources, string afmFileDir) returns map|error { map skills = {}; + string normalizedAfmDir = check file:normalizePath(check file:getAbsolutePath(afmFileDir), file:CLEAN); foreach SkillSource 'source in sources { + if check file:isAbsolutePath('source.path) { + return error(string `Skill source path must be relative, but got: ${'source.path}`); + } string resolvedPath = check file:joinPath(afmFileDir, 'source.path); + string normalizedPath = check file:normalizePath(check file:getAbsolutePath(resolvedPath), file:CLEAN); + if !normalizedPath.startsWith(normalizedAfmDir) { + return error(string `Skill source path '${'source.path}' resolves outside the AFM file directory`); + } map localSkills = check discoverLocalSkills(resolvedPath); foreach [string, SkillInfo] [name, info] in localSkills.entries() { if skills.hasKey(name) { @@ -125,14 +133,17 @@ function listLocalResources(string basePath) returns string[] { foreach string dir in [REFERENCES_DIR, ASSETS_DIR] { do { string dirPath = check file:joinPath(basePath, dir); + if !check file:test(dirPath, file:EXISTS) { + continue; + } file:MetaData[] entries = check file:readDir(dirPath); foreach file:MetaData entry in entries { if !entry.dir { - resources.push(check file:joinPath(dir, check file:basename(entry.absPath))); + resources.push(string `${dir}/${check file:basename(entry.absPath)}`); } } } on fail error e { - log:printDebug(string `Failed to read directory ${dir}`, 'error = e); + log:printWarn(string `Failed to read directory ${dir}`, 'error = e); } } return resources; diff --git a/ballerina-interpreter/tests/skills_test.bal b/ballerina-interpreter/tests/skills_test.bal index bf1b877..54f4afc 100644 --- a/ballerina-interpreter/tests/skills_test.bal +++ b/ballerina-interpreter/tests/skills_test.bal @@ -574,10 +574,6 @@ function testSkillsToolKitGetTools() returns error? { // Should return 2 tools: activate_skill and read_skill_resource test:assertEquals(tools.length(), 2); - - // Verify both tools are present by checking the function references - // The tools array contains ToolConfig records with the function metadata - test:assertTrue(tools.length() == 2, "Should have exactly 2 tools"); } // ============================================ From 13fff5bf3d1b8404a3c5eb13c134f113ff824813 Mon Sep 17 00:00:00 2001 From: MaryamZi Date: Sat, 14 Mar 2026 12:14:01 +0530 Subject: [PATCH 5/9] Make metadata optional --- ballerina-interpreter/main.bal | 4 ++-- ballerina-interpreter/parser.bal | 18 +++++++++++++++--- ballerina-interpreter/skills.bal | 2 +- ballerina-interpreter/tests/main_test.bal | 12 ++++++------ ballerina-interpreter/types.bal | 2 +- ballerina-interpreter/web_chat_ui.bal | 8 ++++---- 6 files changed, 29 insertions(+), 17 deletions(-) diff --git a/ballerina-interpreter/main.bal b/ballerina-interpreter/main.bal index ebadc8f..c43119a 100644 --- a/ballerina-interpreter/main.bal +++ b/ballerina-interpreter/main.bal @@ -50,9 +50,9 @@ public function main(string? filePath = ()) returns error? { } function runAgentFromAFM(AFMRecord afm, int port, string afmFileDir) returns error? { - AgentMetadata metadata = afm.metadata; + AgentMetadata? metadata = afm?.metadata; - Interface[] agentInterfaces = metadata.interfaces ?: [{}]; + Interface[] agentInterfaces = metadata?.interfaces ?: [{}]; var [consoleChatInterface, webChatInterface, webhookInterface] = check validateAndExtractInterfaces(agentInterfaces); diff --git a/ballerina-interpreter/parser.bal b/ballerina-interpreter/parser.bal index d12296e..3158c80 100644 --- a/ballerina-interpreter/parser.bal +++ b/ballerina-interpreter/parser.bal @@ -20,8 +20,15 @@ import ballerina/os; function parseAfm(string content) returns AFMRecord|error { string resolvedContent = check resolveVariables(content); - [map, string] [frontmatterMap, body] = check extractFrontmatter(resolvedContent); - AgentMetadata metadata = check frontmatterMap.fromJsonWithType(); + AgentMetadata? metadata = (); + string body; + if resolvedContent.startsWith(FRONTMATTER_DELIMITER) { + map frontmatterMap; + [frontmatterMap, body] = check extractFrontmatter(resolvedContent); + metadata = check frontmatterMap.fromJsonWithType(); + } else { + body = resolvedContent; + } // Extract Role and Instructions sections string[] bodyLines = splitLines(body); @@ -140,7 +147,12 @@ function validateHttpVariables(AFMRecord afmRecord) returns error? { return error("http: variables are only supported in webhook prompt fields, found in instructions section"); } - AgentMetadata {authors, provider, model, interfaces, tools, skills, max_iterations: _, ...rest} = afmRecord.metadata; + AgentMetadata? metadata = afmRecord?.metadata; + if metadata is () { + return; + } + + AgentMetadata {authors, provider, model, interfaces, tools, skills, max_iterations: _, ...rest} = metadata; string[] erroredKeys = []; diff --git a/ballerina-interpreter/skills.bal b/ballerina-interpreter/skills.bal index 98a67ad..feb6d10 100644 --- a/ballerina-interpreter/skills.bal +++ b/ballerina-interpreter/skills.bal @@ -23,7 +23,7 @@ const SKILL_FILE = "SKILL.md"; const REFERENCES_DIR = "references"; const ASSETS_DIR = "assets"; -function extractSkillCatalog(AgentMetadata metadata, string afmFileDir) returns [string, SkillsToolKit]|error? { +function extractSkillCatalog(AgentMetadata? metadata, string afmFileDir) returns [string, SkillsToolKit]|error? { SkillSource[]? skillSources = metadata?.skills; if skillSources is () || skillSources.length() == 0 { return (); diff --git a/ballerina-interpreter/tests/main_test.bal b/ballerina-interpreter/tests/main_test.bal index 3df4ac9..e9ad324 100644 --- a/ballerina-interpreter/tests/main_test.bal +++ b/ballerina-interpreter/tests/main_test.bal @@ -251,17 +251,17 @@ function testValidateHttpVariablesInStdioTransportEnv() { } @test:Config -function testParseAfmWithoutFrontmatter() { +function testParseAfmWithoutFrontmatter() returns error? { string content = string `# Role This is the role. # Instructions These are instructions.`; - AFMRecord|error parsed = parseAfm(content); - if parsed is AFMRecord { - test:assertFail("Expected error when parsing AFM without frontmatter"); - } + AFMRecord parsed = check parseAfm(content); + test:assertTrue(parsed?.metadata is (), "metadata should be absent"); + test:assertEquals(parsed.role, "This is the role."); + test:assertEquals(parsed.instructions, "These are instructions."); } @test:Config @@ -277,7 +277,7 @@ Agent role here. Agent instructions here.`; AFMRecord parsed = check parseAfm(content); - test:assertEquals(parsed.metadata.spec_version, "0.3.0"); + test:assertEquals(parsed?.metadata?.spec_version, "0.3.0"); test:assertEquals(parsed.role, "Agent role here."); test:assertEquals(parsed.instructions, "Agent instructions here."); } diff --git a/ballerina-interpreter/types.bal b/ballerina-interpreter/types.bal index 77ced96..392e409 100644 --- a/ballerina-interpreter/types.bal +++ b/ballerina-interpreter/types.bal @@ -174,7 +174,7 @@ type AgentMetadata record {| |}; type AFMRecord record {| - AgentMetadata metadata; + AgentMetadata metadata?; string role; string instructions; |}; diff --git a/ballerina-interpreter/web_chat_ui.bal b/ballerina-interpreter/web_chat_ui.bal index 36fd1bb..50b60a0 100644 --- a/ballerina-interpreter/web_chat_ui.bal +++ b/ballerina-interpreter/web_chat_ui.bal @@ -17,7 +17,7 @@ import ballerina/http; import ballerina/io; -function attachWebChatUIService(http:Listener httpListener, string chatPath, AgentMetadata metadata) returns error? { +function attachWebChatUIService(http:Listener httpListener, string chatPath, AgentMetadata? metadata) returns error? { string absoluteChatPath = chatPath.startsWith("/") ? chatPath : "/" + chatPath; http:Service webUIService = check new WebUIService(absoluteChatPath, metadata); return httpListener.attach(webUIService, "/chat/ui"); @@ -28,11 +28,11 @@ service class WebUIService { private final string htmlContent; - function init(string chatPath, AgentMetadata metadata) returns error? { + function init(string chatPath, AgentMetadata? metadata) returns error? { string template = check io:fileReadString("resources/chat-ui.html"); - string agentName = metadata.name ?: "AFM Agent Chat"; - string agentDescription = metadata.description ?: "AI Assistant"; + string agentName = metadata?.name ?: "AFM Agent Chat"; + string agentDescription = metadata?.description ?: "AI Assistant"; string escapedPath = escapeForJavaScript(chatPath); string result = template; From 8f5080bd8c6bf59aa5a56cf9c6d3f9cad0e25f7b Mon Sep 17 00:00:00 2001 From: MaryamZi Date: Mon, 16 Mar 2026 10:40:22 +0530 Subject: [PATCH 6/9] Refactor code --- ballerina-interpreter/parser.bal | 7 ++-- ballerina-interpreter/skills.bal | 26 +++++++------- ballerina-interpreter/tests/skills_test.bal | 38 ++++++++++----------- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/ballerina-interpreter/parser.bal b/ballerina-interpreter/parser.bal index 3158c80..8657ffb 100644 --- a/ballerina-interpreter/parser.bal +++ b/ballerina-interpreter/parser.bal @@ -24,7 +24,7 @@ function parseAfm(string content) returns AFMRecord|error { string body; if resolvedContent.startsWith(FRONTMATTER_DELIMITER) { map frontmatterMap; - [frontmatterMap, body] = check extractFrontmatter(resolvedContent); + [frontmatterMap, body] = check extractFrontMatter(resolvedContent); metadata = check frontmatterMap.fromJsonWithType(); } else { body = resolvedContent; @@ -37,8 +37,7 @@ function parseAfm(string content) returns AFMRecord|error { boolean inRole = false; boolean inInstructions = false; - foreach int k in 0 ..< bodyLines.length() { - string line = bodyLines[k]; + foreach string line in bodyLines { string trimmed = line.trim(); if trimmed.startsWith("# ") { @@ -399,7 +398,7 @@ function toolFilterContainsHttpVariable(ToolFilter? filter) returns boolean { // Extracts YAML frontmatter and the remaining body from a document delimited by `---`. // Returns the parsed YAML as a map and the body text after the closing delimiter. -function extractFrontmatter(string content) returns [map, string]|error { +function extractFrontMatter(string content) returns [map, string]|error { string[] lines = splitLines(content); int length = lines.length(); diff --git a/ballerina-interpreter/skills.bal b/ballerina-interpreter/skills.bal index feb6d10..f555ae3 100644 --- a/ballerina-interpreter/skills.bal +++ b/ballerina-interpreter/skills.bal @@ -105,23 +105,25 @@ function discoverLocalSkills(string path) returns map|error { function parseSkillMd(string skillMdPath, string basePath) returns SkillInfo|error { string content = check io:fileReadString(skillMdPath); - return parseSkillMdContent(content, basePath, listLocalResources(basePath)); + string[] resources = listLocalResources(basePath); + return parseSkillMdContent(content, basePath, resources); } function parseSkillMdContent(string content, string basePath, string[] resources) returns SkillInfo|error { - [map, string] [frontmatterMap, body] = check extractFrontmatter(content); - SkillFrontmatter frontmatter = check frontmatterMap.fromJsonWithType(); + [map, string] [frontMatterMap, body] = check extractFrontMatter(content); + SkillFrontmatter {name, description} = check frontMatterMap.fromJsonWithType(); - if frontmatter.name.trim() == "" { + if name.trim() == "" { return error("SKILL.md 'name' field is required and must not be empty"); } - if frontmatter.description.trim() == "" { + + if description.trim() == "" { return error("SKILL.md 'description' field is required and must not be empty"); } return { - name: frontmatter.name, - description: frontmatter.description, + name, + description, body: body.trim(), basePath, resources @@ -158,7 +160,7 @@ function buildSkillCatalog(map skills) returns string? { ## Available Skills The following skills provide specialized instructions for specific tasks. -When a task matches a skill's description, call the activate_skill tool +When a task matches a skill's description, call the activateSkill tool with the skill's name to load its full instructions. ${xml ` @@ -187,7 +189,7 @@ isolated class SkillsToolKit { # + name - the name of the skill to activate (must match a name from the available skills catalog) # + return - the skill's full instructions and list of available resource files @ai:AgentTool - isolated function activate_skill(string name) returns string|error { + isolated function activateSkill(string name) returns string|error { if !self.skills.hasKey(name) { return error(string `Skill '${name}' not found. Available skills: ${ string:'join(", ", ...self.skills.keys())}`); @@ -201,7 +203,7 @@ isolated class SkillsToolKit { ${string:'join("\n", ...from string res in info.resources select string `${res}`)} -Use the read_skill_resource tool to read any of these files if needed. +Use the readSkillResource tool to read any of these files if needed. ` : ""; return string ` @@ -218,7 +220,7 @@ ${resourcesSection} # + resourcePath - relative path to the resource file (e.g., "references/REFERENCE.md" or "assets/template.json") # + return - the content of the resource file @ai:AgentTool - isolated function read_skill_resource(string skillName, string resourcePath) returns string|error { + isolated function readSkillResource(string skillName, string resourcePath) returns string|error { if !self.skills.hasKey(skillName) { return error(string `Skill '${skillName}' not found`); } @@ -243,5 +245,5 @@ ${resourcesSection} } public isolated function getTools() returns ai:ToolConfig[] => - ai:getToolConfigs([self.activate_skill, self.read_skill_resource]); + ai:getToolConfigs([self.activateSkill, self.readSkillResource]); } diff --git a/ballerina-interpreter/tests/skills_test.bal b/ballerina-interpreter/tests/skills_test.bal index 54f4afc..d9622d2 100644 --- a/ballerina-interpreter/tests/skills_test.bal +++ b/ballerina-interpreter/tests/skills_test.bal @@ -178,7 +178,7 @@ function testActivateSkillFound() returns error? { } }; SkillsToolKit toolkit = new (skills); - string result = check toolkit.activate_skill("doc-gen"); + string result = check toolkit.activateSkill("doc-gen"); test:assertTrue(result.includes("Scan exported symbols and produce Markdown API docs.")); test:assertTrue(result.includes("doc-gen")); } @@ -195,12 +195,12 @@ function testActivateSkillFoundWithResources() returns error? { } }; SkillsToolKit toolkit = new (skills); - string result = check toolkit.activate_skill("perf-profile"); + string result = check toolkit.activateSkill("perf-profile"); test:assertTrue(result.includes("Instrument the critical path and collect flame-graph data.")); test:assertTrue(result.includes("skill_resources")); test:assertTrue(result.includes("references/guide.md")); test:assertTrue(result.includes("assets/schema.json")); - test:assertTrue(result.includes("read_skill_resource")); + test:assertTrue(result.includes("readSkillResource")); } @test:Config @@ -215,7 +215,7 @@ function testActivateSkillNotFound() { } }; SkillsToolKit toolkit = new (skills); - string|error result = toolkit.activate_skill("nonexistent"); + string|error result = toolkit.activateSkill("nonexistent"); if result is string { test:assertFail("Expected error for nonexistent skill"); } @@ -227,7 +227,7 @@ function testActivateSkillNotFound() { function testReadSkillResourceInvalidSkillName() { map skills = {}; SkillsToolKit toolkit = new (skills); - string|error result = toolkit.read_skill_resource("unknown", "references/file.md"); + string|error result = toolkit.readSkillResource("unknown", "references/file.md"); if result is string { test:assertFail("Expected error for unknown skill"); } @@ -246,7 +246,7 @@ function testReadSkillResourceInvalidPathPrefix() { } }; SkillsToolKit toolkit = new (skills); - string|error result = toolkit.read_skill_resource("env-check", "other/file.txt"); + string|error result = toolkit.readSkillResource("env-check", "other/file.txt"); if result is string { test:assertFail("Expected error for invalid path prefix"); } @@ -265,7 +265,7 @@ function testReadSkillResourcePathTraversal() { } }; SkillsToolKit toolkit = new (skills); - string|error result = toolkit.read_skill_resource("env-check", "references/../../../etc/passwd"); + string|error result = toolkit.readSkillResource("env-check", "references/../../../etc/passwd"); if result is string { test:assertFail("Expected error for path traversal"); } @@ -284,7 +284,7 @@ function testReadSkillResourceNotListed() { } }; SkillsToolKit toolkit = new (skills); - string|error result = toolkit.read_skill_resource("env-check", "references/other.md"); + string|error result = toolkit.readSkillResource("env-check", "references/other.md"); if result is string { test:assertFail("Expected error for unlisted resource"); } @@ -424,30 +424,30 @@ function testProgressiveDisclosureEndToEnd() returns error? { // Step 3: Activate skill — body is now revealed SkillsToolKit toolkit = new (skills); - string activateResult = check toolkit.activate_skill("security-review"); + string activateResult = check toolkit.activateSkill("security-review"); // Activation MUST include the full body test:assertTrue(activateResult.includes("Perform a security-focused code review"), - "activate_skill must return the skill body"); + "activateSkill must return the skill body"); // Activation MUST list available resources test:assertTrue(activateResult.includes("skill_resources"), - "activate_skill must include skill_resources section"); + "activateSkill must include skill_resources section"); test:assertTrue(activateResult.includes("references/REFERENCE.md")); test:assertTrue(activateResult.includes("assets/template.json")); // Step 4: Read a resource — file content is now revealed - string refContent = check toolkit.read_skill_resource("security-review", "references/REFERENCE.md"); + string refContent = check toolkit.readSkillResource("security-review", "references/REFERENCE.md"); test:assertTrue(refContent.includes("OWASP Top 10 Quick Reference"), - "read_skill_resource must return actual file content"); + "readSkillResource must return actual file content"); test:assertTrue(refContent.includes("reference file for security-review")); - string assetContent = check toolkit.read_skill_resource("security-review", "assets/template.json"); + string assetContent = check toolkit.readSkillResource("security-review", "assets/template.json"); test:assertTrue(assetContent.includes("security-review"), - "read_skill_resource must return asset file content"); + "readSkillResource must return asset file content"); // Step 5: Verify non-resource skill has no resources disclosed - string alphaResult = check toolkit.activate_skill("pr-summary"); + string alphaResult = check toolkit.activateSkill("pr-summary"); test:assertTrue(alphaResult.includes("Summarize each merged pull request")); test:assertFalse(alphaResult.includes("skill_resources"), "pr-summary has no resources, so no skill_resources section"); @@ -484,9 +484,9 @@ function testProgressiveDisclosureViaExtractSkillCatalog() returns error? { "Catalog must not contain security-review body"); // Activating reveals the body - string body = check toolkit.activate_skill("test-gen"); + string body = check toolkit.activateSkill("test-gen"); test:assertTrue(body.includes("body of the test skill"), - "activate_skill must reveal the full body"); + "activateSkill must reveal the full body"); } // ============================================ @@ -572,7 +572,7 @@ function testSkillsToolKitGetTools() returns error? { SkillsToolKit toolkit = new (skills); var tools = toolkit.getTools(); - // Should return 2 tools: activate_skill and read_skill_resource + // Should return 2 tools: activateSkill and readSkillResource test:assertEquals(tools.length(), 2); } From 8c91e45d225826cf7e9f38a1d27bbff72793a710 Mon Sep 17 00:00:00 2001 From: MaryamZi Date: Mon, 16 Mar 2026 14:59:33 +0530 Subject: [PATCH 7/9] Fix path check --- ballerina-interpreter/skills.bal | 2 +- ballerina-interpreter/tests/skills_test.bal | 38 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/ballerina-interpreter/skills.bal b/ballerina-interpreter/skills.bal index f555ae3..e535013 100644 --- a/ballerina-interpreter/skills.bal +++ b/ballerina-interpreter/skills.bal @@ -52,7 +52,7 @@ function discoverSkills(SkillSource[] sources, string afmFileDir) returns map localSkills = check discoverLocalSkills(resolvedPath); diff --git a/ballerina-interpreter/tests/skills_test.bal b/ballerina-interpreter/tests/skills_test.bal index d9622d2..c8e070e 100644 --- a/ballerina-interpreter/tests/skills_test.bal +++ b/ballerina-interpreter/tests/skills_test.bal @@ -324,6 +324,44 @@ function testDiscoverLocalSkillsInvalidSkill() returns error? { test:assertTrue(result is error); } +@test:Config +function testDiscoverSkillsAbsolutePathRejected() returns error? { + string testDir = check file:getAbsolutePath("tests/skills"); + SkillSource[] sources = [{path: "/absolute/path/to/skills"}]; + map|error result = discoverSkills(sources, testDir); + if result is map { + test:assertFail("Expected error for absolute path"); + } + test:assertTrue(result.message().includes("must be relative")); +} + +@test:Config +function testDiscoverSkillsPathOutsideAfmDirRejected() returns error? { + string testDir = check file:getAbsolutePath("tests/skills"); + SkillSource[] sources = [{path: "../../outside"}]; + map|error result = discoverSkills(sources, testDir); + if result is map { + test:assertFail("Expected error for path outside AFM directory"); + } + test:assertTrue(result.message().includes("resolves outside")); +} + +@test:Config +function testDiscoverSkillsSiblingPrefixPathRejected() returns error? { + // A sibling directory sharing a name prefix must not pass validation. + // E.g. if afm_dir is '/a/skills', a path resolving to '/a/skills-evil' + // should be rejected even though it starts with the same string prefix. + string testDir = check file:getAbsolutePath("tests/skills/single_skill"); + // "../single_skill_extra" would resolve to tests/skills/single_skill_extra + // which shares the prefix "tests/skills/single_skill" but is outside the dir + SkillSource[] sources = [{path: "../single_skill_extra"}]; + map|error result = discoverSkills(sources, testDir); + if result is map { + test:assertFail("Expected error for sibling prefix path"); + } + test:assertTrue(result.message().includes("resolves outside")); +} + @test:Config function testDiscoverSkillsMultipleSources() returns error? { string testDir = check file:getAbsolutePath("tests/skills"); From 608ce9ce4cda747cc50454930e6d7ab057d02fb8 Mon Sep 17 00:00:00 2001 From: MaryamZi Date: Mon, 16 Mar 2026 15:21:09 +0530 Subject: [PATCH 8/9] Fix template --- templates/agent_template.afm.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/agent_template.afm.md b/templates/agent_template.afm.md index b31b8ce..78e78c3 100644 --- a/templates/agent_template.afm.md +++ b/templates/agent_template.afm.md @@ -159,7 +159,7 @@ tools: # SKILLS - OPTIONAL (Agent Skills format: https://agentskills.io) # ============================================================================ skills: - # Local skills directory or (a directory that may contain multiple skill subdirectories) + # Local skills directory (or a directory that may contain multiple skill subdirectories) - type: "local" path: "./skills" --- From f97c1a306a3edc952dff6bd7a4f162c5c27136ea Mon Sep 17 00:00:00 2001 From: MaryamZi Date: Thu, 19 Mar 2026 10:21:49 +0530 Subject: [PATCH 9/9] Allow absolute paths for skill dirs --- ballerina-interpreter/skills.bal | 12 ++---- ballerina-interpreter/tests/skills_test.bal | 41 +++------------------ 2 files changed, 9 insertions(+), 44 deletions(-) diff --git a/ballerina-interpreter/skills.bal b/ballerina-interpreter/skills.bal index e535013..2c84ffc 100644 --- a/ballerina-interpreter/skills.bal +++ b/ballerina-interpreter/skills.bal @@ -44,17 +44,11 @@ type SkillFrontmatter record { function discoverSkills(SkillSource[] sources, string afmFileDir) returns map|error { map skills = {}; - string normalizedAfmDir = check file:normalizePath(check file:getAbsolutePath(afmFileDir), file:CLEAN); foreach SkillSource 'source in sources { - if check file:isAbsolutePath('source.path) { - return error(string `Skill source path must be relative, but got: ${'source.path}`); - } - string resolvedPath = check file:joinPath(afmFileDir, 'source.path); - string normalizedPath = check file:normalizePath(check file:getAbsolutePath(resolvedPath), file:CLEAN); - if normalizedPath != normalizedAfmDir && !normalizedPath.startsWith(normalizedAfmDir + file:pathSeparator) { - return error(string `Skill source path '${'source.path}' resolves outside the AFM file directory`); - } + string resolvedPath = check file:isAbsolutePath('source.path) + ? 'source.path + : check file:joinPath(afmFileDir, 'source.path); map localSkills = check discoverLocalSkills(resolvedPath); foreach [string, SkillInfo] [name, info] in localSkills.entries() { if skills.hasKey(name) { diff --git a/ballerina-interpreter/tests/skills_test.bal b/ballerina-interpreter/tests/skills_test.bal index c8e070e..33268ec 100644 --- a/ballerina-interpreter/tests/skills_test.bal +++ b/ballerina-interpreter/tests/skills_test.bal @@ -325,41 +325,12 @@ function testDiscoverLocalSkillsInvalidSkill() returns error? { } @test:Config -function testDiscoverSkillsAbsolutePathRejected() returns error? { - string testDir = check file:getAbsolutePath("tests/skills"); - SkillSource[] sources = [{path: "/absolute/path/to/skills"}]; - map|error result = discoverSkills(sources, testDir); - if result is map { - test:assertFail("Expected error for absolute path"); - } - test:assertTrue(result.message().includes("must be relative")); -} - -@test:Config -function testDiscoverSkillsPathOutsideAfmDirRejected() returns error? { - string testDir = check file:getAbsolutePath("tests/skills"); - SkillSource[] sources = [{path: "../../outside"}]; - map|error result = discoverSkills(sources, testDir); - if result is map { - test:assertFail("Expected error for path outside AFM directory"); - } - test:assertTrue(result.message().includes("resolves outside")); -} - -@test:Config -function testDiscoverSkillsSiblingPrefixPathRejected() returns error? { - // A sibling directory sharing a name prefix must not pass validation. - // E.g. if afm_dir is '/a/skills', a path resolving to '/a/skills-evil' - // should be rejected even though it starts with the same string prefix. - string testDir = check file:getAbsolutePath("tests/skills/single_skill"); - // "../single_skill_extra" would resolve to tests/skills/single_skill_extra - // which shares the prefix "tests/skills/single_skill" but is outside the dir - SkillSource[] sources = [{path: "../single_skill_extra"}]; - map|error result = discoverSkills(sources, testDir); - if result is map { - test:assertFail("Expected error for sibling prefix path"); - } - test:assertTrue(result.message().includes("resolves outside")); +function testDiscoverSkillsAbsolutePathAccepted() returns error? { + string absPath = check file:getAbsolutePath("tests/skills/single_skill"); + SkillSource[] sources = [{path: absPath}]; + map skills = check discoverSkills(sources, "some/other/dir"); + test:assertEquals(skills.length(), 1); + test:assertTrue(skills.hasKey("test-gen")); } @test:Config