mirror of
https://github.com/encounter/adk-python.git
synced 2026-07-09 18:19:28 -07:00
feat: Agent Skills spec compliance — validation, aliases, scripts, and auto-injection
Close gaps between ADK's Agent Skills implementation and the public Agent Skills spec (agentskills.io/specification): - Frontmatter: add field validators for name (kebab-case, max 64), description (non-empty, max 1024), compatibility (max 500); add allowed-tools alias; add extra='allow'; add populate_by_name - utils: extract _parse_skill_md helper; use model_validate() for alias support; enforce name-dir matching; add validate_skill_dir() and read_skill_properties() - prompt: accept Union[Frontmatter, Skill]; - skill_toolset: add scripts/ resource loading; auto-inject system instruction (with inject_instruction opt-out); duplicate name check; _list_skills() returns Skill objects - sample agent: remove manual instruction (auto-injected now) Co-authored-by: Haiyuan Cao <haiyuan@google.com> PiperOrigin-RevId: 873177060
This commit is contained in:
committed by
Copybara-Service
parent
4260ef0c7c
commit
223d9a7ff5
@@ -0,0 +1,182 @@
|
||||
# Copyright 2026 Google LLC
|
||||
#
|
||||
# Licensed 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.
|
||||
|
||||
"""Unit tests for skill utilities."""
|
||||
|
||||
from google.adk.skills import load_skill_from_dir as _load_skill_from_dir
|
||||
from google.adk.skills._utils import _read_skill_properties
|
||||
from google.adk.skills._utils import _validate_skill_dir
|
||||
import pytest
|
||||
|
||||
|
||||
def test__load_skill_from_dir(tmp_path):
|
||||
"""Tests loading a skill from a directory."""
|
||||
skill_dir = tmp_path / "test-skill"
|
||||
skill_dir.mkdir()
|
||||
|
||||
skill_md_content = """---
|
||||
name: test-skill
|
||||
description: Test description
|
||||
---
|
||||
Test instructions
|
||||
"""
|
||||
(skill_dir / "SKILL.md").write_text(skill_md_content)
|
||||
|
||||
# Create references
|
||||
ref_dir = skill_dir / "references"
|
||||
ref_dir.mkdir()
|
||||
(ref_dir / "ref1.md").write_text("ref1 content")
|
||||
|
||||
# Create assets
|
||||
assets_dir = skill_dir / "assets"
|
||||
assets_dir.mkdir()
|
||||
(assets_dir / "asset1.txt").write_text("asset1 content")
|
||||
|
||||
# Create scripts
|
||||
scripts_dir = skill_dir / "scripts"
|
||||
scripts_dir.mkdir()
|
||||
(scripts_dir / "script1.sh").write_text("echo hello")
|
||||
|
||||
skill = _load_skill_from_dir(skill_dir)
|
||||
|
||||
assert skill.name == "test-skill"
|
||||
assert skill.description == "Test description"
|
||||
assert skill.instructions == "Test instructions"
|
||||
assert skill.resources.get_reference("ref1.md") == "ref1 content"
|
||||
assert skill.resources.get_asset("asset1.txt") == "asset1 content"
|
||||
assert skill.resources.get_script("script1.sh").src == "echo hello"
|
||||
|
||||
|
||||
def test_allowed_tools_yaml_key(tmp_path):
|
||||
"""Tests that allowed-tools YAML key loads correctly."""
|
||||
skill_dir = tmp_path / "my-skill"
|
||||
skill_dir.mkdir()
|
||||
|
||||
skill_md = """---
|
||||
name: my-skill
|
||||
description: A skill
|
||||
allowed-tools: "some-tool-*"
|
||||
---
|
||||
Instructions here
|
||||
"""
|
||||
(skill_dir / "SKILL.md").write_text(skill_md)
|
||||
|
||||
skill = _load_skill_from_dir(skill_dir)
|
||||
assert skill.frontmatter.allowed_tools == "some-tool-*"
|
||||
|
||||
|
||||
def test_name_directory_mismatch(tmp_path):
|
||||
"""Tests that name-directory mismatch raises ValueError."""
|
||||
skill_dir = tmp_path / "wrong-dir"
|
||||
skill_dir.mkdir()
|
||||
|
||||
skill_md = """---
|
||||
name: my-skill
|
||||
description: A skill
|
||||
---
|
||||
Body
|
||||
"""
|
||||
(skill_dir / "SKILL.md").write_text(skill_md)
|
||||
|
||||
with pytest.raises(ValueError, match="does not match directory"):
|
||||
_load_skill_from_dir(skill_dir)
|
||||
|
||||
|
||||
def test_validate_skill_dir_valid(tmp_path):
|
||||
"""Tests validate_skill_dir with a valid skill."""
|
||||
skill_dir = tmp_path / "my-skill"
|
||||
skill_dir.mkdir()
|
||||
|
||||
skill_md = """---
|
||||
name: my-skill
|
||||
description: A skill
|
||||
---
|
||||
Body
|
||||
"""
|
||||
(skill_dir / "SKILL.md").write_text(skill_md)
|
||||
|
||||
problems = _validate_skill_dir(skill_dir)
|
||||
assert problems == []
|
||||
|
||||
|
||||
def test_validate_skill_dir_missing_dir(tmp_path):
|
||||
"""Tests validate_skill_dir with missing directory."""
|
||||
problems = _validate_skill_dir(tmp_path / "nonexistent")
|
||||
assert len(problems) == 1
|
||||
assert "does not exist" in problems[0]
|
||||
|
||||
|
||||
def test_validate_skill_dir_missing_skill_md(tmp_path):
|
||||
"""Tests validate_skill_dir with missing SKILL.md."""
|
||||
skill_dir = tmp_path / "my-skill"
|
||||
skill_dir.mkdir()
|
||||
|
||||
problems = _validate_skill_dir(skill_dir)
|
||||
assert len(problems) == 1
|
||||
assert "SKILL.md not found" in problems[0]
|
||||
|
||||
|
||||
def test_validate_skill_dir_name_mismatch(tmp_path):
|
||||
"""Tests validate_skill_dir catches name-directory mismatch."""
|
||||
skill_dir = tmp_path / "wrong-dir"
|
||||
skill_dir.mkdir()
|
||||
|
||||
skill_md = """---
|
||||
name: my-skill
|
||||
description: A skill
|
||||
---
|
||||
Body
|
||||
"""
|
||||
(skill_dir / "SKILL.md").write_text(skill_md)
|
||||
|
||||
problems = _validate_skill_dir(skill_dir)
|
||||
assert any("does not match" in p for p in problems)
|
||||
|
||||
|
||||
def test_validate_skill_dir_unknown_fields(tmp_path):
|
||||
"""Tests validate_skill_dir detects unknown frontmatter fields."""
|
||||
skill_dir = tmp_path / "my-skill"
|
||||
skill_dir.mkdir()
|
||||
|
||||
skill_md = """---
|
||||
name: my-skill
|
||||
description: A skill
|
||||
unknown-field: something
|
||||
---
|
||||
Body
|
||||
"""
|
||||
(skill_dir / "SKILL.md").write_text(skill_md)
|
||||
|
||||
problems = _validate_skill_dir(skill_dir)
|
||||
assert any("Unknown frontmatter" in p for p in problems)
|
||||
|
||||
|
||||
def test__read_skill_properties(tmp_path):
|
||||
"""Tests read_skill_properties basic usage."""
|
||||
skill_dir = tmp_path / "my-skill"
|
||||
skill_dir.mkdir()
|
||||
|
||||
skill_md = """---
|
||||
name: my-skill
|
||||
description: A cool skill
|
||||
license: MIT
|
||||
---
|
||||
Body content
|
||||
"""
|
||||
(skill_dir / "SKILL.md").write_text(skill_md)
|
||||
|
||||
fm = _read_skill_properties(skill_dir)
|
||||
assert fm.name == "my-skill"
|
||||
assert fm.description == "A cool skill"
|
||||
assert fm.license == "MIT"
|
||||
@@ -15,6 +15,7 @@
|
||||
"""Unit tests for skill models."""
|
||||
|
||||
from google.adk.skills import models
|
||||
from pydantic import ValidationError
|
||||
import pytest
|
||||
|
||||
|
||||
@@ -68,3 +69,107 @@ def test_script_to_string():
|
||||
"""Tests Script model."""
|
||||
script = models.Script(src="print('hello')")
|
||||
assert str(script) == "print('hello')"
|
||||
|
||||
|
||||
# --- Name validation tests ---
|
||||
|
||||
|
||||
def test_name_too_long():
|
||||
with pytest.raises(ValidationError, match="at most 64 characters"):
|
||||
models.Frontmatter(name="a" * 65, description="desc")
|
||||
|
||||
|
||||
def test_name_uppercase_rejected():
|
||||
with pytest.raises(ValidationError, match="lowercase kebab-case"):
|
||||
models.Frontmatter(name="My-Skill", description="desc")
|
||||
|
||||
|
||||
def test_name_leading_hyphen():
|
||||
with pytest.raises(ValidationError, match="lowercase kebab-case"):
|
||||
models.Frontmatter(name="-my-skill", description="desc")
|
||||
|
||||
|
||||
def test_name_trailing_hyphen():
|
||||
with pytest.raises(ValidationError, match="lowercase kebab-case"):
|
||||
models.Frontmatter(name="my-skill-", description="desc")
|
||||
|
||||
|
||||
def test_name_consecutive_hyphens():
|
||||
with pytest.raises(ValidationError, match="lowercase kebab-case"):
|
||||
models.Frontmatter(name="my--skill", description="desc")
|
||||
|
||||
|
||||
def test_name_invalid_chars_underscore():
|
||||
with pytest.raises(ValidationError, match="lowercase kebab-case"):
|
||||
models.Frontmatter(name="my_skill", description="desc")
|
||||
|
||||
|
||||
def test_name_invalid_chars_ampersand():
|
||||
with pytest.raises(ValidationError, match="lowercase kebab-case"):
|
||||
models.Frontmatter(name="skill&name", description="desc")
|
||||
|
||||
|
||||
def test_name_valid_passes():
|
||||
fm = models.Frontmatter(name="my-skill-2", description="desc")
|
||||
assert fm.name == "my-skill-2"
|
||||
|
||||
|
||||
def test_name_single_word():
|
||||
fm = models.Frontmatter(name="skill", description="desc")
|
||||
assert fm.name == "skill"
|
||||
|
||||
|
||||
# --- Description validation tests ---
|
||||
|
||||
|
||||
def test_description_empty():
|
||||
with pytest.raises(ValidationError, match="must not be empty"):
|
||||
models.Frontmatter(name="my-skill", description="")
|
||||
|
||||
|
||||
def test_description_too_long():
|
||||
with pytest.raises(ValidationError, match="at most 1024 characters"):
|
||||
models.Frontmatter(name="my-skill", description="x" * 1025)
|
||||
|
||||
|
||||
# --- Compatibility validation tests ---
|
||||
|
||||
|
||||
def test_compatibility_too_long():
|
||||
with pytest.raises(ValidationError, match="at most 500 characters"):
|
||||
models.Frontmatter(
|
||||
name="my-skill", description="desc", compatibility="c" * 501
|
||||
)
|
||||
|
||||
|
||||
# --- Extra field rejected ---
|
||||
|
||||
|
||||
def test_extra_field_allowed():
|
||||
fm = models.Frontmatter.model_validate({
|
||||
"name": "my-skill",
|
||||
"description": "desc",
|
||||
"unknown_field": "value",
|
||||
})
|
||||
assert fm.name == "my-skill"
|
||||
|
||||
|
||||
# --- allowed-tools alias ---
|
||||
|
||||
|
||||
def test_allowed_tools_alias_via_model_validate():
|
||||
fm = models.Frontmatter.model_validate({
|
||||
"name": "my-skill",
|
||||
"description": "desc",
|
||||
"allowed-tools": "tool-pattern",
|
||||
})
|
||||
assert fm.allowed_tools == "tool-pattern"
|
||||
|
||||
|
||||
def test_allowed_tools_serialization_alias():
|
||||
fm = models.Frontmatter(
|
||||
name="my-skill", description="desc", allowed_tools="tool-pattern"
|
||||
)
|
||||
dumped = fm.model_dump(by_alias=True)
|
||||
assert "allowed-tools" in dumped
|
||||
assert dumped["allowed-tools"] == "tool-pattern"
|
||||
|
||||
@@ -42,8 +42,8 @@ class TestPrompt:
|
||||
|
||||
def test_format_skills_as_xml_escaping(self):
|
||||
skills = [
|
||||
models.Frontmatter(name="skill&name", description="desc<ription>"),
|
||||
models.Frontmatter(name="my-skill", description="desc<ription>"),
|
||||
]
|
||||
xml = prompt.format_skills_as_xml(skills)
|
||||
assert "skill&name" in xml
|
||||
assert "my-skill" in xml
|
||||
assert "desc<ription>" in xml
|
||||
|
||||
@@ -1,56 +0,0 @@
|
||||
# Copyright 2026 Google LLC
|
||||
#
|
||||
# Licensed 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.
|
||||
|
||||
"""Unit tests for skill utilities."""
|
||||
|
||||
from google.adk.skills import load_skill_from_dir
|
||||
import pytest
|
||||
|
||||
|
||||
def test_load_skill_from_dir(tmp_path):
|
||||
"""Tests loading a skill from a directory."""
|
||||
skill_dir = tmp_path / "test-skill"
|
||||
skill_dir.mkdir()
|
||||
|
||||
skill_md_content = """---
|
||||
name: test-skill
|
||||
description: Test description
|
||||
---
|
||||
Test instructions
|
||||
"""
|
||||
(skill_dir / "SKILL.md").write_text(skill_md_content)
|
||||
|
||||
# Create references
|
||||
ref_dir = skill_dir / "references"
|
||||
ref_dir.mkdir()
|
||||
(ref_dir / "ref1.md").write_text("ref1 content")
|
||||
|
||||
# Create assets
|
||||
assets_dir = skill_dir / "assets"
|
||||
assets_dir.mkdir()
|
||||
(assets_dir / "asset1.txt").write_text("asset1 content")
|
||||
|
||||
# Create scripts
|
||||
scripts_dir = skill_dir / "scripts"
|
||||
scripts_dir.mkdir()
|
||||
(scripts_dir / "script1.sh").write_text("echo hello")
|
||||
|
||||
skill = load_skill_from_dir(skill_dir)
|
||||
|
||||
assert skill.name == "test-skill"
|
||||
assert skill.description == "Test description"
|
||||
assert skill.instructions == "Test instructions"
|
||||
assert skill.resources.get_reference("ref1.md") == "ref1 content"
|
||||
assert skill.resources.get_asset("asset1.txt") == "asset1 content"
|
||||
assert skill.resources.get_script("script1.sh").src == "echo hello"
|
||||
@@ -39,6 +39,7 @@ def mock_skill1(mock_skill1_frontmatter):
|
||||
"""Fixture for skill1."""
|
||||
skill = mock.create_autospec(models.Skill, instance=True)
|
||||
skill.name = "skill1"
|
||||
skill.description = "Skill 1 description"
|
||||
skill.instructions = "instructions for skill1"
|
||||
skill.frontmatter = mock_skill1_frontmatter
|
||||
skill.resources = mock.MagicMock(
|
||||
@@ -55,8 +56,14 @@ def mock_skill1(mock_skill1_frontmatter):
|
||||
return "asset content 1"
|
||||
return None
|
||||
|
||||
def get_script(name):
|
||||
if name == "setup.sh":
|
||||
return models.Script(src="echo setup")
|
||||
return None
|
||||
|
||||
skill.resources.get_reference.side_effect = get_ref
|
||||
skill.resources.get_asset.side_effect = get_asset
|
||||
skill.resources.get_script.side_effect = get_script
|
||||
return skill
|
||||
|
||||
|
||||
@@ -78,6 +85,7 @@ def mock_skill2(mock_skill2_frontmatter):
|
||||
"""Fixture for skill2."""
|
||||
skill = mock.create_autospec(models.Skill, instance=True)
|
||||
skill.name = "skill2"
|
||||
skill.description = "Skill 2 description"
|
||||
skill.instructions = "instructions for skill2"
|
||||
skill.frontmatter = mock_skill2_frontmatter
|
||||
skill.resources = mock.MagicMock(
|
||||
@@ -114,10 +122,10 @@ def test_get_skill(mock_skill1, mock_skill2):
|
||||
|
||||
def test_list_skills(mock_skill1, mock_skill2):
|
||||
toolset = skill_toolset.SkillToolset([mock_skill1, mock_skill2])
|
||||
frontmatters = toolset._list_skills()
|
||||
assert len(frontmatters) == 2
|
||||
assert mock_skill1.frontmatter in frontmatters
|
||||
assert mock_skill2.frontmatter in frontmatters
|
||||
skills = toolset._list_skills()
|
||||
assert len(skills) == 2
|
||||
assert mock_skill1 in skills
|
||||
assert mock_skill2 in skills
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -203,6 +211,14 @@ async def test_load_skill_run_async(
|
||||
"content": "asset content 1",
|
||||
},
|
||||
),
|
||||
(
|
||||
{"skill_name": "skill1", "path": "scripts/setup.sh"},
|
||||
{
|
||||
"skill_name": "skill1",
|
||||
"path": "scripts/setup.sh",
|
||||
"content": "echo setup",
|
||||
},
|
||||
),
|
||||
(
|
||||
{"skill_name": "nonexistent", "path": "references/ref1.md"},
|
||||
{
|
||||
@@ -223,7 +239,10 @@ async def test_load_skill_run_async(
|
||||
(
|
||||
{"skill_name": "skill1", "path": "invalid/path.txt"},
|
||||
{
|
||||
"error": "Path must start with 'references/' or 'assets/'.",
|
||||
"error": (
|
||||
"Path must start with 'references/', 'assets/',"
|
||||
" or 'scripts/'."
|
||||
),
|
||||
"error_code": "INVALID_RESOURCE_PATH",
|
||||
},
|
||||
),
|
||||
@@ -266,7 +285,26 @@ async def test_process_llm_request(
|
||||
llm_req.append_instructions.assert_called_once()
|
||||
args, _ = llm_req.append_instructions.call_args
|
||||
instructions = args[0]
|
||||
assert len(instructions) == 1
|
||||
assert "<available_skills>" in instructions[0]
|
||||
assert "skill1" in instructions[0]
|
||||
assert "skill2" in instructions[0]
|
||||
assert len(instructions) == 2
|
||||
assert instructions[0] == skill_toolset.DEFAULT_SKILL_SYSTEM_INSTRUCTION
|
||||
assert "<available_skills>" in instructions[1]
|
||||
assert "skill1" in instructions[1]
|
||||
assert "skill2" in instructions[1]
|
||||
|
||||
|
||||
def test_duplicate_skill_name_raises(mock_skill1):
|
||||
skill_dup = mock.create_autospec(models.Skill, instance=True)
|
||||
skill_dup.name = "skill1"
|
||||
with pytest.raises(ValueError, match="Duplicate skill name"):
|
||||
skill_toolset.SkillToolset([mock_skill1, skill_dup])
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_scripts_resource_not_found(mock_skill1, tool_context_instance):
|
||||
toolset = skill_toolset.SkillToolset([mock_skill1])
|
||||
tool = skill_toolset.LoadSkillResourceTool(toolset)
|
||||
result = await tool.run_async(
|
||||
args={"skill_name": "skill1", "path": "scripts/nonexistent.sh"},
|
||||
tool_context=tool_context_instance,
|
||||
)
|
||||
assert result["error_code"] == "RESOURCE_NOT_FOUND"
|
||||
|
||||
Reference in New Issue
Block a user