Simplify skill frontmatter warnings

Keep the malformed SKILL.md warning behavior, but reduce the diagnostic machinery to a once-per-path warning with line numbers only for parser-owned structural errors.

Update the regression to assert the stable warning message without depending on the previous path/line/error dedupe key.
This commit is contained in:
Alessandro 2026-06-25 21:48:16 +02:00
parent 53a12a2ddd
commit bf72345da6
3 changed files with 16 additions and 25 deletions

View file

@ -25,7 +25,7 @@ CONTEXT_DATA_NAME_LOADED_SKILLS = AGENT_DATA_NAME_LOADED_SKILLS
CONTEXT_DATA_NAME_CHAT_ACTIVE_SKILLS = "skills_chat_active"
CONTEXT_DATA_NAME_CHAT_DISABLED_SKILLS = "skills_chat_disabled"
CONTEXT_DATA_NAME_CHAT_VISIBLE_SKILLS = "skills_chat_visible"
_SKILL_PARSE_WARNINGS: set[str] = set()
_WARNED_SKILL_PARSE_PATHS: set[Path] = set()
class ActiveSkillEntry(TypedDict, total=False):
@ -264,9 +264,7 @@ def _emit_skill_scan_warning(message: str) -> None:
print(f"Warning: {message}")
def _frontmatter_error_line(markdown: str, error: str) -> int:
text = markdown or ""
lines = text.splitlines()
def _frontmatter_error_line(lines: List[str], error: str) -> int | None:
if not lines:
return 1
@ -279,30 +277,22 @@ def _frontmatter_error_line(markdown: str, error: str) -> int:
return 1
if error.startswith("Unterminated YAML frontmatter"):
return max(len(lines), 1)
match = re.search(r"line\s+(\d+)", error, flags=re.IGNORECASE)
if match:
start_idx = 0
for index, line in enumerate(lines):
if line.strip() == "---":
start_idx = index
break
return start_idx + int(match.group(1)) + 1
return 1
return None
def _warn_skill_skipped(skill_md_path: Path, markdown: str, errors: List[str]) -> None:
if not errors:
return
error = str(errors[0] or "invalid frontmatter").strip()
line = _frontmatter_error_line(markdown, error)
key = f"{skill_md_path}:{line}:{error}"
if key in _SKILL_PARSE_WARNINGS:
if skill_md_path in _WARNED_SKILL_PARSE_PATHS:
return
_SKILL_PARSE_WARNINGS.add(key)
_WARNED_SKILL_PARSE_PATHS.add(skill_md_path)
error = str(errors[0] or "invalid frontmatter").strip()
line = _frontmatter_error_line((markdown or "").splitlines(), error)
skill_label = skill_md_path.parent.name or str(skill_md_path)
location = f" at line {line}" if line is not None else ""
_emit_skill_scan_warning(
f"skill {skill_label} skipped: invalid frontmatter at line {line}: {error}"
f"skill {skill_label} skipped: invalid frontmatter{location}: {error}"
)

View file

@ -56,7 +56,7 @@
- Helper modules own reusable framework APIs and must preserve public callers unless all callers, tests, and docs are updated together.
- Update this file whenever public functions, classes, persistence behavior, path/security assumptions, side effects, or cross-module contracts change.
- Loaded skill names are chat-wide context data under `CONTEXT_DATA_NAME_LOADED_SKILLS`; legacy agent-local `loaded_skills` lists are migrated into context data and cleared when read.
- Invalid `SKILL.md` frontmatter emits a deduplicated scan warning with the skipped skill path/name and best-effort line number instead of disappearing silently from lists, search, catalog, and load.
- Invalid `SKILL.md` frontmatter emits a once-per-path scan warning with the skipped skill path/name and a line number when the parser can identify one directly.
- Observed side-effect areas: filesystem reads, filesystem deletion, plugin state, settings/state persistence, context data, secret handling.
- Imported dependency areas include: `__future__`, `dataclasses`, `helpers`, `os`, `pathlib`, `re`, `typing`.

View file

@ -341,14 +341,15 @@ def test_invalid_skill_frontmatter_warns_when_skill_is_skipped(monkeypatch, tmp_
)
warnings: list[str] = []
runtime._SKILL_PARSE_WARNINGS.clear()
runtime._WARNED_SKILL_PARSE_PATHS.clear()
monkeypatch.setattr(runtime, "get_skill_roots", lambda agent=None: [str(skills_root)])
monkeypatch.setattr(runtime, "_emit_skill_scan_warning", warnings.append)
assert runtime.list_skills() == []
assert len(warnings) == 1
assert "skill broken-skill skipped: invalid frontmatter at line 4" in warnings[0]
assert "Unterminated YAML frontmatter" in warnings[0]
assert warnings == [
"skill broken-skill skipped: invalid frontmatter at line 4: "
"Unterminated YAML frontmatter"
]
assert runtime.list_skills() == []
assert len(warnings) == 1