diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index c7d59d789e..854f81a3c4 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -295,8 +295,8 @@ def decorator(f: Callable[..., Any]) -> Callable[..., Any]: ) # Validates skill names: lowercase letters, numbers, hyphens only; -# must not start or end with a hyphen. -VALID_NAME_RE = re.compile(r"^[a-z0-9]([a-z0-9\-]*[a-z0-9])?$") +# must not start or end with a hyphen, and must not contain consecutive hyphens. +VALID_NAME_RE = re.compile(r"^[a-z0-9]([a-z0-9]*-[a-z0-9])*[a-z0-9]*$") # Default system prompt template for advertising available skills to the model. # Use {skills} as the placeholder for the generated skills XML list. @@ -734,7 +734,8 @@ def _validate_skill_metadata( if len(name) > MAX_NAME_LENGTH or not VALID_NAME_RE.match(name): return ( f"Skill from '{source}' has an invalid name '{name}': Must be {MAX_NAME_LENGTH} characters or fewer, " - "using only lowercase letters, numbers, and hyphens, and must not start or end with a hyphen." + "using only lowercase letters, numbers, and hyphens, and must not start or end with a hyphen " + "or contain consecutive hyphens." ) if not description or not description.strip(): @@ -819,6 +820,17 @@ def _read_and_parse_skill_file( return None name, description = result + + dir_name = Path(skill_dir_path).name + if name != dir_name: + logger.error( + "SKILL.md at '%s' has frontmatter name '%s' that does not match the directory name '%s'; skipping.", + skill_file, + name, + dir_name, + ) + return None + return name, description, content diff --git a/python/packages/core/tests/core/test_skills.py b/python/packages/core/tests/core/test_skills.py index cb829b7b9f..5e6e96ec07 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -285,6 +285,15 @@ def test_skips_invalid_skill(self, tmp_path: Path) -> None: skills = _discover_file_skills([str(tmp_path)]) assert len(skills) == 0 + def test_skips_skill_with_name_directory_mismatch(self, tmp_path: Path) -> None: + skill_dir = tmp_path / "wrong-dir-name" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text( + "---\nname: actual-skill-name\ndescription: A skill.\n---\nBody.", encoding="utf-8" + ) + skills = _discover_file_skills([str(tmp_path)]) + assert len(skills) == 0 + def test_deduplicates_skill_names(self, tmp_path: Path) -> None: dir1 = tmp_path / "dir1" dir2 = tmp_path / "dir2" @@ -820,6 +829,11 @@ def test_name_starts_with_hyphen_skipped(self) -> None: provider = SkillsProvider(skills=[invalid_skill]) assert len(provider._skills) == 0 + def test_name_with_consecutive_hyphens_skipped(self) -> None: + invalid_skill = Skill(name="consecutive--hyphens", description="A skill.", content="Body") + provider = SkillsProvider(skills=[invalid_skill]) + assert len(provider._skills) == 0 + def test_name_too_long_skipped(self) -> None: invalid_skill = Skill(name="a" * 65, description="A skill.", content="Body") provider = SkillsProvider(skills=[invalid_skill]) @@ -1301,6 +1315,11 @@ def test_name_ends_with_hyphen(self) -> None: assert result is not None assert "invalid name" in result + def test_name_with_consecutive_hyphens(self) -> None: + result = _validate_skill_metadata("consecutive--hyphens", "desc", "source") + assert result is not None + assert "invalid name" in result + def test_single_char_name(self) -> None: assert _validate_skill_metadata("a", "desc", "source") is None @@ -1406,6 +1425,15 @@ def test_invalid_frontmatter_returns_none(self, tmp_path: Path) -> None: result = _read_and_parse_skill_file(str(skill_dir)) assert result is None + def test_name_directory_mismatch_returns_none(self, tmp_path: Path) -> None: + skill_dir = tmp_path / "wrong-dir-name" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text( + "---\nname: actual-skill-name\ndescription: A skill.\n---\nBody.", encoding="utf-8" + ) + result = _read_and_parse_skill_file(str(skill_dir)) + assert result is None + # --------------------------------------------------------------------------- # Tests: _create_resource_element