Support Agent Skills with the Ballerina Interpreter#13
Merged
RadCod3 merged 9 commits intowso2:agent-skillsfrom Mar 19, 2026
Merged
Support Agent Skills with the Ballerina Interpreter#13RadCod3 merged 9 commits intowso2:agent-skillsfrom
RadCod3 merged 9 commits intowso2:agent-skillsfrom
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request introduces a local skills system for the Ballerina interpreter. It adds skill discovery from local directories, YAML frontmatter parsing for SKILL.md files, a SkillsToolKit class for skill activation and resource access, comprehensive test coverage, new type definitions, and full integration into the agent creation flow with directory-based context passing. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main Entry
participant AFMParser as AFM Parser
participant SkillDisco as Skill Discovery
participant SkillParse as SKILL.md Parser
participant AgentBuilder as Agent Builder
participant SkillsKit as SkillsToolKit
participant AIAgent as AI Agent
Main->>AFMParser: parseAfm(afmContent, afmFileDir)
AFMParser->>AFMParser: extractFrontmatter(content)
AFMParser->>Main: AFMRecord with metadata.skills
Main->>AgentBuilder: createAgent(afmRecord, afmFileDir)
AgentBuilder->>SkillDisco: extractSkillCatalog(metadata, afmFileDir)
SkillDisco->>SkillDisco: discoverSkills(sources, afmFileDir)
SkillDisco->>SkillParse: parseSkillMd(skillMdPath, basePath)
SkillParse->>SkillParse: extractFrontmatter(skillContent)
SkillParse->>SkillParse: listLocalResources(basePath)
SkillParse->>SkillDisco: SkillInfo record
SkillDisco->>SkillDisco: buildSkillCatalog(skills)
SkillDisco->>AgentBuilder: [catalogText, SkillsToolKit]
AgentBuilder->>AgentBuilder: Augment instructions & toolKits with catalog
AgentBuilder->>AIAgent: create with effectiveInstructions & toolKits
AIAgent->>SkillsKit: activate_skill(name)
SkillsKit->>AIAgent: skill content + resources
AIAgent->>SkillsKit: read_skill_resource(skillName, resourcePath)
SkillsKit->>AIAgent: resource content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable poems in the walkthrough.Disable the |
MaryamZi
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (5)
ballerina-interpreter/tests/skills/invalid_skill/SKILL.md (1)
1-4: Consider verifying complete test coverage for invalid field combinations.The test suite now covers two invalid scenarios:
- Empty
namewith validdescription(inpartial_invalid/invalid_skill/)- Both fields empty (this file)
Consider verifying that test coverage also includes the inverse case: valid
namewith emptydescription, if that's also an invalid scenario per the validation rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-interpreter/tests/skills/invalid_skill/SKILL.md` around lines 1 - 4, The test set currently has cases for empty name with valid description and both fields empty (file SKILL.md under tests/skills/invalid_skill), so add or verify a test for the inverse invalid case (valid name with empty description) if validation rules treat an empty description as invalid; create a new test file or update the invalid suite (e.g., under partial_invalid/invalid_skill/) that supplies a non-empty name and an empty description and assert the same validation failure path (same error type/message) as the other invalid cases to ensure full coverage.ballerina-interpreter/tests/skills_test.bal (1)
575-581: Redundant assertion.Line 576 already asserts
tools.length() == 2, making the assertion on line 580 redundant.🔧 Suggested cleanup
// 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"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-interpreter/tests/skills_test.bal` around lines 575 - 581, The test contains a redundant assertion: after calling test:assertEquals(tools.length(), 2) the later test:assertTrue(tools.length() == 2, "Should have exactly 2 tools") duplicates the same check; remove the redundant test:assertTrue call (or replace it with a different verification such as checking presence of function refs) so only one assertion verifies tools.length(), referencing the tools variable and the existing test:assertEquals assertion in the test function.ballerina-interpreter/main.bal (1)
46-46: Consider edge case for root-level AFM files.The computation correctly derives the parent directory for relative path resolution. However, if the AFM file is located at the filesystem root (e.g.,
/agent.afm.md),file:parentPathmay return an empty string or/, which could affect skill path resolution.This is a minor edge case unlikely to occur in practice, but worth noting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-interpreter/main.bal` at line 46, The parent-path result for afmFileDir (computed via file:parentPath(file:getAbsolutePath(fileToUse))) can be empty for root-level AFM files; add a guard after computing afmFileDir to detect an empty string (or a non-useful value such as "") and normalize it to "/" (or another appropriate root indicator) so subsequent skill path resolution uses a valid directory; update the code that sets string afmFileDir = check file:parentPath(check file:getAbsolutePath(fileToUse)); to compute afmFileDir, then if afmFileDir == "" set afmFileDir = "/" (and ensure any downstream joins respect this normalized value).ballerina-interpreter/skills.bal (2)
123-139: LGTM with minor note.The error handling silently swallows failures when directories don't exist, which is intentional. However, this also hides permission errors on existing directories. The debug-level logging is sufficient for troubleshooting, but consider logging at
warnlevel if the directory exists but is unreadable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-interpreter/skills.bal` around lines 123 - 139, In listLocalResources, the on-fail handler currently always logs a debug message, which hides permission errors on existing directories; change the handler to first check whether dirPath exists (use file:exists or equivalent for the computed dirPath from file:joinPath) and if it exists but file:readDir failed, log a warn (including the error e) instead of debug, otherwise keep the debug-level log for non-existent dirs; update the on-fail block around file:readDir/file:basename to perform that exists check and include the error details in the log message.
45-61: Consider validating that skill paths are relative.If
source.pathis an absolute path,file:joinPathmay ignoreafmFileDirentirely on some platforms, allowing skills to be loaded from arbitrary filesystem locations. If this is unintentional, add validation:Suggested fix
foreach SkillSource 'source in sources { + if check file:isAbsolutePath('source.path) { + return error(string `Skill path must be relative: ${'source.path}`); + } string resolvedPath = check file:joinPath(afmFileDir, 'source.path);If loading from absolute paths is intentional, this can be ignored.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-interpreter/skills.bal` around lines 45 - 61, The discoverSkills function currently joins afmFileDir with source.path via file:joinPath which can be bypassed if source.path is absolute; update discoverSkills to validate that each SkillSource's 'source.path is a relative path before joining (use a platform-aware check such as file:isAbsolutePath or equivalent) and return an error or skip the entry if it's absolute; ensure you perform this check right before calling file:joinPath and reference the same symbols (discoverSkills, SkillSource, 'source.path, file:joinPath, discoverLocalSkills) so absolute paths are rejected or handled consistently rather than allowing arbitrary filesystem access.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ballerina-interpreter/main.bal`:
- Line 46: The parent-path result for afmFileDir (computed via
file:parentPath(file:getAbsolutePath(fileToUse))) can be empty for root-level
AFM files; add a guard after computing afmFileDir to detect an empty string (or
a non-useful value such as "") and normalize it to "/" (or another appropriate
root indicator) so subsequent skill path resolution uses a valid directory;
update the code that sets string afmFileDir = check file:parentPath(check
file:getAbsolutePath(fileToUse)); to compute afmFileDir, then if afmFileDir ==
"" set afmFileDir = "/" (and ensure any downstream joins respect this normalized
value).
In `@ballerina-interpreter/skills.bal`:
- Around line 123-139: In listLocalResources, the on-fail handler currently
always logs a debug message, which hides permission errors on existing
directories; change the handler to first check whether dirPath exists (use
file:exists or equivalent for the computed dirPath from file:joinPath) and if it
exists but file:readDir failed, log a warn (including the error e) instead of
debug, otherwise keep the debug-level log for non-existent dirs; update the
on-fail block around file:readDir/file:basename to perform that exists check and
include the error details in the log message.
- Around line 45-61: The discoverSkills function currently joins afmFileDir with
source.path via file:joinPath which can be bypassed if source.path is absolute;
update discoverSkills to validate that each SkillSource's 'source.path is a
relative path before joining (use a platform-aware check such as
file:isAbsolutePath or equivalent) and return an error or skip the entry if it's
absolute; ensure you perform this check right before calling file:joinPath and
reference the same symbols (discoverSkills, SkillSource, 'source.path,
file:joinPath, discoverLocalSkills) so absolute paths are rejected or handled
consistently rather than allowing arbitrary filesystem access.
In `@ballerina-interpreter/tests/skills_test.bal`:
- Around line 575-581: The test contains a redundant assertion: after calling
test:assertEquals(tools.length(), 2) the later test:assertTrue(tools.length() ==
2, "Should have exactly 2 tools") duplicates the same check; remove the
redundant test:assertTrue call (or replace it with a different verification such
as checking presence of function refs) so only one assertion verifies
tools.length(), referencing the tools variable and the existing
test:assertEquals assertion in the test function.
In `@ballerina-interpreter/tests/skills/invalid_skill/SKILL.md`:
- Around line 1-4: The test set currently has cases for empty name with valid
description and both fields empty (file SKILL.md under
tests/skills/invalid_skill), so add or verify a test for the inverse invalid
case (valid name with empty description) if validation rules treat an empty
description as invalid; create a new test file or update the invalid suite
(e.g., under partial_invalid/invalid_skill/) that supplies a non-empty name and
an empty description and assert the same validation failure path (same error
type/message) as the other invalid cases to ensure full coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9ebd02d5-d64e-4256-b21f-5b27fbcbb517
📒 Files selected for processing (24)
ballerina-interpreter/Dependencies.tomlballerina-interpreter/README.mdballerina-interpreter/agent.balballerina-interpreter/main.balballerina-interpreter/parser.balballerina-interpreter/skills.balballerina-interpreter/tests/agent_test.balballerina-interpreter/tests/skills/duplicate_names/error_translator/SKILL.mdballerina-interpreter/tests/skills/duplicate_names/error_translator_dup/SKILL.mdballerina-interpreter/tests/skills/empty_dir/.gitkeepballerina-interpreter/tests/skills/invalid_skill/SKILL.mdballerina-interpreter/tests/skills/multi_skills/pr_summary/SKILL.mdballerina-interpreter/tests/skills/multi_skills/security_review/SKILL.mdballerina-interpreter/tests/skills/multi_skills/security_review/assets/template.jsonballerina-interpreter/tests/skills/multi_skills/security_review/references/REFERENCE.mdballerina-interpreter/tests/skills/no_frontmatter/SKILL.mdballerina-interpreter/tests/skills/partial_invalid/invalid_skill/SKILL.mdballerina-interpreter/tests/skills/partial_invalid/valid_skill/SKILL.mdballerina-interpreter/tests/skills/single_skill/SKILL.mdballerina-interpreter/tests/skills/source1/release_notes/SKILL.mdballerina-interpreter/tests/skills/source2/release_notes/SKILL.mdballerina-interpreter/tests/skills_test.balballerina-interpreter/types.baltemplates/agent_template.afm.md
There was a problem hiding this comment.
Pull request overview
Adds support for Agent Skills (AgentSkills format) to the Ballerina interpreter, including local skill discovery, progressive activation, and resource access.
Changes:
- Introduces new skill-related types and AFM metadata (
skills:) plus a local skills discovery/activation toolkit. - Refactors frontmatter parsing into a shared
extractFrontmatter()utility and extends validation forhttp:variables into skill paths. - Adds extensive fixtures + tests for discovery, duplicate handling, progressive disclosure, and resource reading; updates docs/template and bumps dependencies.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/agent_template.afm.md | Adds an optional skills: section example for AFM authors. |
| ballerina-interpreter/types.bal | Defines skill source + skill info records; adds skills? to agent metadata. |
| ballerina-interpreter/skills.bal | Implements local skill discovery, catalog building, and skill/resource AgentTools. |
| ballerina-interpreter/parser.bal | Replaces inline AFM frontmatter parsing with extractFrontmatter() and validates skills.path for http: vars. |
| ballerina-interpreter/agent.bal | Wires discovered skills into agent instructions and toolkits; minor model/provider cleanup. |
| ballerina-interpreter/main.bal | Passes AFM directory into agent creation so relative skill paths resolve correctly. |
| ballerina-interpreter/tests/agent_test.bal | Updates integration tests to pass afmFileDir into runAgentFromAFM. |
| ballerina-interpreter/tests/skills_test.bal | Adds unit + fixture-based tests covering skill parsing, discovery, duplicates, and progressive disclosure. |
| ballerina-interpreter/tests/skills/** | Adds skill fixtures (SKILL.md) + resources (references/assets) used by tests. |
| ballerina-interpreter/README.md | Documents running the container with skills mounted and path expectations. |
| ballerina-interpreter/Dependencies.toml | Bumps Ballerina distribution and selected packages; adds ballerina/file dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Adds support for “Agent Skills” (local skill discovery + activation tooling) to the Ballerina interpreter, and updates the CLI/runtime flow to resolve skills relative to the AFM file location.
Changes:
- Introduces skill source / skill info types and a skills discovery + toolkit implementation.
- Updates agent creation/runtime to load skills and append a skills catalog to the system instructions.
- Adds extensive tests and fixtures covering skill parsing, discovery, progressive disclosure, and security/path validation.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/agent_template.afm.md | Adds an optional skills: section to the AFM template. |
| ballerina-interpreter/web_chat_ui.bal | Allows web UI metadata to be optional and safely dereferenced. |
| ballerina-interpreter/types.bal | Introduces skill-related types and makes AFM metadata optional. |
| ballerina-interpreter/skills.bal | Implements local skill discovery, catalog generation, and agent tools. |
| ballerina-interpreter/parser.bal | Refactors frontmatter parsing and adds extractFrontMatter() helper. |
| ballerina-interpreter/main.bal | Passes AFM file directory into agent creation to resolve relative skill paths. |
| ballerina-interpreter/agent.bal | Appends skill catalog to instructions and registers the Skills toolkit. |
| ballerina-interpreter/tests/skills_test.bal | Adds comprehensive unit/E2E tests for skills behavior and validation. |
| ballerina-interpreter/tests/main_test.bal | Updates AFM parsing tests to match optional metadata behavior. |
| ballerina-interpreter/tests/agent_test.bal | Updates agent start helper to pass a base directory for skills resolution. |
| ballerina-interpreter/tests/skills/** | Adds SKILL.md fixtures, resources, and duplicate/invalid skill scenarios. |
| ballerina-interpreter/README.md | Documents running with skills via Docker and relative-path expectations. |
| ballerina-interpreter/Dependencies.toml | Bumps distribution/deps and adds file module dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
RadCod3
reviewed
Mar 18, 2026
RadCod3
approved these changes
Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
$title.
Summary by CodeRabbit
New Features
Documentation
Chores