diff --git a/libs/deepagents/deepagents/middleware/skills.py b/libs/deepagents/deepagents/middleware/skills.py index c05f4a07f5..81f6ac8398 100644 --- a/libs/deepagents/deepagents/middleware/skills.py +++ b/libs/deepagents/deepagents/middleware/skills.py @@ -351,12 +351,18 @@ def _validate_skill_name(name: str, directory_name: str) -> tuple[bool, str]: def _parse_allowed_tools(raw_tools: object, skill_path: str) -> list[str]: - """Parse the `allowed-tools` frontmatter value into a list of tool names.""" + """Parse the `allowed-tools` frontmatter value into a list of tool names. + + Accepts either a space- or comma-separated string or a YAML list of strings. + Non-string list items and empty entries are skipped. + """ if isinstance(raw_tools, str): - return [t.strip(",").strip() for t in raw_tools.split() if t.strip(",").strip()] + return [tool for tool in re.split(r"[\s,]+", raw_tools) if tool] + if isinstance(raw_tools, list): + return [t.strip() for t in raw_tools if isinstance(t, str) and t.strip()] if raw_tools is not None: logger.warning( - "Ignoring non-string 'allowed-tools' in %s (got %s)", + "Ignoring 'allowed-tools' in %s: expected a string or list, got %s", skill_path, type(raw_tools).__name__, ) diff --git a/libs/deepagents/tests/unit_tests/middleware/test_skills_middleware.py b/libs/deepagents/tests/unit_tests/middleware/test_skills_middleware.py index 8d3c62a373..a985de4c92 100644 --- a/libs/deepagents/tests/unit_tests/middleware/test_skills_middleware.py +++ b/libs/deepagents/tests/unit_tests/middleware/test_skills_middleware.py @@ -480,7 +480,8 @@ def test_validate_metadata_valid_dict_passthrough() -> None: assert result == {"author": "acme"} -def test_parse_skill_metadata_allowed_tools_yaml_list_ignored() -> None: +def test_parse_skill_metadata_allowed_tools_yaml_list() -> None: + """Test _parse_skill_metadata accepts a YAML list of tool names.""" content = """--- name: test-skill description: A test skill @@ -495,10 +496,11 @@ def test_parse_skill_metadata_allowed_tools_yaml_list_ignored() -> None: result = _parse_skill_metadata(content, "/skills/test-skill/SKILL.md", "test-skill") assert result is not None - assert result["allowed_tools"] == [] + assert result["allowed_tools"] == ["Bash", "Read", "Write"] -def test_parse_skill_metadata_allowed_tools_yaml_list_non_strings_ignored() -> None: +def test_parse_skill_metadata_allowed_tools_yaml_list_non_strings_skipped() -> None: + """Test _parse_skill_metadata skips non-string and blank YAML list items.""" content = """--- name: test-skill description: A test skill @@ -511,6 +513,72 @@ def test_parse_skill_metadata_allowed_tools_yaml_list_non_strings_ignored() -> N - Write --- +Content +""" + + result = _parse_skill_metadata(content, "/skills/test-skill/SKILL.md", "test-skill") + assert result is not None + assert result["allowed_tools"] == ["Read", "Write"] + + +def test_parse_skill_metadata_allowed_tools_yaml_list_trims_items() -> None: + """Test _parse_skill_metadata strips surrounding whitespace from kept items.""" + content = """--- +name: test-skill +description: A test skill +allowed-tools: + - " Read " + - "Write" +--- + +Content +""" + + result = _parse_skill_metadata(content, "/skills/test-skill/SKILL.md", "test-skill") + assert result is not None + assert result["allowed_tools"] == ["Read", "Write"] + + +def test_parse_skill_metadata_allowed_tools_empty_list() -> None: + """Test _parse_skill_metadata handles an empty YAML list.""" + content = """--- +name: test-skill +description: A test skill +allowed-tools: [] +--- + +Content +""" + + result = _parse_skill_metadata(content, "/skills/test-skill/SKILL.md", "test-skill") + assert result is not None + assert result["allowed_tools"] == [] + + +def test_parse_skill_metadata_allowed_tools_comma_separated_string() -> None: + """Test _parse_skill_metadata splits a comma-separated string of tool names.""" + content = """--- +name: test-skill +description: A test skill +allowed-tools: Bash,Read,Write +--- + +Content +""" + + result = _parse_skill_metadata(content, "/skills/test-skill/SKILL.md", "test-skill") + assert result is not None + assert result["allowed_tools"] == ["Bash", "Read", "Write"] + + +def test_parse_skill_metadata_allowed_tools_scalar_ignored() -> None: + """Test _parse_skill_metadata ignores a non-string, non-list scalar value.""" + content = """--- +name: test-skill +description: A test skill +allowed-tools: 42 +--- + Content """