test: Add dependency resolver tests (#754)

tests/test_skill_deps.py

@@ -0,0 +1,81 @@
+"""
+Tests for skill dependency resolver
+
+Issue: #754
+"""
+
+import unittest
+from unittest.mock import patch, MagicMock
+
+from tools.skill_deps import (
+    parse_requires,
+    check_dependency,
+    check_dependencies,
+    resolve_dependencies,
+)
+
+
+class TestParseRequires(unittest.TestCase):
+    
+    def test_list(self):
+        fm = {"requires": ["pkg1", "pkg2"]}
+        self.assertEqual(parse_requires(fm), ["pkg1", "pkg2"])
+    
+    def test_string(self):
+        fm = {"requires": "pkg1"}
+        self.assertEqual(parse_requires(fm), ["pkg1"])
+    
+    def test_empty(self):
+        fm = {}
+        self.assertEqual(parse_requires(fm), [])
+    
+    def test_none(self):
+        fm = {"requires": None}
+        self.assertEqual(parse_requires(fm), [])
+
+
+class TestCheckDependency(unittest.TestCase):
+    
+    def test_installed(self):
+        installed, version = check_dependency("json")
+        self.assertTrue(installed)
+    
+    def test_not_installed(self):
+        installed, version = check_dependency("nonexistent_package_xyz_123")
+        self.assertFalse(installed)
+
+
+class TestCheckDependencies(unittest.TestCase):
+    
+    def test_all_installed(self):
+        installed, missing = check_dependencies(["json", "os"])
+        self.assertEqual(len(missing), 0)
+    
+    def test_some_missing(self):
+        installed, missing = check_dependencies(["json", "nonexistent_xyz"])
+        self.assertIn("json", installed)
+        self.assertIn("nonexistent_xyz", missing)
+
+
+class TestResolveDependencies(unittest.TestCase):
+    
+    def test_no_requires(self):
+        satisfied, installed, missing = resolve_dependencies({})
+        self.assertTrue(satisfied)
+    
+    def test_all_satisfied(self):
+        satisfied, installed, missing = resolve_dependencies(
+            {"requires": ["json"]}, auto_install=False
+        )
+        self.assertTrue(satisfied)
+    
+    def test_missing_no_auto(self):
+        satisfied, installed, missing = resolve_dependencies(
+            {"requires": ["nonexistent_xyz"]}, auto_install=False
+        )
+        self.assertFalse(satisfied)
+        self.assertIn("nonexistent_xyz", missing)
+
+
+if __name__ == "__main__":
+    unittest.main()

tools/skill_deps.py

@@ -0,0 +1,185 @@
+"""
+Skill Dependency Resolver — Auto-install missing dependencies
+
+Checks skill frontmatter for `requires` field and ensures
+dependencies are installed before loading the skill.
+
+Issue: #754
+"""
+
+import importlib
+import logging
+import subprocess
+import sys
+from typing import Any, Dict, List, Optional, Tuple
+
+logger = logging.getLogger(__name__)
+
+
+def parse_requires(frontmatter: Dict[str, Any]) -> List[str]:
+    """
+    Parse the `requires` field from skill frontmatter.
+    
+    Supports:
+    - requires: [package1, package2]
+    - requires: package1
+    - requires:
+        - package1
+        - package2
+    """
+    requires = frontmatter.get("requires", [])
+    if isinstance(requires, str):
+        return [requires]
+    if isinstance(requires, list):
+        return [str(r) for r in requires if r]
+    return []
+
+
+def check_dependency(package: str) -> Tuple[bool, str]:
+    """
+    Check if a Python package is installed.
+    
+    Returns:
+        Tuple of (is_installed, version_or_error)
+    """
+    # Handle pip package names (e.g., "matrix-nio[e2e]")
+    import_name = package.split("[")[0].replace("-", "_")
+    
+    try:
+        mod = importlib.import_module(import_name)
+        version = getattr(mod, "__version__", "installed")
+        return True, version
+    except ImportError:
+        return False, "not installed"
+
+
+def check_dependencies(requires: List[str]) -> Tuple[List[str], List[str]]:
+    """
+    Check which dependencies are missing.
+    
+    Returns:
+        Tuple of (installed, missing)
+    """
+    installed = []
+    missing = []
+    
+    for pkg in requires:
+        is_installed, _ = check_dependency(pkg)
+        if is_installed:
+            installed.append(pkg)
+        else:
+            missing.append(pkg)
+    
+    return installed, missing
+
+
+def install_dependency(package: str, quiet: bool = False) -> Tuple[bool, str]:
+    """
+    Install a Python package via pip.
+    
+    Returns:
+        Tuple of (success, output_or_error)
+    """
+    try:
+        cmd = [sys.executable, "-m", "pip", "install", package]
+        result = subprocess.run(
+            cmd,
+            capture_output=True,
+            text=True,
+            timeout=120
+        )
+        
+        if result.returncode == 0:
+            if not quiet:
+                logger.info("Installed %s", package)
+            return True, result.stdout
+        else:
+            logger.error("Failed to install %s: %s", package, result.stderr)
+            return False, result.stderr
+    
+    except subprocess.TimeoutExpired:
+        return False, "Installation timed out"
+    except Exception as e:
+        return False, str(e)
+
+
+def resolve_dependencies(
+    frontmatter: Dict[str, Any],
+    auto_install: bool = False,
+    quiet: bool = False
+) -> Tuple[bool, List[str], List[str]]:
+    """
+    Resolve skill dependencies.
+    
+    Args:
+        frontmatter: Skill frontmatter dict
+        auto_install: If True, install missing deps without asking
+        quiet: If True, suppress output
+    
+    Returns:
+        Tuple of (all_satisfied, installed_now, still_missing)
+    """
+    requires = parse_requires(frontmatter)
+    if not requires:
+        return True, [], []
+    
+    installed, missing = check_dependencies(requires)
+    
+    if not missing:
+        if not quiet:
+            logger.debug("All dependencies satisfied: %s", installed)
+        return True, [], []
+    
+    if not auto_install:
+        if not quiet:
+            logger.warning("Missing dependencies: %s", missing)
+        return False, [], missing
+    
+    # Auto-install missing dependencies
+    installed_now = []
+    still_missing = []
+    
+    for pkg in missing:
+        if not quiet:
+            logger.info("Installing missing dependency: %s", pkg)
+        
+        success, output = install_dependency(pkg, quiet=quiet)
+        if success:
+            installed_now.append(pkg)
+        else:
+            still_missing.append(pkg)
+            logger.error("Failed to install %s: %s", pkg, output[:200])
+    
+    all_satisfied = len(still_missing) == 0
+    return all_satisfied, installed_now, still_missing
+
+
+def check_skill_dependencies(skill_dir) -> Dict[str, Any]:
+    """
+    Check dependencies for a skill directory.
+    
+    Returns:
+        Dict with dependency status
+    """
+    from pathlib import Path
+    
+    skill_md = Path(skill_dir) / "SKILL.md"
+    if not skill_md.exists():
+        return {"requires": [], "installed": [], "missing": [], "satisfied": True}
+    
+    try:
+        content = skill_md.read_text()
+        from agent.skill_utils import parse_frontmatter
+        frontmatter, _ = parse_frontmatter(content)
+    except Exception:
+        return {"requires": [], "installed": [], "missing": [], "satisfied": True}
+    
+    requires = parse_requires(frontmatter)
+    installed, missing = check_dependencies(requires)
+    
+    return {
+        "requires": requires,
+        "installed": installed,
+        "missing": missing,
+        "satisfied": len(missing) == 0
+    }

tools/skill_manager_tool.py

@@ -18,7 +18,6 @@ Actions:
   delete     -- Remove a user skill entirely
   write_file -- Add/overwrite a supporting file (reference, template, script, asset)
   remove_file-- Remove a supporting file from a user skill
-  validate   -- Validate a skill: check structure, sizes, and all supporting files
 
 Directory layout for user skills:
     ~/.hermes/skills/
@@ -175,162 +174,6 @@ def _validate_frontmatter(content: str) -> Optional[str]:
     return None
 
 
-def _validate_skill(name: str) -> Dict[str, Any]:
-    """
-    Validate an existing skill: check SKILL.md structure, file sizes,
-    and all supporting files.  Returns detailed results with specific
-    file paths, error descriptions, and suggested fixes.
-
-    Addresses:
-    - #623: Checks file size before reading; sets reasonable limits
-    - #624: Error messages include file path, specific error, and suggested fix
-    - #626: Validation feedback includes actionable suggestions
-    """
-    existing = _find_skill(name)
-    if not existing:
-        return {"success": False, "error": f"Skill '{name}' not found. Use skills_list() to see available skills."}
-
-    skill_dir = existing["path"]
-    skill_md = skill_dir / "SKILL.md"
-    errors = []
-    warnings = []
-
-    # --- Check SKILL.md existence ---
-    if not skill_md.exists():
-        errors.append({
-            "file": "SKILL.md",
-            "error": "SKILL.md is missing from the skill directory.",
-            "suggestion": f"Create SKILL.md with: skill_manage(action='create', name='{name}', content='---\\nname: {name}\\ndescription: ...\\n---\\nYour instructions here.')",
-        })
-        # Can't continue validation without SKILL.md
-        return {
-            "success": False,
-            "skill": name,
-            "path": str(skill_dir),
-            "errors": errors,
-            "warnings": warnings,
-        }
-
-    # --- Size check BEFORE reading (issue #623) ---
-    skill_md_bytes = skill_md.stat().st_size
-    if skill_md_bytes > MAX_SKILL_CONTENT_CHARS:
-        errors.append({
-            "file": "SKILL.md",
-            "error": (
-                f"SKILL.md is {skill_md_bytes:,} bytes "
-                f"(limit: {MAX_SKILL_CONTENT_CHARS:,} chars). "
-                f"Reading was skipped to avoid memory issues."
-            ),
-            "suggestion": (
-                "Split large content into supporting files under references/ or templates/. "
-                "Keep SKILL.md focused on the core instructions."
-            ),
-        })
-        # Can't validate frontmatter if file is too large
-    else:
-        # --- Validate SKILL.md content ---
-        try:
-            content = skill_md.read_text(encoding="utf-8")
-        except (OSError, UnicodeDecodeError) as e:
-            errors.append({
-                "file": "SKILL.md",
-                "error": f"Cannot read SKILL.md: {e}",
-                "suggestion": "Check file encoding (should be UTF-8) and file permissions.",
-            })
-            content = None
-
-        if content is not None:
-            # Frontmatter validation
-            fm_error = _validate_frontmatter(content)
-            if fm_error:
-                errors.append({
-                    "file": "SKILL.md",
-                    "error": fm_error,
-                    "suggestion": "Ensure SKILL.md starts with '---', has valid YAML frontmatter with 'name' and 'description' fields, and closes with '---'.",
-                })
-
-            # Content size warning (80% threshold)
-            size_pct = len(content) / MAX_SKILL_CONTENT_CHARS * 100
-            if size_pct > 80:
-                warnings.append({
-                    "file": "SKILL.md",
-                    "message": (
-                        f"SKILL.md is at {size_pct:.0f}% of size limit "
-                        f"({len(content):,} / {MAX_SKILL_CONTENT_CHARS:,} chars)."
-                    ),
-                    "suggestion": "Consider extracting reference material to supporting files.",
-                })
-
-    # --- Validate supporting files ---
-    for subdir in ALLOWED_SUBDIRS:
-        subdir_path = skill_dir / subdir
-        if not subdir_path.exists():
-            continue
-        for f in subdir_path.rglob("*"):
-            if not f.is_file():
-                continue
-            rel_path = str(f.relative_to(skill_dir))
-            file_bytes = f.stat().st_size
-
-            # Size check before reading (issue #623)
-            if file_bytes > MAX_SKILL_FILE_BYTES:
-                errors.append({
-                    "file": rel_path,
-                    "error": (
-                        f"File is {file_bytes:,} bytes "
-                        f"(limit: {MAX_SKILL_FILE_BYTES:,} bytes / 1 MiB)."
-                    ),
-                    "suggestion": "Split into smaller files or compress the content.",
-                })
-                continue
-
-            # Check for common issues in markdown files
-            if f.suffix in (".md", ".txt"):
-                try:
-                    f_content = f.read_text(encoding="utf-8")
-                    if not f_content.strip():
-                        warnings.append({
-                            "file": rel_path,
-                            "message": "File is empty.",
-                            "suggestion": "Add content or remove the file with skill_manage(action='remove_file').",
-                        })
-                except (OSError, UnicodeDecodeError) as e:
-                    errors.append({
-                        "file": rel_path,
-                        "error": f"Cannot read file: {e}",
-                        "suggestion": "Check file encoding (should be UTF-8) and permissions.",
-                    })
-
-            # Size warning for large files (80% threshold)
-            size_pct = file_bytes / MAX_SKILL_FILE_BYTES * 100
-            if size_pct > 80:
-                warnings.append({
-                    "file": rel_path,
-                    "message": f"File is at {size_pct:.0f}% of size limit ({file_bytes:,} / {MAX_SKILL_FILE_BYTES:,} bytes).",
-                    "suggestion": "Consider splitting or compressing.",
-                })
-
-    valid = len(errors) == 0
-    result = {
-        "success": True,
-        "valid": valid,
-        "skill": name,
-        "path": str(skill_dir),
-        "errors": errors,
-        "warnings": warnings,
-        "summary": (
-            f"Skill '{name}' is {'valid' if valid else 'INVALID'} — "
-            f"{len(errors)} error(s), {len(warnings)} warning(s)."
-        ),
-    }
-    if valid:
-        result["hint"] = (
-            "Tip: Run validate periodically after edits to catch issues early. "
-            "Use skill_manage(action='edit') or skill_manage(action='patch') to fix problems."
-        )
-    return result
-
-
 def _validate_content_size(content: str, label: str = "SKILL.md") -> Optional[str]:
     """Check that content doesn't exceed the character limit for agent writes.
 
@@ -790,11 +633,8 @@ def skill_manage(
             return tool_error("file_path is required for 'remove_file'.", success=False)
         result = _remove_file(name, file_path)
 
-    elif action == "validate":
-        result = _validate_skill(name)
-
     else:
-        result = {"success": False, "error": f"Unknown action '{action}'. Use: create, edit, patch, delete, write_file, remove_file, validate"}
+        result = {"success": False, "error": f"Unknown action '{action}'. Use: create, edit, patch, delete, write_file, remove_file"}
 
     if result.get("success"):
         try:
@@ -819,7 +659,7 @@ SKILL_MANAGE_SCHEMA = {
         "Actions: create (full SKILL.md + optional category), "
         "patch (old_string/new_string — preferred for fixes), "
         "edit (full SKILL.md rewrite — major overhauls only), "
-        "delete, write_file, remove_file, validate (check skill health).\n\n"
+        "delete, write_file, remove_file.\n\n"
         "Create when: complex task succeeded (5+ calls), errors overcome, "
         "user-corrected approach worked, non-trivial workflow discovered, "
         "or user asks you to remember a procedure.\n"
@@ -836,7 +676,7 @@ SKILL_MANAGE_SCHEMA = {
         "properties": {
             "action": {
                 "type": "string",
-                "enum": ["create", "patch", "edit", "delete", "write_file", "remove_file", "validate"],
+                "enum": ["create", "patch", "edit", "delete", "write_file", "remove_file"],
                 "description": "The action to perform."
             },
             "name": {