chnjinlei/hive
1
0
Fork 0
Code Issues Pull Requests Actions 41 Packages Projects Releases Wiki Activity

Merge pull request #6635 from vakrahul/fix/skill-structured-errors-6366

Browse Source 
feat: structured skill error codes and diagnostics (closes #6366)
This commit is contained in:
Timothy @aden
2026-03-20 12:45:35 -07:00
committed by GitHub
parent 936e02e8e6 2844dbf19f
commit 8549fe8238
8 changed files with 386 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/framework/skills/__init__.py
+4
View File
@@ -12,6 +12,7 @@ from framework.skills.discovery import DiscoveryConfig, SkillDiscovery
from framework.skills.manager import SkillsManager, SkillsManagerConfig
from framework.skills.models import TrustStatus
from framework.skills.parser import ParsedSkill, parse_skill_md
from framework.skills.skill_errors import SkillError, SkillErrorCode, log_skill_error
from framework.skills.trust import TrustedRepoStore, TrustGate
__all__ = [
@@ -28,4 +29,7 @@ __all__ = [
"TrustedRepoStore",
"TrustStatus",
"parse_skill_md",
"SkillError",
"SkillErrorCode",
"log_skill_error",
]
core/framework/skills/catalog.py
+9 -1
View File
@@ -10,6 +10,7 @@ import logging
from xml.sax.saxutils import escape
from framework.skills.parser import ParsedSkill
from framework.skills.skill_errors import SkillErrorCode, log_skill_error
logger = logging.getLogger(__name__)
@@ -97,7 +98,14 @@ class SkillCatalog:
for name in skill_names:
skill = self.get(name)
if skill is None:
logger.warning("Pre-activated skill '%s' not found in catalog", name)
log_skill_error(
logger,
"warning",
SkillErrorCode.SKILL_NOT_FOUND,
what=f"Pre-activated skill '{name}' not found in catalog",
why="The skill was listed for pre-activation but was not discovered.",
fix=f"Check that a SKILL.md for '{name}' exists in a scanned directory.",
)
continue
if self.is_activated(name):
continue # Already activated, skip duplicate
core/framework/skills/defaults.py
+53 -4
View File
@@ -11,6 +11,7 @@ from pathlib import Path
from framework.skills.config import SkillsConfig
from framework.skills.parser import ParsedSkill, parse_skill_md
from framework.skills.skill_errors import SkillErrorCode, log_skill_error
logger = logging.getLogger(__name__)
@@ -60,12 +61,14 @@ class DefaultSkillManager:
self._config = config or SkillsConfig()
self._skills: dict[str, ParsedSkill] = {}
self._loaded = False
self._error_count = 0
def load(self) -> None:
"""Load all enabled default skill SKILL.md files."""
if self._loaded:
return
error_count = 0
for skill_name, dir_name in SKILL_REGISTRY.items():
if not self._config.is_default_enabled(skill_name):
logger.info("Default skill '%s' disabled by config", skill_name)
@@ -73,17 +76,34 @@ class DefaultSkillManager:
skill_path = _DEFAULT_SKILLS_DIR / dir_name / "SKILL.md"
if not skill_path.is_file():
logger.error("Default skill SKILL.md not found: %s", skill_path)
log_skill_error(
logger,
"error",
SkillErrorCode.SKILL_NOT_FOUND,
what=f"Default skill SKILL.md not found: '{skill_path}'",
why=f"The framework skill '{skill_name}' is missing its SKILL.md file.",
fix="Reinstall the hive framework — this file is part of the package.",
)
error_count += 1
continue
parsed = parse_skill_md(skill_path, source_scope="framework")
if parsed is None:
logger.error("Failed to parse default skill: %s", skill_path)
log_skill_error(
logger,
"error",
SkillErrorCode.SKILL_PARSE_ERROR,
what=f"Failed to parse default skill '{skill_name}'",
why=f"parse_skill_md returned None for '{skill_path}'.",
fix="Reinstall the hive framework — this file may be corrupted.",
)
error_count += 1
continue
self._skills[skill_name] = parsed
self._loaded = True
self._error_count = error_count
def build_protocols_prompt(self) -> str:
"""Build the combined operational protocols section.
@@ -127,8 +147,23 @@ class DefaultSkillManager:
"""Log which default skills are active and their configuration."""
if not self._skills:
logger.info("Default skills: all disabled")
return
# DX-3: Per-skill structured startup log
for skill_name in SKILL_REGISTRY:
if skill_name in self._skills:
overrides = self._config.get_default_overrides(skill_name)
status = f"loaded overrides={overrides}" if overrides else "loaded"
elif not self._config.is_default_enabled(skill_name):
status = "disabled"
else:
status = "error"
logger.info(
"skill_startup name=%s scope=framework status=%s",
skill_name,
status,
)
# Original active skills log line (preserved for backward compatibility)
active = []
for skill_name in SKILL_REGISTRY:
if skill_name in self._skills:
@@ -138,7 +173,21 @@ class DefaultSkillManager:
else:
active.append(skill_name)
logger.info("Default skills active: %s", ", ".join(active))
if active:
logger.info("Default skills active: %s", ", ".join(active))
# DX-3: Summary line with error count
total = len(SKILL_REGISTRY)
active_count = len(self._skills)
error_count = getattr(self, "_error_count", 0)
disabled_count = total - active_count - error_count
logger.info(
"Skills: %d default (%d active, %d disabled, %d error)",
total,
active_count,
disabled_count,
error_count,
)
@property
def active_skill_names(self) -> list[str]:
core/framework/skills/discovery.py
+8 -5
View File
@@ -11,6 +11,7 @@ from dataclasses import dataclass
from pathlib import Path
from framework.skills.parser import ParsedSkill, parse_skill_md
from framework.skills.skill_errors import SkillErrorCode, log_skill_error
logger = logging.getLogger(__name__)
@@ -172,11 +173,13 @@ class SkillDiscovery:
for skill in skills:
if skill.name in seen:
existing = seen[skill.name]
logger.warning(
"Skill name collision: '%s' from %s overrides %s",
skill.name,
skill.location,
existing.location,
log_skill_error(
logger,
"warning",
SkillErrorCode.SKILL_COLLISION,
what=f"Skill name collision: '{skill.name}'",
why=f"'{skill.location}' overrides '{existing.location}'.",
fix="Rename one of the conflicting skill directories to use a unique name.",
)
seen[skill.name] = skill
core/framework/skills/manager.py
+10
View File
@@ -146,6 +146,16 @@ class SkillsManager:
default_mgr.load()
default_mgr.log_active_skills()
protocols_prompt = default_mgr.build_protocols_prompt()
# DX-3: Community skill startup summary
if self._config.project_root is not None and not self._config.skip_community_discovery:
community_count = len(catalog._skills) if catalog_prompt else 0
pre_activated_count = len(skills_config.skills) if skills_config.skills else 0
logger.info(
"Skills: %d community (%d catalog, %d pre-activated)",
community_count,
community_count,
pre_activated_count,
)
# 3. Cache
self._catalog_prompt = catalog_prompt
core/framework/skills/parser.py
+81 -14
View File
@@ -13,6 +13,8 @@ from dataclasses import dataclass
from pathlib import Path
from typing import Any
from framework.skills.skill_errors import SkillErrorCode, log_skill_error
logger = logging.getLogger(__name__)
# Maximum name length before a warning is logged
@@ -74,17 +76,38 @@ def parse_skill_md(path: Path, source_scope: str = "project") -> ParsedSkill | N
try:
content = path.read_text(encoding="utf-8")
except OSError as exc:
logger.error("Failed to read %s: %s", path, exc)
log_skill_error(
logger,
"error",
SkillErrorCode.SKILL_ACTIVATION_FAILED,
what=f"Failed to read '{path}'",
why=str(exc),
fix="Check the file exists and has read permissions.",
)
return None
if not content.strip():
logger.error("Empty SKILL.md: %s", path)
log_skill_error(
logger,
"error",
SkillErrorCode.SKILL_PARSE_ERROR,
what=f"Invalid SKILL.md at '{path}'",
why="The file exists but contains no content.",
fix="Add valid YAML frontmatter and a markdown body to the SKILL.md.",
)
return None
# Split on --- delimiters (first two occurrences)
parts = content.split("---", 2)
if len(parts) < 3:
logger.error("SKILL.md missing YAML frontmatter delimiters (---): %s", path)
log_skill_error(
logger,
"error",
SkillErrorCode.SKILL_PARSE_ERROR,
what=f"Invalid SKILL.md at '{path}'",
why="Missing YAML frontmatter (---).",
fix="Wrap the frontmatter with --- on its own line at the top and bottom.",
)
return None
# parts[0] is content before first --- (should be empty or whitespace)
@@ -94,7 +117,14 @@ def parse_skill_md(path: Path, source_scope: str = "project") -> ParsedSkill | N
body = parts[2].strip()
if not raw_yaml:
logger.error("Empty YAML frontmatter in %s", path)
log_skill_error(
logger,
"error",
SkillErrorCode.SKILL_PARSE_ERROR,
what=f"Invalid SKILL.md at '{path}'",
why="The --- delimiters are present but the YAML block is empty.",
fix="Add at least 'name' and 'description' fields to the frontmatter.",
)
return None
# Parse YAML
@@ -108,19 +138,47 @@ def parse_skill_md(path: Path, source_scope: str = "project") -> ParsedSkill | N
try:
fixed = _try_fix_yaml(raw_yaml)
frontmatter = yaml.safe_load(fixed)
logger.warning("Fixed YAML parse issues in %s (unquoted colons)", path)
log_skill_error(
logger,
"warning",
SkillErrorCode.SKILL_YAML_FIXUP,
what=f"Auto-fixed YAML in '{path}'",
why="Unquoted colon values detected in frontmatter.",
fix='Wrap values containing colons in quotes e.g. description: "Use for: research"',
)
except yaml.YAMLError as exc:
logger.error("Unparseable YAML in %s: %s", path, exc)
log_skill_error(
logger,
"error",
SkillErrorCode.SKILL_PARSE_ERROR,
what=f"Invalid SKILL.md at '{path}'",
why=str(exc),
fix="Validate the YAML frontmatter at https://yaml-online-parser.appspot.com/",
)
return None
if not isinstance(frontmatter, dict):
logger.error("YAML frontmatter is not a mapping in %s", path)
log_skill_error(
logger,
"error",
SkillErrorCode.SKILL_PARSE_ERROR,
what=f"Invalid SKILL.md at '{path}'",
why="YAML frontmatter is not a key-value mapping.",
fix="Ensure the frontmatter is valid YAML with key: value pairs.",
)
return None
# Required: description
description = frontmatter.get("description")
if not description or not str(description).strip():
logger.error("Missing or empty 'description' in %s — skipping skill", path)
log_skill_error(
logger,
"error",
SkillErrorCode.SKILL_MISSING_DESCRIPTION,
what=f"Missing 'description' in '{path}'",
why="The 'description' field is required but is absent or empty.",
fix="Add a non-empty 'description' field to the YAML frontmatter.",
)
return None
# Required: name (fallback to parent directory name)
@@ -128,7 +186,14 @@ def parse_skill_md(path: Path, source_scope: str = "project") -> ParsedSkill | N
parent_dir_name = path.parent.name
if not name or not str(name).strip():
name = parent_dir_name
logger.warning("Missing 'name' in %s — using directory name '%s'", path, name)
log_skill_error(
logger,
"warning",
SkillErrorCode.SKILL_NAME_MISMATCH,
what=f"Missing 'name' in '{path}' — using directory name '{name}'",
why="The 'name' field is absent from the YAML frontmatter.",
fix=f"Add 'name: {name}' to the frontmatter to make this explicit.",
)
else:
name = str(name).strip()
@@ -137,11 +202,13 @@ def parse_skill_md(path: Path, source_scope: str = "project") -> ParsedSkill | N
logger.warning("Skill name exceeds %d chars in %s: '%s'", _MAX_NAME_LENGTH, path, name)
if name != parent_dir_name and not name.endswith(f".{parent_dir_name}"):
logger.warning(
"Skill name '%s' doesn't match parent directory '%s' in %s",
name,
parent_dir_name,
path,
log_skill_error(
logger,
"warning",
SkillErrorCode.SKILL_NAME_MISMATCH,
what=f"Name mismatch in '{path}'",
why=f"Skill name '{name}' doesn't match directory '{parent_dir_name}'.",
fix=f"Rename the directory to '{name}' or set name to '{parent_dir_name}'.",
)
return ParsedSkill(
core/framework/skills/skill_errors.py
+70
View File
@@ -0,0 +1,70 @@
"""Structured error codes and diagnostics for the Hive skill system.
Implements DX-1 (structured error codes) and DX-2 (what/why/fix format)
from the skill system PRD §7.5.
"""
from __future__ import annotations
import logging
from enum import Enum
class SkillErrorCode(Enum):
"""Standardized error codes for skill system operations (DX-1)."""
SKILL_NOT_FOUND = "SKILL_NOT_FOUND"
SKILL_PARSE_ERROR = "SKILL_PARSE_ERROR"
SKILL_ACTIVATION_FAILED = "SKILL_ACTIVATION_FAILED"
SKILL_MISSING_DESCRIPTION = "SKILL_MISSING_DESCRIPTION"
SKILL_YAML_FIXUP = "SKILL_YAML_FIXUP"
SKILL_NAME_MISMATCH = "SKILL_NAME_MISMATCH"
SKILL_COLLISION = "SKILL_COLLISION"
class SkillError(Exception):
"""Structured exception for skill system errors (DX-2).
Raised in strict validation paths. Also used as the base
format contract for log_skill_error() log messages.
"""
def __init__(self, code: SkillErrorCode, what: str, why: str, fix: str):
self.code = code
self.what = what
self.why = why
self.fix = fix
self.message = (
f"[{self.code.value}]\nWhat failed: {self.what}\nWhy: {self.why}\nFix: {self.fix}"
)
super().__init__(self.message)
def log_skill_error(
logger: logging.Logger,
level: str,
code: SkillErrorCode,
what: str,
why: str,
fix: str,
) -> None:
"""Emit a structured skill diagnostic log with consistent format (DX-2).
Args:
logger: The module logger to emit to.
level: Log level string — 'error', 'warning', or 'info'.
code: Structured error code.
what: What failed (specific skill name and path).
why: Root cause.
fix: Concrete next step for the developer.
"""
msg = f"[{code.value}] What failed: {what} | Why: {why} | Fix: {fix}"
getattr(logger, level)(
msg,
extra={
"skill_error_code": code.value,
"what": what,
"why": why,
"fix": fix,
},
)
core/tests/test_skill_errors.py
+151
View File
@@ -0,0 +1,151 @@
"""Tests for skill system structured error codes and diagnostics."""
from __future__ import annotations
import logging
from framework.skills.skill_errors import (
SkillError,
SkillErrorCode,
log_skill_error,
)
class TestSkillErrorCode:
def test_all_codes_defined(self):
codes = {e.value for e in SkillErrorCode}
assert "SKILL_NOT_FOUND" in codes
assert "SKILL_PARSE_ERROR" in codes
assert "SKILL_ACTIVATION_FAILED" in codes
assert "SKILL_MISSING_DESCRIPTION" in codes
assert "SKILL_YAML_FIXUP" in codes
assert "SKILL_NAME_MISMATCH" in codes
assert "SKILL_COLLISION" in codes
class TestSkillError:
def test_code_stored(self):
err = SkillError(
code=SkillErrorCode.SKILL_NOT_FOUND,
what="Skill 'my-skill' not found",
why="Not in catalog",
fix="Check discovery paths",
)
assert err.code == SkillErrorCode.SKILL_NOT_FOUND
def test_message_format(self):
err = SkillError(
code=SkillErrorCode.SKILL_MISSING_DESCRIPTION,
what="Missing description in '/path/SKILL.md'",
why="The description field is absent",
fix="Add a description field to the frontmatter",
)
expected = (
"[SKILL_MISSING_DESCRIPTION]\n"
"What failed: Missing description in '/path/SKILL.md'\n"
"Why: The description field is absent\n"
"Fix: Add a description field to the frontmatter"
)
assert str(err) == expected
def test_is_exception(self):
err = SkillError(
code=SkillErrorCode.SKILL_PARSE_ERROR,
what="Parse failed",
why="Invalid YAML",
fix="Fix the YAML",
)
assert isinstance(err, Exception)
def test_what_why_fix_attributes(self):
err = SkillError(
code=SkillErrorCode.SKILL_COLLISION,
what="Name collision",
why="Two skills share the same name",
fix="Rename one skill directory",
)
assert err.what == "Name collision"
assert err.why == "Two skills share the same name"
assert err.fix == "Rename one skill directory"
class TestLogSkillError:
def test_emits_log(self, caplog):
test_logger = logging.getLogger("test_skill")
with caplog.at_level(logging.ERROR, logger="test_skill"):
log_skill_error(
test_logger,
"error",
SkillErrorCode.SKILL_PARSE_ERROR,
what="Invalid SKILL.md at '/path'",
why="Empty file",
fix="Add content",
)
assert "SKILL_PARSE_ERROR" in caplog.text
def test_warning_level(self, caplog):
test_logger = logging.getLogger("test_skill_warn")
with caplog.at_level(logging.WARNING, logger="test_skill_warn"):
log_skill_error(
test_logger,
"warning",
SkillErrorCode.SKILL_YAML_FIXUP,
what="Auto-fixed YAML",
why="Unquoted colons",
fix="Quote values",
)
assert "SKILL_YAML_FIXUP" in caplog.text
def test_message_contains_all_parts(self, caplog):
test_logger = logging.getLogger("test_skill_parts")
with caplog.at_level(logging.ERROR, logger="test_skill_parts"):
log_skill_error(
test_logger,
"error",
SkillErrorCode.SKILL_NOT_FOUND,
what="Skill not found",
why="Not discovered",
fix="Check paths",
)
assert "Skill not found" in caplog.text
assert "Not discovered" in caplog.text
assert "Check paths" in caplog.text
class TestSkillErrorInParser:
def test_missing_description_returns_none(self, tmp_path):
from framework.skills.parser import parse_skill_md
skill_dir = tmp_path / "no-desc"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text("---\nname: no-desc\n---\nBody.\n", encoding="utf-8")
result = parse_skill_md(skill_dir / "SKILL.md")
assert result is None
def test_empty_file_returns_none(self, tmp_path):
from framework.skills.parser import parse_skill_md
skill_dir = tmp_path / "empty"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text("", encoding="utf-8")
result = parse_skill_md(skill_dir / "SKILL.md")
assert result is None
def test_nonexistent_returns_none(self, tmp_path):
from framework.skills.parser import parse_skill_md
result = parse_skill_md(tmp_path / "ghost" / "SKILL.md")
assert result is None
def test_yaml_fixup_still_parses(self, tmp_path):
from framework.skills.parser import parse_skill_md
skill_dir = tmp_path / "colon-test"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"---\nname: colon-test\ndescription: Use for: research\n---\nBody.\n",
encoding="utf-8",
)
result = parse_skill_md(skill_dir / "SKILL.md")
assert result is not None
assert "research" in result.description