Keep skills tooling compatible
Default empty skills_tool calls to list, accept legacy method as a deprecated action alias, and warn when malformed SKILL.md frontmatter causes a skill to be skipped. Update the helper/tool DOX notes and focused regressions.
Alessandro committed
Jun 25, 2026 at 09:41 UTC
884f13dea653bf86e87e6d1cbf9f6faa9d3aa3d8
6 files changed
+141
-24
helpers/skills.py
+53
@@ -25,6 +25,7 @@ CONTEXT_DATA_NAME_LOADED_SKILLS = AGENT_DATA_NAME_LOADED_SKILLS
25
CONTEXT_DATA_NAME_CHAT_ACTIVE_SKILLS = "skills_chat_active"
26
CONTEXT_DATA_NAME_CHAT_DISABLED_SKILLS = "skills_chat_disabled"
27
CONTEXT_DATA_NAME_CHAT_VISIBLE_SKILLS = "skills_chat_visible"
28
+_SKILL_PARSE_WARNINGS: set[str] = set()
29
30
31
class ActiveSkillEntry(TypedDict, total=False):
@@ -254,6 +255,57 @@ def parse_frontmatter(frontmatter_text: str) -> Tuple[Dict[str, Any], List[str]]
255
return parsed, errors
256
257
258
+def _emit_skill_scan_warning(message: str) -> None:
259
+ try:
260
+ from helpers.print_style import PrintStyle
261
+
262
+ PrintStyle.warning(message)
263
+ except Exception:
264
+ print(f"Warning: {message}")
265
+
266
+
267
+def _frontmatter_error_line(markdown: str, error: str) -> int:
268
+ text = markdown or ""
269
+ lines = text.splitlines()
270
+ if not lines:
271
+ return 1
272
+
273
+ if error.startswith("Frontmatter must start"):
274
+ for index, line in enumerate(lines, start=1):
275
+ if line.strip():
276
+ return index
277
+ return 1
278
+ if error.startswith("Missing YAML frontmatter"):
279
+ return 1
280
+ if error.startswith("Unterminated YAML frontmatter"):
281
+ return max(len(lines), 1)
282
+
283
+ match = re.search(r"line\s+(\d+)", error, flags=re.IGNORECASE)
284
+ if match:
285
+ start_idx = 0
286
+ for index, line in enumerate(lines):
287
+ if line.strip() == "---":
288
+ start_idx = index
289
+ break
290
+ return start_idx + int(match.group(1)) + 1
291
+ return 1
292
+
293
+
294
+def _warn_skill_skipped(skill_md_path: Path, markdown: str, errors: List[str]) -> None:
295
+ if not errors:
296
+ return
297
+ error = str(errors[0] or "invalid frontmatter").strip()
298
+ line = _frontmatter_error_line(markdown, error)
299
+ key = f"{skill_md_path}:{line}:{error}"
300
+ if key in _SKILL_PARSE_WARNINGS:
301
+ return
302
+ _SKILL_PARSE_WARNINGS.add(key)
303
+ skill_label = skill_md_path.parent.name or str(skill_md_path)
304
+ _emit_skill_scan_warning(
305
+ f"skill {skill_label} skipped: invalid frontmatter at line {line}: {error}"
306
+ )
307
+
308
+
309
def skill_from_markdown(
310
skill_md_path: Path,
311
*,
@@ -267,6 +319,7 @@ def skill_from_markdown(
319
320
fm, body, fm_errors = split_frontmatter(text)
321
if fm_errors:
322
+ _warn_skill_skipped(skill_md_path, text, fm_errors)
323
return None
324
skill_dir = Path(files.normalize_a0_path(str(skill_md_path.parent)))
325
helpers/skills.py.dox.md
+1
@@ -56,6 +56,7 @@
56
- Helper modules own reusable framework APIs and must preserve public callers unless all callers, tests, and docs are updated together.
57
- Update this file whenever public functions, classes, persistence behavior, path/security assumptions, side effects, or cross-module contracts change.
58
- 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.
59
+- 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.
60
- Observed side-effect areas: filesystem reads, filesystem deletion, plugin state, settings/state persistence, context data, secret handling.
61
- Imported dependency areas include: `__future__`, `dataclasses`, `helpers`, `os`, `pathlib`, `re`, `typing`.
62
tests/test_skills_runtime.py
+23
@@ -331,6 +331,29 @@ def test_invalid_skill_frontmatter_reports_yaml_errors():
331
assert errors[0].startswith("Invalid YAML frontmatter")
332
333
334
+def test_invalid_skill_frontmatter_warns_when_skill_is_skipped(monkeypatch, tmp_path: Path):
335
+ skills_root = tmp_path / "skills"
336
+ broken = skills_root / "broken-skill"
337
+ broken.mkdir(parents=True)
338
+ (broken / "SKILL.md").write_text(
339
+ "---\nname: broken-skill\ndescription: missing closing fence\nBody\n",
340
+ encoding="utf-8",
341
+ )
342
+
343
+ warnings: list[str] = []
344
+ runtime._SKILL_PARSE_WARNINGS.clear()
345
+ monkeypatch.setattr(runtime, "get_skill_roots", lambda agent=None: [str(skills_root)])
346
+ monkeypatch.setattr(runtime, "_emit_skill_scan_warning", warnings.append)
347
+
348
+ assert runtime.list_skills() == []
349
+ assert len(warnings) == 1
350
+ assert "skill broken-skill skipped: invalid frontmatter at line 4" in warnings[0]
351
+ assert "Unterminated YAML frontmatter" in warnings[0]
352
+
353
+ assert runtime.list_skills() == []
354
+ assert len(warnings) == 1
355
+
356
+
357
def test_a0_manage_plugin_skill_frontmatter_is_valid_yaml():
358
text = (PROJECT_ROOT / "skills" / "a0-manage-plugin" / "SKILL.md").read_text(
359
encoding="utf-8"
tests/test_tool_action_contracts.py
+36
@@ -273,6 +273,42 @@ def test_skills_tool_accepts_action_alias_for_search(monkeypatch, tmp_path: Path
273
assert "browser-form-workflows" in response.message
274
275
276
+def test_skills_tool_accepts_method_as_deprecated_action_alias(
277
+ monkeypatch, tmp_path: Path
278
+):
279
+ module = _load_skills_tool(monkeypatch, tmp_path)
280
+ tool = module.SkillsTool(
281
+ _FakeAgent(),
282
+ "skills_tool",
283
+ None,
284
+ {"method": "search", "query": "browser forms"},
285
+ "",
286
+ None,
287
+ )
288
+
289
+ response = asyncio.run(tool.execute(**tool.args))
290
+
291
+ assert "browser-form-workflows" in response.message
292
+ assert tool.args["action"] == "search"
293
+
294
+
295
+def test_skills_tool_defaults_missing_action_to_list(monkeypatch, tmp_path: Path):
296
+ module = _load_skills_tool(monkeypatch, tmp_path)
297
+ tool = module.SkillsTool(
298
+ _FakeAgent(),
299
+ "skills_tool",
300
+ None,
301
+ {},
302
+ "",
303
+ None,
304
+ )
305
+
306
+ response = asyncio.run(tool.execute())
307
+
308
+ assert "Available skills" in response.message
309
+ assert "browser-form-workflows" in response.message
310
+
311
+
312
def test_skills_tool_load_appends_skill_instructions_as_tool_result(
313
monkeypatch, tmp_path: Path
314
):
tools/skills_tool.py
+27
-24
@@ -24,17 +24,26 @@ class SkillsTool(Tool):
24
Script execution is handled by code_execution_tool directly.
25
"""
26
27
- def _current_action(self) -> str:
27
+ @staticmethod
28
+ def _normalize_action(action: object) -> str:
29
return (
30
str(
30
- self.args.get("action")
31
- or ""
31
+ action
32
+ or "list"
33
)
34
.strip()
35
.lower()
36
.replace("-", "_")
37
)
38
39
+ def _current_action(self, **kwargs) -> str:
40
+ return self._normalize_action(
41
+ kwargs.get("action")
42
+ or self.args.get("action")
43
+ or kwargs.get("method")
44
+ or self.args.get("method")
45
+ )
46
+
47
@staticmethod
48
def _normalize_skill_name(skill_name: str) -> str:
49
skill_name = skill_name.strip()
@@ -65,14 +74,14 @@ class SkillsTool(Tool):
74
return super().get_log_object()
75
76
async def before_execution(self, **kwargs):
68
- if self._current_action() != "load":
77
+ if self._current_action(**kwargs) != "load":
78
await super().before_execution(**kwargs)
79
return
80
81
skill_name = self._normalize_skill_name(
82
str(kwargs.get("skill_name") or self.args.get("skill_name") or "")
83
)
75
- label = f"{self.name} action {self._current_action()}"
84
+ label = f"{self.name} action {self._current_action(**kwargs)}"
85
if skill_name:
86
PrintStyle(
87
font_color="#1B4F72",
@@ -90,35 +99,29 @@ class SkillsTool(Tool):
99
self.log = self.get_log_object()
100
101
async def execute(self, **kwargs) -> Response:
93
- action = (
94
- str(
95
- kwargs.get("action")
96
- or self.args.get("action")
97
- or ""
98
- )
99
- .strip()
100
- .lower()
101
- .replace("-", "_")
102
+ action = self._current_action(**kwargs)
103
+
104
+ query = str(kwargs.get("query") or self.args.get("query") or "").strip()
105
+ skill_name = self._normalize_skill_name(
106
+ str(kwargs.get("skill_name") or self.args.get("skill_name") or "")
107
)
108
+ file_path = str(
109
+ kwargs.get("file_path") or self.args.get("file_path") or ""
110
+ ).strip()
111
+
112
+ if "action" not in kwargs and "action" not in self.args and "method" in kwargs:
113
+ kwargs["action"] = action
114
+ if "action" not in self.args and "method" in self.args:
115
+ self.args["action"] = action
116
117
try:
118
if action == "list":
119
return Response(message=self._list(), break_loop=False)
120
if action == "search":
108
- query = str(kwargs.get("query") or self.args.get("query") or "").strip()
121
return Response(message=self._search(query), break_loop=False)
122
if action == "load":
111
- skill_name = self._normalize_skill_name(
112
- str(kwargs.get("skill_name") or self.args.get("skill_name") or "")
113
- )
123
return self._load(skill_name)
124
if action == "read_file":
116
- skill_name = self._normalize_skill_name(
117
- str(kwargs.get("skill_name") or self.args.get("skill_name") or "")
118
- )
119
- file_path = str(
120
- kwargs.get("file_path") or self.args.get("file_path") or ""
121
- ).strip()
125
return Response(
126
message=self._read_file(skill_name, file_path),
127
break_loop=False,
tools/skills_tool.py.dox.md
+1
@@ -29,6 +29,7 @@
29
- Loading a skill appends the full skill body as a normal tool-result history message with `skill_instructions` metadata containing name, path, source, and content visibility.
30
- Loaded skill IDs are stored in chat-wide context data.
31
- Duplicate loads omit the full body when the same skill name remains visible in model history.
32
+- Missing or empty `action` defaults to `list`, and legacy `method` is accepted as a deprecated alias when `action` is absent.
33
- Observed side-effect areas: filesystem reads, filesystem deletion, settings/state persistence, chat history persistence.
34
- Imported dependency areas include: `__future__`, `helpers`, `helpers.print_style`, `helpers.tool`, `pathlib`, `typing`.
35