minor bug fix, and lint issue fixes
This commit is contained in:
@@ -191,9 +191,7 @@ def register_skill_commands(subparsers) -> None:
|
||||
search_parser.set_defaults(func=cmd_skill_search)
|
||||
|
||||
# hive skill fork
|
||||
fork_parser = skill_sub.add_parser(
|
||||
"fork", help="Create a local editable copy of a skill"
|
||||
)
|
||||
fork_parser = skill_sub.add_parser("fork", help="Create a local editable copy of a skill")
|
||||
fork_parser.add_argument("name", help="Skill name to fork")
|
||||
fork_parser.add_argument(
|
||||
"--name",
|
||||
@@ -281,6 +279,7 @@ def cmd_skill_install(args) -> int:
|
||||
from framework.skills.skill_errors import SkillError
|
||||
|
||||
maybe_show_install_notice()
|
||||
sys.stdout.flush()
|
||||
|
||||
target_dir = USER_SKILLS_DIR
|
||||
|
||||
@@ -291,7 +290,7 @@ def cmd_skill_install(args) -> int:
|
||||
# hive skill install --from <url> [--name <name>]
|
||||
if args.from_url:
|
||||
skill_name = args.install_name or _derive_name_from_url(args.from_url)
|
||||
print(f"Installing '{skill_name}' from {args.from_url} ...")
|
||||
print(f"Installing '{skill_name}' from {args.from_url} ...", flush=True)
|
||||
try:
|
||||
dest = install_from_git(
|
||||
git_url=args.from_url,
|
||||
@@ -489,7 +488,7 @@ def cmd_skill_validate(args) -> int:
|
||||
|
||||
def cmd_skill_doctor(args) -> int:
|
||||
"""Health-check skills: parseable, scripts executable, tools available."""
|
||||
from framework.skills.defaults import SKILL_REGISTRY, _DEFAULT_SKILLS_DIR
|
||||
from framework.skills.defaults import _DEFAULT_SKILLS_DIR, SKILL_REGISTRY
|
||||
from framework.skills.discovery import DiscoveryConfig, SkillDiscovery
|
||||
from framework.skills.parser import parse_skill_md
|
||||
|
||||
@@ -510,6 +509,16 @@ def cmd_skill_doctor(args) -> int:
|
||||
if args.name:
|
||||
skills = [s for s in skills if s.name == args.name]
|
||||
if not skills:
|
||||
# Skill failed to parse (e.g. missing description) — look for the file directly
|
||||
from framework.skills.installer import USER_SKILLS_DIR
|
||||
|
||||
candidate = USER_SKILLS_DIR / args.name / "SKILL.md"
|
||||
if candidate.exists():
|
||||
print(f"\nChecking skill: {args.name} [user]")
|
||||
overall_errors += _doctor_skill_file(args.name, candidate, parse_skill_md)
|
||||
print()
|
||||
print(f"✗ {overall_errors} error(s) found.")
|
||||
return 1
|
||||
print(f"Error: skill '{args.name}' not found.", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
@@ -796,7 +805,7 @@ def _doctor_skill_file(skill_name: str, skill_md: Path, parse_fn) -> int:
|
||||
print(f" ✗ SKILL.md not parseable: {skill_md}")
|
||||
errors += 1
|
||||
return errors
|
||||
print(f" ✓ SKILL.md parseable")
|
||||
print(" ✓ SKILL.md parseable")
|
||||
|
||||
base_dir = skill_md.parent
|
||||
|
||||
|
||||
@@ -49,7 +49,7 @@ def maybe_show_install_notice() -> None:
|
||||
"""
|
||||
if INSTALL_NOTICE_SENTINEL.exists():
|
||||
return
|
||||
print(_INSTALL_NOTICE)
|
||||
print(_INSTALL_NOTICE, flush=True)
|
||||
try:
|
||||
INSTALL_NOTICE_SENTINEL.parent.mkdir(parents=True, exist_ok=True)
|
||||
INSTALL_NOTICE_SENTINEL.touch()
|
||||
@@ -293,14 +293,14 @@ def _git_clone_shallow(git_url: str, target: Path, version: str | None = None) -
|
||||
what=f"git clone timed out for {git_url}",
|
||||
why="The clone operation took longer than 60 seconds.",
|
||||
fix="Check your network connection and retry.",
|
||||
)
|
||||
) from None
|
||||
except (FileNotFoundError, OSError) as exc:
|
||||
raise SkillError(
|
||||
code=SkillErrorCode.SKILL_ACTIVATION_FAILED,
|
||||
what=f"Cannot run git for {git_url}",
|
||||
why=str(exc),
|
||||
fix="Ensure git is installed and on PATH.",
|
||||
)
|
||||
) from exc
|
||||
|
||||
if result.returncode != 0:
|
||||
stderr = result.stderr.strip()
|
||||
@@ -314,9 +314,7 @@ def _git_clone_shallow(git_url: str, target: Path, version: str | None = None) -
|
||||
|
||||
def _copy_skill_dir(src: Path, dst: Path) -> None:
|
||||
"""Copy a skill directory, ignoring VCS and cache artifacts."""
|
||||
ignore = shutil.ignore_patterns(
|
||||
".git", "__pycache__", "*.pyc", ".venv", "venv", "node_modules"
|
||||
)
|
||||
ignore = shutil.ignore_patterns(".git", "__pycache__", "*.pyc", ".venv", "venv", "node_modules")
|
||||
shutil.copytree(src, dst, ignore=ignore)
|
||||
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@ import os
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
|
||||
from framework.skills.parser import _MAX_NAME_LENGTH, parse_skill_md
|
||||
from framework.skills.parser import _MAX_NAME_LENGTH
|
||||
|
||||
|
||||
@dataclass
|
||||
@@ -141,8 +141,7 @@ def validate_strict(path: Path) -> ValidationResult:
|
||||
if script_path.is_file():
|
||||
if not os.access(script_path, os.X_OK):
|
||||
errors.append(
|
||||
f"Script not executable: {script_path.name}. "
|
||||
f"Run: chmod +x {script_path}"
|
||||
f"Script not executable: {script_path.name}. Run: chmod +x {script_path}"
|
||||
)
|
||||
|
||||
# 12. allowed-tools entries are non-empty strings — warning if malformed
|
||||
@@ -153,9 +152,7 @@ def validate_strict(path: Path) -> ValidationResult:
|
||||
else:
|
||||
for tool in allowed_tools:
|
||||
if not isinstance(tool, str) or not tool.strip():
|
||||
warnings.append(
|
||||
f"'allowed-tools' entry {tool!r} is not a non-empty string."
|
||||
)
|
||||
warnings.append(f"'allowed-tools' entry {tool!r} is not a non-empty string.")
|
||||
|
||||
# 13. compatibility is a list of strings — error if malformed
|
||||
compatibility = frontmatter.get("compatibility")
|
||||
@@ -165,9 +162,7 @@ def validate_strict(path: Path) -> ValidationResult:
|
||||
else:
|
||||
for item in compatibility:
|
||||
if not isinstance(item, str):
|
||||
errors.append(
|
||||
f"'compatibility' entry {item!r} is not a string."
|
||||
)
|
||||
errors.append(f"'compatibility' entry {item!r} is not a string.")
|
||||
|
||||
# 14. metadata is a dict — error if malformed
|
||||
metadata = frontmatter.get("metadata")
|
||||
|
||||
@@ -7,19 +7,15 @@ from __future__ import annotations
|
||||
|
||||
from argparse import Namespace
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from unittest.mock import patch
|
||||
|
||||
from framework.skills.cli import (
|
||||
cmd_skill_doctor,
|
||||
cmd_skill_fork,
|
||||
cmd_skill_info,
|
||||
cmd_skill_init,
|
||||
cmd_skill_install,
|
||||
cmd_skill_remove,
|
||||
cmd_skill_search,
|
||||
cmd_skill_update,
|
||||
cmd_skill_validate,
|
||||
)
|
||||
|
||||
@@ -116,6 +112,7 @@ class TestCmdSkillDoctor:
|
||||
args = Namespace(name=None, defaults=False, project_dir=str(tmp_path))
|
||||
with patch("framework.skills.discovery.SkillDiscovery.discover") as mock_discover:
|
||||
from framework.skills.parser import ParsedSkill
|
||||
|
||||
mock_discover.return_value = [
|
||||
ParsedSkill(
|
||||
name="my-skill",
|
||||
@@ -171,7 +168,9 @@ class TestCmdSkillInstall:
|
||||
version=None,
|
||||
)
|
||||
|
||||
with patch("framework.skills.installer.install_from_git", return_value=installed_path) as mock_install:
|
||||
with patch(
|
||||
"framework.skills.installer.install_from_git", return_value=installed_path
|
||||
) as mock_install:
|
||||
result = cmd_skill_install(args)
|
||||
|
||||
mock_install.assert_called_once()
|
||||
@@ -203,7 +202,9 @@ class TestCmdSkillInstall:
|
||||
sentinel.touch()
|
||||
monkeypatch.setattr("framework.skills.installer.INSTALL_NOTICE_SENTINEL", sentinel)
|
||||
|
||||
args = Namespace(name_or_url=None, from_url=None, pack=None, install_name=None, version=None)
|
||||
args = Namespace(
|
||||
name_or_url=None, from_url=None, pack=None, install_name=None, version=None
|
||||
)
|
||||
result = cmd_skill_install(args)
|
||||
assert result == 1
|
||||
|
||||
@@ -215,7 +216,7 @@ class TestCmdSkillRemove:
|
||||
skill_dir.mkdir(parents=True)
|
||||
|
||||
with patch("framework.skills.installer.USER_SKILLS_DIR", skills_dir):
|
||||
with patch("framework.skills.installer.remove_skill", return_value=True) as mock_remove:
|
||||
with patch("framework.skills.installer.remove_skill", return_value=True):
|
||||
args = Namespace(name="my-skill")
|
||||
result = cmd_skill_remove(args)
|
||||
|
||||
|
||||
@@ -2,18 +2,14 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from framework.skills.installer import (
|
||||
INSTALL_NOTICE_SENTINEL,
|
||||
USER_SKILLS_DIR,
|
||||
fork_skill,
|
||||
install_from_git,
|
||||
install_from_registry,
|
||||
maybe_show_install_notice,
|
||||
remove_skill,
|
||||
)
|
||||
@@ -55,6 +51,7 @@ class TestInstallFromGit:
|
||||
def fake_clone(git_url, target_path, version=None):
|
||||
# Simulate git clone by copying source_repo into target_path
|
||||
import shutil
|
||||
|
||||
if target_path.exists():
|
||||
shutil.rmtree(target_path)
|
||||
shutil.copytree(source_repo, target_path)
|
||||
@@ -87,6 +84,7 @@ class TestInstallFromGit:
|
||||
|
||||
def fake_clone(git_url, target_path, version=None):
|
||||
import shutil
|
||||
|
||||
if target_path.exists():
|
||||
shutil.rmtree(target_path)
|
||||
shutil.copytree(empty_repo, target_path)
|
||||
@@ -126,8 +124,10 @@ class TestInstallFromGit:
|
||||
return d
|
||||
|
||||
def failing_clone(git_url, target_path, version=None):
|
||||
from framework.skills.skill_errors import SkillErrorCode as SEC
|
||||
|
||||
raise SkillError(
|
||||
code=__import__("framework.skills.skill_errors", fromlist=["SkillErrorCode"]).SkillErrorCode.SKILL_ACTIVATION_FAILED,
|
||||
code=SEC.SKILL_ACTIVATION_FAILED,
|
||||
what="clone failed",
|
||||
why="network error",
|
||||
fix="check network",
|
||||
@@ -167,7 +167,7 @@ class TestRemoveSkill:
|
||||
|
||||
def test_raises_on_permission_error(self, tmp_path):
|
||||
skills_dir = tmp_path / "skills"
|
||||
skill_dir = _make_skill_dir(skills_dir, "locked-skill")
|
||||
_make_skill_dir(skills_dir, "locked-skill")
|
||||
|
||||
with patch("shutil.rmtree", side_effect=OSError("permission denied")):
|
||||
with pytest.raises(SkillError) as exc_info:
|
||||
@@ -194,6 +194,7 @@ class TestForkSkill:
|
||||
dest = fork_skill(source, "forked", target_parent)
|
||||
|
||||
import yaml
|
||||
|
||||
content = (dest / "SKILL.md").read_text(encoding="utf-8")
|
||||
parts = content.split("---", 2)
|
||||
fm = yaml.safe_load(parts[1])
|
||||
|
||||
@@ -5,12 +5,12 @@ from __future__ import annotations
|
||||
import json
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
from unittest.mock import patch
|
||||
from urllib.error import URLError
|
||||
|
||||
import pytest
|
||||
|
||||
from framework.skills.registry import RegistryClient, _CACHE_TTL_SECONDS
|
||||
from framework.skills.registry import _CACHE_TTL_SECONDS, RegistryClient
|
||||
|
||||
_SAMPLE_INDEX = {
|
||||
"version": 1,
|
||||
@@ -85,7 +85,11 @@ class TestFetchIndex:
|
||||
(cache_dir / "metadata.json").write_text(json.dumps(meta))
|
||||
|
||||
fetch_called = []
|
||||
with patch.object(client, "_http_fetch", side_effect=lambda *a, **kw: fetch_called.append(1)):
|
||||
|
||||
def _no_fetch(*a, **kw):
|
||||
fetch_called.append(1)
|
||||
|
||||
with patch.object(client, "_http_fetch", side_effect=_no_fetch):
|
||||
result = client.fetch_index()
|
||||
|
||||
assert not fetch_called, "Should not hit network when cache is fresh"
|
||||
|
||||
@@ -5,11 +5,8 @@ One test per strict check — happy path plus each individual failure mode.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from framework.skills.validator import validate_strict
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user