fix(skill-creator): restore validation and align packaging docs
This commit is contained in:
@@ -268,6 +268,8 @@ Skip this step only if the skill being developed already exists, and iteration o
|
||||
|
||||
When creating a new skill from scratch, always run the `init_skill.py` script. The script conveniently generates a new template skill directory that automatically includes everything a skill requires, making the skill creation process much more efficient and reliable.
|
||||
|
||||
For `nanobot`, custom skills should live under the active workspace `skills/` directory so they can be discovered automatically at runtime (for example, `<workspace>/skills/my-skill/SKILL.md`).
|
||||
|
||||
Usage:
|
||||
|
||||
```bash
|
||||
@@ -277,9 +279,9 @@ scripts/init_skill.py <skill-name> --path <output-directory> [--resources script
|
||||
Examples:
|
||||
|
||||
```bash
|
||||
scripts/init_skill.py my-skill --path skills/public
|
||||
scripts/init_skill.py my-skill --path skills/public --resources scripts,references
|
||||
scripts/init_skill.py my-skill --path skills/public --resources scripts --examples
|
||||
scripts/init_skill.py my-skill --path ./workspace/skills
|
||||
scripts/init_skill.py my-skill --path ./workspace/skills --resources scripts,references
|
||||
scripts/init_skill.py my-skill --path ./workspace/skills --resources scripts --examples
|
||||
```
|
||||
|
||||
The script:
|
||||
@@ -326,7 +328,7 @@ Write the YAML frontmatter with `name` and `description`:
|
||||
- Include all "when to use" information here - Not in the body. The body is only loaded after triggering, so "When to Use This Skill" sections in the body are not helpful to the agent.
|
||||
- Example description for a `docx` skill: "Comprehensive document creation, editing, and analysis with support for tracked changes, comments, formatting preservation, and text extraction. Use when the agent needs to work with professional documents (.docx files) for: (1) Creating new documents, (2) Modifying or editing content, (3) Working with tracked changes, (4) Adding comments, or any other document tasks"
|
||||
|
||||
Do not include any other fields in YAML frontmatter.
|
||||
Keep frontmatter minimal. In `nanobot`, `metadata` and `always` are also supported when needed, but avoid adding extra fields unless they are actually required.
|
||||
|
||||
##### Body
|
||||
|
||||
|
||||
@@ -3,11 +3,11 @@
|
||||
Skill Packager - Creates a distributable .skill file of a skill folder
|
||||
|
||||
Usage:
|
||||
python utils/package_skill.py <path/to/skill-folder> [output-directory]
|
||||
python package_skill.py <path/to/skill-folder> [output-directory]
|
||||
|
||||
Example:
|
||||
python utils/package_skill.py skills/public/my-skill
|
||||
python utils/package_skill.py skills/public/my-skill ./dist
|
||||
python package_skill.py skills/public/my-skill
|
||||
python package_skill.py skills/public/my-skill ./dist
|
||||
"""
|
||||
|
||||
import sys
|
||||
@@ -25,6 +25,14 @@ def _is_within(path: Path, root: Path) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def _cleanup_partial_archive(skill_filename: Path) -> None:
|
||||
try:
|
||||
if skill_filename.exists():
|
||||
skill_filename.unlink()
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
|
||||
def package_skill(skill_path, output_dir=None):
|
||||
"""
|
||||
Package a skill folder into a .skill file.
|
||||
@@ -74,49 +82,56 @@ def package_skill(skill_path, output_dir=None):
|
||||
|
||||
EXCLUDED_DIRS = {".git", ".svn", ".hg", "__pycache__", "node_modules"}
|
||||
|
||||
files_to_package = []
|
||||
resolved_archive = skill_filename.resolve()
|
||||
|
||||
for file_path in skill_path.rglob("*"):
|
||||
# Fail closed on symlinks so the packaged contents are explicit and predictable.
|
||||
if file_path.is_symlink():
|
||||
print(f"[ERROR] Symlink not allowed in packaged skill: {file_path}")
|
||||
_cleanup_partial_archive(skill_filename)
|
||||
return None
|
||||
|
||||
rel_parts = file_path.relative_to(skill_path).parts
|
||||
if any(part in EXCLUDED_DIRS for part in rel_parts):
|
||||
continue
|
||||
|
||||
if file_path.is_file():
|
||||
resolved_file = file_path.resolve()
|
||||
if not _is_within(resolved_file, skill_path):
|
||||
print(f"[ERROR] File escapes skill root: {file_path}")
|
||||
_cleanup_partial_archive(skill_filename)
|
||||
return None
|
||||
# If output lives under skill_path, avoid writing archive into itself.
|
||||
if resolved_file == resolved_archive:
|
||||
print(f"[WARN] Skipping output archive: {file_path}")
|
||||
continue
|
||||
files_to_package.append(file_path)
|
||||
|
||||
# Create the .skill file (zip format)
|
||||
try:
|
||||
with zipfile.ZipFile(skill_filename, "w", zipfile.ZIP_DEFLATED) as zipf:
|
||||
# Walk through the skill directory
|
||||
for file_path in skill_path.rglob("*"):
|
||||
# Security: never follow or package symlinks.
|
||||
if file_path.is_symlink():
|
||||
print(f"[WARN] Skipping symlink: {file_path}")
|
||||
continue
|
||||
|
||||
rel_parts = file_path.relative_to(skill_path).parts
|
||||
if any(part in EXCLUDED_DIRS for part in rel_parts):
|
||||
continue
|
||||
|
||||
if file_path.is_file():
|
||||
resolved_file = file_path.resolve()
|
||||
if not _is_within(resolved_file, skill_path):
|
||||
print(f"[ERROR] File escapes skill root: {file_path}")
|
||||
return None
|
||||
# If output lives under skill_path, avoid writing archive into itself.
|
||||
if resolved_file == skill_filename.resolve():
|
||||
print(f"[WARN] Skipping output archive: {file_path}")
|
||||
continue
|
||||
|
||||
# Calculate the relative path within the zip.
|
||||
arcname = Path(skill_name) / file_path.relative_to(skill_path)
|
||||
zipf.write(file_path, arcname)
|
||||
print(f" Added: {arcname}")
|
||||
for file_path in files_to_package:
|
||||
# Calculate the relative path within the zip.
|
||||
arcname = Path(skill_name) / file_path.relative_to(skill_path)
|
||||
zipf.write(file_path, arcname)
|
||||
print(f" Added: {arcname}")
|
||||
|
||||
print(f"\n[OK] Successfully packaged skill to: {skill_filename}")
|
||||
return skill_filename
|
||||
|
||||
except Exception as e:
|
||||
_cleanup_partial_archive(skill_filename)
|
||||
print(f"[ERROR] Error creating .skill file: {e}")
|
||||
return None
|
||||
|
||||
|
||||
def main():
|
||||
if len(sys.argv) < 2:
|
||||
print("Usage: python utils/package_skill.py <path/to/skill-folder> [output-directory]")
|
||||
print("Usage: python package_skill.py <path/to/skill-folder> [output-directory]")
|
||||
print("\nExample:")
|
||||
print(" python utils/package_skill.py skills/public/my-skill")
|
||||
print(" python utils/package_skill.py skills/public/my-skill ./dist")
|
||||
print(" python package_skill.py skills/public/my-skill")
|
||||
print(" python package_skill.py skills/public/my-skill ./dist")
|
||||
sys.exit(1)
|
||||
|
||||
skill_path = sys.argv[1]
|
||||
|
||||
213
nanobot/skills/skill-creator/scripts/quick_validate.py
Normal file
213
nanobot/skills/skill-creator/scripts/quick_validate.py
Normal file
@@ -0,0 +1,213 @@
|
||||
#!/usr/bin/env python3
|
||||
"""
|
||||
Minimal validator for nanobot skill folders.
|
||||
"""
|
||||
|
||||
import re
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Optional
|
||||
|
||||
try:
|
||||
import yaml
|
||||
except ModuleNotFoundError:
|
||||
yaml = None
|
||||
|
||||
MAX_SKILL_NAME_LENGTH = 64
|
||||
ALLOWED_FRONTMATTER_KEYS = {
|
||||
"name",
|
||||
"description",
|
||||
"metadata",
|
||||
"always",
|
||||
"license",
|
||||
"allowed-tools",
|
||||
}
|
||||
ALLOWED_RESOURCE_DIRS = {"scripts", "references", "assets"}
|
||||
PLACEHOLDER_MARKERS = ("[todo", "todo:")
|
||||
|
||||
|
||||
def _extract_frontmatter(content: str) -> Optional[str]:
|
||||
lines = content.splitlines()
|
||||
if not lines or lines[0].strip() != "---":
|
||||
return None
|
||||
for i in range(1, len(lines)):
|
||||
if lines[i].strip() == "---":
|
||||
return "\n".join(lines[1:i])
|
||||
return None
|
||||
|
||||
|
||||
def _parse_simple_frontmatter(frontmatter_text: str) -> Optional[dict[str, str]]:
|
||||
"""Fallback parser for simple frontmatter when PyYAML is unavailable."""
|
||||
parsed: dict[str, str] = {}
|
||||
current_key: Optional[str] = None
|
||||
multiline_key: Optional[str] = None
|
||||
|
||||
for raw_line in frontmatter_text.splitlines():
|
||||
stripped = raw_line.strip()
|
||||
if not stripped or stripped.startswith("#"):
|
||||
continue
|
||||
|
||||
is_indented = raw_line[:1].isspace()
|
||||
if is_indented:
|
||||
if current_key is None:
|
||||
return None
|
||||
current_value = parsed[current_key]
|
||||
parsed[current_key] = f"{current_value}\n{stripped}" if current_value else stripped
|
||||
continue
|
||||
|
||||
if ":" not in stripped:
|
||||
return None
|
||||
|
||||
key, value = stripped.split(":", 1)
|
||||
key = key.strip()
|
||||
value = value.strip()
|
||||
if not key:
|
||||
return None
|
||||
|
||||
if value in {"|", ">"}:
|
||||
parsed[key] = ""
|
||||
current_key = key
|
||||
multiline_key = key
|
||||
continue
|
||||
|
||||
if (value.startswith('"') and value.endswith('"')) or (
|
||||
value.startswith("'") and value.endswith("'")
|
||||
):
|
||||
value = value[1:-1]
|
||||
parsed[key] = value
|
||||
current_key = key
|
||||
multiline_key = None
|
||||
|
||||
if multiline_key is not None and multiline_key not in parsed:
|
||||
return None
|
||||
return parsed
|
||||
|
||||
|
||||
def _load_frontmatter(frontmatter_text: str) -> tuple[Optional[dict], Optional[str]]:
|
||||
if yaml is not None:
|
||||
try:
|
||||
frontmatter = yaml.safe_load(frontmatter_text)
|
||||
except yaml.YAMLError as exc:
|
||||
return None, f"Invalid YAML in frontmatter: {exc}"
|
||||
if not isinstance(frontmatter, dict):
|
||||
return None, "Frontmatter must be a YAML dictionary"
|
||||
return frontmatter, None
|
||||
|
||||
frontmatter = _parse_simple_frontmatter(frontmatter_text)
|
||||
if frontmatter is None:
|
||||
return None, "Invalid YAML in frontmatter: unsupported syntax without PyYAML installed"
|
||||
return frontmatter, None
|
||||
|
||||
|
||||
def _validate_skill_name(name: str, folder_name: str) -> Optional[str]:
|
||||
if not re.fullmatch(r"[a-z0-9]+(?:-[a-z0-9]+)*", name):
|
||||
return (
|
||||
f"Name '{name}' should be hyphen-case "
|
||||
"(lowercase letters, digits, and single hyphens only)"
|
||||
)
|
||||
if len(name) > MAX_SKILL_NAME_LENGTH:
|
||||
return (
|
||||
f"Name is too long ({len(name)} characters). "
|
||||
f"Maximum is {MAX_SKILL_NAME_LENGTH} characters."
|
||||
)
|
||||
if name != folder_name:
|
||||
return f"Skill name '{name}' must match directory name '{folder_name}'"
|
||||
return None
|
||||
|
||||
|
||||
def _validate_description(description: str) -> Optional[str]:
|
||||
trimmed = description.strip()
|
||||
if not trimmed:
|
||||
return "Description cannot be empty"
|
||||
lowered = trimmed.lower()
|
||||
if any(marker in lowered for marker in PLACEHOLDER_MARKERS):
|
||||
return "Description still contains TODO placeholder text"
|
||||
if "<" in trimmed or ">" in trimmed:
|
||||
return "Description cannot contain angle brackets (< or >)"
|
||||
if len(trimmed) > 1024:
|
||||
return f"Description is too long ({len(trimmed)} characters). Maximum is 1024 characters."
|
||||
return None
|
||||
|
||||
|
||||
def validate_skill(skill_path):
|
||||
"""Validate a skill folder structure and required frontmatter."""
|
||||
skill_path = Path(skill_path).resolve()
|
||||
|
||||
if not skill_path.exists():
|
||||
return False, f"Skill folder not found: {skill_path}"
|
||||
if not skill_path.is_dir():
|
||||
return False, f"Path is not a directory: {skill_path}"
|
||||
|
||||
skill_md = skill_path / "SKILL.md"
|
||||
if not skill_md.exists():
|
||||
return False, "SKILL.md not found"
|
||||
|
||||
try:
|
||||
content = skill_md.read_text(encoding="utf-8")
|
||||
except OSError as exc:
|
||||
return False, f"Could not read SKILL.md: {exc}"
|
||||
|
||||
frontmatter_text = _extract_frontmatter(content)
|
||||
if frontmatter_text is None:
|
||||
return False, "Invalid frontmatter format"
|
||||
|
||||
frontmatter, error = _load_frontmatter(frontmatter_text)
|
||||
if error:
|
||||
return False, error
|
||||
|
||||
unexpected_keys = sorted(set(frontmatter.keys()) - ALLOWED_FRONTMATTER_KEYS)
|
||||
if unexpected_keys:
|
||||
allowed = ", ".join(sorted(ALLOWED_FRONTMATTER_KEYS))
|
||||
unexpected = ", ".join(unexpected_keys)
|
||||
return (
|
||||
False,
|
||||
f"Unexpected key(s) in SKILL.md frontmatter: {unexpected}. Allowed properties are: {allowed}",
|
||||
)
|
||||
|
||||
if "name" not in frontmatter:
|
||||
return False, "Missing 'name' in frontmatter"
|
||||
if "description" not in frontmatter:
|
||||
return False, "Missing 'description' in frontmatter"
|
||||
|
||||
name = frontmatter["name"]
|
||||
if not isinstance(name, str):
|
||||
return False, f"Name must be a string, got {type(name).__name__}"
|
||||
name_error = _validate_skill_name(name.strip(), skill_path.name)
|
||||
if name_error:
|
||||
return False, name_error
|
||||
|
||||
description = frontmatter["description"]
|
||||
if not isinstance(description, str):
|
||||
return False, f"Description must be a string, got {type(description).__name__}"
|
||||
description_error = _validate_description(description)
|
||||
if description_error:
|
||||
return False, description_error
|
||||
|
||||
always = frontmatter.get("always")
|
||||
if always is not None and not isinstance(always, bool):
|
||||
return False, f"'always' must be a boolean, got {type(always).__name__}"
|
||||
|
||||
for child in skill_path.iterdir():
|
||||
if child.name == "SKILL.md":
|
||||
continue
|
||||
if child.is_dir() and child.name in ALLOWED_RESOURCE_DIRS:
|
||||
continue
|
||||
if child.is_symlink():
|
||||
continue
|
||||
return (
|
||||
False,
|
||||
f"Unexpected file or directory in skill root: {child.name}. "
|
||||
"Only SKILL.md, scripts/, references/, and assets/ are allowed.",
|
||||
)
|
||||
|
||||
return True, "Skill is valid!"
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
if len(sys.argv) != 2:
|
||||
print("Usage: python quick_validate.py <skill_directory>")
|
||||
sys.exit(1)
|
||||
|
||||
valid, message = validate_skill(sys.argv[1])
|
||||
print(message)
|
||||
sys.exit(0 if valid else 1)
|
||||
127
tests/test_skill_creator_scripts.py
Normal file
127
tests/test_skill_creator_scripts.py
Normal file
@@ -0,0 +1,127 @@
|
||||
import importlib
|
||||
import shutil
|
||||
import sys
|
||||
import zipfile
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
SCRIPT_DIR = Path("nanobot/skills/skill-creator/scripts").resolve()
|
||||
if str(SCRIPT_DIR) not in sys.path:
|
||||
sys.path.insert(0, str(SCRIPT_DIR))
|
||||
|
||||
init_skill = importlib.import_module("init_skill")
|
||||
package_skill = importlib.import_module("package_skill")
|
||||
quick_validate = importlib.import_module("quick_validate")
|
||||
|
||||
|
||||
def test_init_skill_creates_expected_files(tmp_path: Path) -> None:
|
||||
skill_dir = init_skill.init_skill(
|
||||
"demo-skill",
|
||||
tmp_path,
|
||||
["scripts", "references", "assets"],
|
||||
include_examples=True,
|
||||
)
|
||||
|
||||
assert skill_dir == tmp_path / "demo-skill"
|
||||
assert (skill_dir / "SKILL.md").exists()
|
||||
assert (skill_dir / "scripts" / "example.py").exists()
|
||||
assert (skill_dir / "references" / "api_reference.md").exists()
|
||||
assert (skill_dir / "assets" / "example_asset.txt").exists()
|
||||
|
||||
|
||||
def test_validate_skill_accepts_existing_skill_creator() -> None:
|
||||
valid, message = quick_validate.validate_skill(
|
||||
Path("nanobot/skills/skill-creator").resolve()
|
||||
)
|
||||
|
||||
assert valid, message
|
||||
|
||||
|
||||
def test_validate_skill_rejects_placeholder_description(tmp_path: Path) -> None:
|
||||
skill_dir = tmp_path / "placeholder-skill"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\n"
|
||||
"name: placeholder-skill\n"
|
||||
'description: "[TODO: fill me in]"\n'
|
||||
"---\n"
|
||||
"# Placeholder\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
valid, message = quick_validate.validate_skill(skill_dir)
|
||||
|
||||
assert not valid
|
||||
assert "TODO placeholder" in message
|
||||
|
||||
|
||||
def test_validate_skill_rejects_root_files_outside_allowed_dirs(tmp_path: Path) -> None:
|
||||
skill_dir = tmp_path / "bad-root-skill"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\n"
|
||||
"name: bad-root-skill\n"
|
||||
"description: Valid description\n"
|
||||
"---\n"
|
||||
"# Skill\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
(skill_dir / "README.md").write_text("extra\n", encoding="utf-8")
|
||||
|
||||
valid, message = quick_validate.validate_skill(skill_dir)
|
||||
|
||||
assert not valid
|
||||
assert "Unexpected file or directory in skill root" in message
|
||||
|
||||
|
||||
def test_package_skill_creates_archive(tmp_path: Path) -> None:
|
||||
skill_dir = tmp_path / "package-me"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\n"
|
||||
"name: package-me\n"
|
||||
"description: Package this skill.\n"
|
||||
"---\n"
|
||||
"# Skill\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
scripts_dir = skill_dir / "scripts"
|
||||
scripts_dir.mkdir()
|
||||
(scripts_dir / "helper.py").write_text("print('ok')\n", encoding="utf-8")
|
||||
|
||||
archive_path = package_skill.package_skill(skill_dir, tmp_path / "dist")
|
||||
|
||||
assert archive_path == (tmp_path / "dist" / "package-me.skill")
|
||||
assert archive_path.exists()
|
||||
with zipfile.ZipFile(archive_path, "r") as archive:
|
||||
names = set(archive.namelist())
|
||||
assert "package-me/SKILL.md" in names
|
||||
assert "package-me/scripts/helper.py" in names
|
||||
|
||||
|
||||
def test_package_skill_rejects_symlink(tmp_path: Path) -> None:
|
||||
skill_dir = tmp_path / "symlink-skill"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"---\n"
|
||||
"name: symlink-skill\n"
|
||||
"description: Reject symlinks during packaging.\n"
|
||||
"---\n"
|
||||
"# Skill\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
scripts_dir = skill_dir / "scripts"
|
||||
scripts_dir.mkdir()
|
||||
target = tmp_path / "outside.txt"
|
||||
target.write_text("secret\n", encoding="utf-8")
|
||||
link = scripts_dir / "outside.txt"
|
||||
|
||||
try:
|
||||
link.symlink_to(target)
|
||||
except (OSError, NotImplementedError):
|
||||
return
|
||||
|
||||
archive_path = package_skill.package_skill(skill_dir, tmp_path / "dist")
|
||||
|
||||
assert archive_path is None
|
||||
assert not (tmp_path / "dist" / "symlink-skill.skill").exists()
|
||||
Reference in New Issue
Block a user