diff --git a/strix/agents/prompts/system_prompt.jinja b/strix/agents/prompts/system_prompt.jinja index eb5c9074..5fc697d7 100644 --- a/strix/agents/prompts/system_prompt.jinja +++ b/strix/agents/prompts/system_prompt.jinja @@ -492,7 +492,7 @@ On-demand specialist skills. Spawn a specialist via `create_agent(skills=[...])` {% for category, skills in available_skills | dictsort -%} {% for skill in skills -%} -- {{ category }}/{{ skill.name }}: {{ skill.description }} +- {{ category }}/{{ skill.name }}{% if skill.description %}: {{ skill.description }}{% endif %} {% endfor -%} {% endfor -%} diff --git a/strix/skills/__init__.py b/strix/skills/__init__.py index 60a9cb1c..334dd8ee 100644 --- a/strix/skills/__init__.py +++ b/strix/skills/__init__.py @@ -4,6 +4,9 @@ import threading from collections import Counter from collections.abc import Iterator from pathlib import Path +from typing import cast + +import yaml from strix.telemetry import posthog, scarf from strix.utils.resource_paths import get_strix_resource_path @@ -17,6 +20,7 @@ _INTERNAL_SKILL_CATEGORIES: frozenset[str] = frozenset({"scan_modes", "coordinat _ROOT_SKILL_CATEGORY = "root" _EXTRA_SKILL_DIRS: list[Path] = [] +_SKILL_METADATA_CACHE: dict[tuple[Path, int, int], dict[str, str]] = {} def register_skill_dir(path: str | Path) -> None: @@ -109,13 +113,18 @@ def _get_ambiguous_skill_names() -> set[str]: return {name for name, count in counts.items() if count > 1} -def _qualified_skill_files(skill_name: str) -> list[Path]: +def _qualified_skill_file_for_name(skill_name: str) -> Path | None: category, _, name = skill_name.partition("/") for skills_dir in skill_search_dirs(): candidate = _qualified_skill_file(skills_dir, category, name) if candidate is not None: - return [candidate] - return [] + return candidate + return None + + +def _qualified_skill_files(skill_name: str) -> list[Path]: + candidate = _qualified_skill_file_for_name(skill_name) + return [candidate] if candidate is not None else [] def _bare_skill_files(skill_name: str) -> list[Path]: @@ -145,25 +154,75 @@ def _bare_skill_files(skill_name: str) -> list[Path]: return candidates -def _skill_description(file_path: Path) -> str: +def _parse_skill_content(content: str) -> tuple[dict[str, str], str]: + """Parse skill frontmatter once and return metadata plus markdown body.""" + frontmatter = _FRONTMATTER_PATTERN.match(content) + if frontmatter is None: + return {}, content.lstrip() + + try: + metadata_text = frontmatter.group(0)[4:].rsplit("\n---", 1)[0] + try: + metadata_value: object = yaml.safe_load(metadata_text) or {} + except yaml.YAMLError: + # Preserve the historical support for an unquoted colon in a + # description while accepting full YAML for other frontmatter. + normalized_lines: list[str] = [] + for line in metadata_text.splitlines(): + if line.startswith("description: ") and not line.startswith( + ('description: "', "description: '") + ): + value = line.removeprefix("description: ") + value = value.replace("\\", "\\\\").replace('"', '\\"') + normalized_lines.append(f'description: "{value}"') + else: + normalized_lines.append(line) + metadata_value = yaml.safe_load("\n".join(normalized_lines)) or {} + except yaml.YAMLError: + logger.warning("Invalid skill frontmatter") + metadata_value = {} + metadata = ( + cast("dict[object, object]", metadata_value) if isinstance(metadata_value, dict) else {} + ) + + normalized = {str(key): "" if value is None else str(value) for key, value in metadata.items()} + return normalized, content[frontmatter.end() :].lstrip() + + +def _read_skill_metadata(file_path: Path) -> dict[str, str]: + try: + stat = file_path.stat() + except OSError: + logger.warning("Skill file disappeared while reading metadata: %s", file_path) + return {} + cache_key = (file_path, stat.st_mtime_ns, stat.st_size) + cached = _SKILL_METADATA_CACHE.get(cache_key) + if cached is not None: + return cached try: content = file_path.read_text(encoding="utf-8") except (OSError, ValueError): - return "" - frontmatter = _FRONTMATTER_PATTERN.match(content) - if frontmatter is None: - return "" - match = re.search(r"^description:\s*(.+?)\s*$", frontmatter.group(0), re.MULTILINE) - return match.group(1).strip() if match else "" + logger.warning("Failed to read skill metadata: %s", file_path) + return {} + metadata, _ = _parse_skill_content(content) + _SKILL_METADATA_CACHE[cache_key] = metadata + return metadata def get_available_skills() -> dict[str, list[dict[str, str]]]: grouped: dict[str, list[dict[str, str]]] = {} for category, name in _iter_user_skill_files(): - path = _qualified_skill_files(f"{category}/{name}") - grouped.setdefault(category, []).append( - {"name": name, "description": _skill_description(path[0]) if path else ""} - ) + file_path = _qualified_skill_file_for_name(f"{category}/{name}") + if file_path is None: + logger.warning( + "Skill disappeared while gathering available skills: %s/%s", + category, + name, + ) + continue + metadata = _read_skill_metadata(file_path) + description = " ".join(metadata.get("description", "").split()) + grouped.setdefault(category, []).append({"name": name, "description": description}) return grouped @@ -243,7 +302,8 @@ def load_skills(skill_names: list[str]) -> dict[str, str]: continue var_name = skill_name.split("/")[-1] - skill_content[var_name] = _FRONTMATTER_PATTERN.sub("", content).lstrip() + _, skill_body = _parse_skill_content(content) + skill_content[var_name] = skill_body logger.debug("Loaded skill: %s -> %s", skill_name, var_name) _track_skill_loaded(var_name, file_path) diff --git a/strix/skills/technologies/firebase.md b/strix/skills/technologies/firebase.md index 7bf90232..34104033 100644 --- a/strix/skills/technologies/firebase.md +++ b/strix/skills/technologies/firebase.md @@ -164,7 +164,6 @@ Storage rules use OR-across-matches semantics: a later permissive match can reop **Tests** - GET GCS object paths via HTTPS without auth; verify Content-Type and `Content-Disposition: attachment` -- Test the Firebase Storage list/read/write matrix above with each available principal - Generate and reuse signed URLs across accounts and paths; try case/URL-encoding variants - Upload HTML/SVG and verify `X-Content-Type-Options: nosniff`; check for script execution diff --git a/tests/test_skill_dir_extension.py b/tests/test_skill_dir_extension.py index 11ffa142..88532da0 100644 --- a/tests/test_skill_dir_extension.py +++ b/tests/test_skill_dir_extension.py @@ -84,12 +84,64 @@ def test_available_skill_includes_frontmatter_description(tmp_path: Path) -> Non ] +def test_available_skill_supports_colon_in_description(tmp_path: Path) -> None: + _write_skill( + tmp_path, + "extra", + "widget", + "---\nname: widget\ndescription: Useful widget: handles YAML\n---\nwidget body", + ) + register_skill_dir(tmp_path) + + assert get_available_skills()["extra"] == [ + {"name": "widget", "description": "Useful widget: handles YAML"} + ] + + +def test_available_skill_normalizes_quoted_multiline_description(tmp_path: Path) -> None: + _write_skill( + tmp_path, + "extra", + "widget", + '---\nname: widget\ndescription: "Useful: widget guidance"\n---\nwidget body', + ) + register_skill_dir(tmp_path) + + assert get_available_skills()["extra"] == [ + {"name": "widget", "description": "Useful: widget guidance"} + ] + + +def test_available_skill_normalizes_block_description(tmp_path: Path) -> None: + _write_skill( + tmp_path, + "extra", + "widget", + "---\nname: widget\ndescription: |\n First line\n Second line\n---\nwidget body", + ) + register_skill_dir(tmp_path) + + assert get_available_skills()["extra"] == [ + {"name": "widget", "description": "First line Second line"} + ] + + def test_system_prompt_renders_skill_descriptions() -> None: prompt = render_system_prompt(scan_mode="quick", is_root=True) assert "- technologies/firebase: Firebase security testing covering" in prompt +def test_system_prompt_omits_empty_skill_description(tmp_path: Path) -> None: + _write_skill(tmp_path, "extra", "widget", "---\nname: widget\ndescription:\n---\nwidget body") + register_skill_dir(tmp_path) + + prompt = render_system_prompt(scan_mode="quick", is_root=True) + + assert "- extra/widget\n" in prompt + assert "- extra/widget: " not in prompt + + def test_registered_root_skill_is_discoverable_and_valid(tmp_path: Path) -> None: _write_root_skill(tmp_path, "widget", "widget body") register_skill_dir(tmp_path)