mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 15:59:30 +00:00
fix: prevent skill-creator from bypassing workshop proposals (#98346)
Summary: - The branch adds bundled skill-creator guidance to route durable OpenClaw skill work through Skill Workshop proposals and removes the direct `init_skill.py` scaffold helper plus its test. - PR surface: Docs +6, Other -429. Total -423 across 3 files. - Reproducibility: yes. source-level: current main has `init_skill.py` writing a live `SKILL.md` directly, whi ... `. I did not run the direct helper as a repro because this review was read-only and that path writes files. Automerge notes: - No ClawSweeper repair was needed after automerge opt-in. Validation: - ClawSweeper review passed for head 8eca1654472e50283ba7deed5f3220cc8213be94. - Required merge gates passed before the squash merge. Prepared head SHA: 8eca1654472e50283ba7deed5f3220cc8213be94 Review: https://github.com/openclaw/openclaw/pull/98346#issuecomment-4849855806 Co-authored-by: momothemage <niuzhengnan@163.com> Approved-by: momothemage
This commit is contained in:
Momo
GitHub
parent
fa3c9de459
commit
8c5d1dca78
3 changed files with 11 additions and 434 deletions
|
|
@ -9,6 +9,9 @@ Skills are compact triggerable workflows. Metadata is always visible; body loads
|
|||
|
||||
## Hard rules
|
||||
|
||||
- For durable OpenClaw skill creation or updates in an agent session, use
|
||||
`skill_workshop` to create or revise a pending proposal. Do not scaffold or
|
||||
apply live `SKILL.md` files with shell commands or helper scripts.
|
||||
- Keep `SKILL.md` lean; Codex is already capable.
|
||||
- Put only trigger-critical facts in frontmatter `description`.
|
||||
- Quote frontmatter `description`.
|
||||
|
|
@ -52,11 +55,14 @@ Use for PDF manipulation. Prefer deterministic scripts for page edits.
|
|||
## Edit workflow
|
||||
|
||||
1. Read existing skill and nearby resource names.
|
||||
2. Remove generic advice the base model already knows.
|
||||
3. Keep brittle command syntax, auth caveats, safety rules, and validation.
|
||||
4. Replace tables with bullets unless a table is clearly needed.
|
||||
5. Relax prose; fragments ok.
|
||||
6. Validate frontmatter and run any script tests touched.
|
||||
2. Draft the proposed `SKILL.md` content.
|
||||
3. Create or revise the pending proposal through `skill_workshop` when the
|
||||
change should persist as an OpenClaw skill.
|
||||
4. Remove generic advice the base model already knows.
|
||||
5. Keep brittle command syntax, auth caveats, safety rules, and validation.
|
||||
6. Replace tables with bullets unless a table is clearly needed.
|
||||
7. Relax prose; fragments ok.
|
||||
8. Validate frontmatter and run any script tests touched.
|
||||
|
||||
## Validation
|
||||
|
||||
|
|
|
|||
|
|
@ -1,378 +0,0 @@
|
|||
#!/usr/bin/env python3
|
||||
"""
|
||||
Skill Initializer - Creates a new skill from template
|
||||
|
||||
Usage:
|
||||
init_skill.py <skill-name> --path <path> [--resources scripts,references,assets] [--examples]
|
||||
|
||||
Examples:
|
||||
init_skill.py my-new-skill --path skills/public
|
||||
init_skill.py my-new-skill --path skills/public --resources scripts,references
|
||||
init_skill.py my-api-helper --path skills/private --resources scripts --examples
|
||||
init_skill.py custom-skill --path /custom/location
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import re
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
MAX_SKILL_NAME_LENGTH = 64
|
||||
ALLOWED_RESOURCES = {"scripts", "references", "assets"}
|
||||
|
||||
SKILL_TEMPLATE = """---
|
||||
name: {skill_name}
|
||||
description: '[TODO: Complete and informative explanation of what the skill does and when to use it. Include WHEN to use this skill - specific scenarios, file types, or tasks that trigger it.]'
|
||||
---
|
||||
|
||||
# {skill_title}
|
||||
|
||||
## Overview
|
||||
|
||||
[TODO: 1-2 sentences explaining what this skill enables]
|
||||
|
||||
## Structuring This Skill
|
||||
|
||||
[TODO: Choose the structure that best fits this skill's purpose. Common patterns:
|
||||
|
||||
**1. Workflow-Based** (best for sequential processes)
|
||||
- Works well when there are clear step-by-step procedures
|
||||
- Example: DOCX skill with "Workflow Decision Tree" -> "Reading" -> "Creating" -> "Editing"
|
||||
- Structure: ## Overview -> ## Workflow Decision Tree -> ## Step 1 -> ## Step 2...
|
||||
|
||||
**2. Task-Based** (best for tool collections)
|
||||
- Works well when the skill offers different operations/capabilities
|
||||
- Example: PDF skill with "Quick Start" -> "Merge PDFs" -> "Split PDFs" -> "Extract Text"
|
||||
- Structure: ## Overview -> ## Quick Start -> ## Task Category 1 -> ## Task Category 2...
|
||||
|
||||
**3. Reference/Guidelines** (best for standards or specifications)
|
||||
- Works well for brand guidelines, coding standards, or requirements
|
||||
- Example: Brand styling with "Brand Guidelines" -> "Colors" -> "Typography" -> "Features"
|
||||
- Structure: ## Overview -> ## Guidelines -> ## Specifications -> ## Usage...
|
||||
|
||||
**4. Capabilities-Based** (best for integrated systems)
|
||||
- Works well when the skill provides multiple interrelated features
|
||||
- Example: Product Management with "Core Capabilities" -> numbered capability list
|
||||
- Structure: ## Overview -> ## Core Capabilities -> ### 1. Feature -> ### 2. Feature...
|
||||
|
||||
Patterns can be mixed and matched as needed. Most skills combine patterns (e.g., start with task-based, add workflow for complex operations).
|
||||
|
||||
Delete this entire "Structuring This Skill" section when done - it's just guidance.]
|
||||
|
||||
## [TODO: Replace with the first main section based on chosen structure]
|
||||
|
||||
[TODO: Add content here. See examples in existing skills:
|
||||
- Code samples for technical skills
|
||||
- Decision trees for complex workflows
|
||||
- Concrete examples with realistic user requests
|
||||
- References to scripts/templates/references as needed]
|
||||
|
||||
## Resources (optional)
|
||||
|
||||
Create only the resource directories this skill actually needs. Delete this section if no resources are required.
|
||||
|
||||
### scripts/
|
||||
Executable code (Python/Bash/etc.) that can be run directly to perform specific operations.
|
||||
|
||||
**Examples from other skills:**
|
||||
- PDF skill: `fill_fillable_fields.py`, `extract_form_field_info.py` - utilities for PDF manipulation
|
||||
- DOCX skill: `document.py`, `utilities.py` - Python modules for document processing
|
||||
|
||||
**Appropriate for:** Python scripts, shell scripts, or any executable code that performs automation, data processing, or specific operations.
|
||||
|
||||
**Note:** Scripts may be executed without loading into context, but can still be read by Codex for patching or environment adjustments.
|
||||
|
||||
### references/
|
||||
Documentation and reference material intended to be loaded into context to inform Codex's process and thinking.
|
||||
|
||||
**Examples from other skills:**
|
||||
- Product management: `communication.md`, `context_building.md` - detailed workflow guides
|
||||
- BigQuery: API reference documentation and query examples
|
||||
- Finance: Schema documentation, company policies
|
||||
|
||||
**Appropriate for:** In-depth documentation, API references, database schemas, comprehensive guides, or any detailed information that Codex should reference while working.
|
||||
|
||||
### assets/
|
||||
Files not intended to be loaded into context, but rather used within the output Codex produces.
|
||||
|
||||
**Examples from other skills:**
|
||||
- Brand styling: PowerPoint template files (.pptx), logo files
|
||||
- Frontend builder: HTML/React boilerplate project directories
|
||||
- Typography: Font files (.ttf, .woff2)
|
||||
|
||||
**Appropriate for:** Templates, boilerplate code, document templates, images, icons, fonts, or any files meant to be copied or used in the final output.
|
||||
|
||||
---
|
||||
|
||||
**Not every skill requires all three types of resources.**
|
||||
"""
|
||||
|
||||
EXAMPLE_SCRIPT = '''#!/usr/bin/env python3
|
||||
"""
|
||||
Example helper script for {skill_name}
|
||||
|
||||
This is a placeholder script that can be executed directly.
|
||||
Replace with actual implementation or delete if not needed.
|
||||
|
||||
Example real scripts from other skills:
|
||||
- pdf/scripts/fill_fillable_fields.py - Fills PDF form fields
|
||||
- pdf/scripts/convert_pdf_to_images.py - Converts PDF pages to images
|
||||
"""
|
||||
|
||||
def main():
|
||||
print("This is an example script for {skill_name}")
|
||||
# TODO: Add actual script logic here
|
||||
# This could be data processing, file conversion, API calls, etc.
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
'''
|
||||
|
||||
EXAMPLE_REFERENCE = """# Reference Documentation for {skill_title}
|
||||
|
||||
This is a placeholder for detailed reference documentation.
|
||||
Replace with actual reference content or delete if not needed.
|
||||
|
||||
Example real reference docs from other skills:
|
||||
- product-management/references/communication.md - Comprehensive guide for status updates
|
||||
- product-management/references/context_building.md - Deep-dive on gathering context
|
||||
- bigquery/references/ - API references and query examples
|
||||
|
||||
## When Reference Docs Are Useful
|
||||
|
||||
Reference docs are ideal for:
|
||||
- Comprehensive API documentation
|
||||
- Detailed workflow guides
|
||||
- Complex multi-step processes
|
||||
- Information too lengthy for main SKILL.md
|
||||
- Content that's only needed for specific use cases
|
||||
|
||||
## Structure Suggestions
|
||||
|
||||
### API Reference Example
|
||||
- Overview
|
||||
- Authentication
|
||||
- Endpoints with examples
|
||||
- Error codes
|
||||
- Rate limits
|
||||
|
||||
### Workflow Guide Example
|
||||
- Prerequisites
|
||||
- Step-by-step instructions
|
||||
- Common patterns
|
||||
- Troubleshooting
|
||||
- Best practices
|
||||
"""
|
||||
|
||||
EXAMPLE_ASSET = """# Example Asset File
|
||||
|
||||
This placeholder represents where asset files would be stored.
|
||||
Replace with actual asset files (templates, images, fonts, etc.) or delete if not needed.
|
||||
|
||||
Asset files are NOT intended to be loaded into context, but rather used within
|
||||
the output Codex produces.
|
||||
|
||||
Example asset files from other skills:
|
||||
- Brand guidelines: logo.png, slides_template.pptx
|
||||
- Frontend builder: hello-world/ directory with HTML/React boilerplate
|
||||
- Typography: custom-font.ttf, font-family.woff2
|
||||
- Data: sample_data.csv, test_dataset.json
|
||||
|
||||
## Common Asset Types
|
||||
|
||||
- Templates: .pptx, .docx, boilerplate directories
|
||||
- Images: .png, .jpg, .svg, .gif
|
||||
- Fonts: .ttf, .otf, .woff, .woff2
|
||||
- Boilerplate code: Project directories, starter files
|
||||
- Icons: .ico, .svg
|
||||
- Data files: .csv, .json, .xml, .yaml
|
||||
|
||||
Note: This is a text placeholder. Actual assets can be any file type.
|
||||
"""
|
||||
|
||||
|
||||
def normalize_skill_name(skill_name):
|
||||
"""Normalize a skill name to lowercase hyphen-case."""
|
||||
normalized = skill_name.strip().lower()
|
||||
normalized = re.sub(r"[^a-z0-9]+", "-", normalized)
|
||||
normalized = normalized.strip("-")
|
||||
normalized = re.sub(r"-{2,}", "-", normalized)
|
||||
return normalized
|
||||
|
||||
|
||||
def title_case_skill_name(skill_name):
|
||||
"""Convert hyphenated skill name to Title Case for display."""
|
||||
return " ".join(word.capitalize() for word in skill_name.split("-"))
|
||||
|
||||
|
||||
def parse_resources(raw_resources):
|
||||
if not raw_resources:
|
||||
return []
|
||||
resources = [item.strip() for item in raw_resources.split(",") if item.strip()]
|
||||
invalid = sorted({item for item in resources if item not in ALLOWED_RESOURCES})
|
||||
if invalid:
|
||||
allowed = ", ".join(sorted(ALLOWED_RESOURCES))
|
||||
print(f"[ERROR] Unknown resource type(s): {', '.join(invalid)}")
|
||||
print(f" Allowed: {allowed}")
|
||||
sys.exit(1)
|
||||
deduped = []
|
||||
seen = set()
|
||||
for resource in resources:
|
||||
if resource not in seen:
|
||||
deduped.append(resource)
|
||||
seen.add(resource)
|
||||
return deduped
|
||||
|
||||
|
||||
def create_resource_dirs(skill_dir, skill_name, skill_title, resources, include_examples):
|
||||
for resource in resources:
|
||||
resource_dir = skill_dir / resource
|
||||
resource_dir.mkdir(exist_ok=True)
|
||||
if resource == "scripts":
|
||||
if include_examples:
|
||||
example_script = resource_dir / "example.py"
|
||||
example_script.write_text(EXAMPLE_SCRIPT.format(skill_name=skill_name))
|
||||
example_script.chmod(0o755)
|
||||
print("[OK] Created scripts/example.py")
|
||||
else:
|
||||
print("[OK] Created scripts/")
|
||||
elif resource == "references":
|
||||
if include_examples:
|
||||
example_reference = resource_dir / "api_reference.md"
|
||||
example_reference.write_text(EXAMPLE_REFERENCE.format(skill_title=skill_title))
|
||||
print("[OK] Created references/api_reference.md")
|
||||
else:
|
||||
print("[OK] Created references/")
|
||||
elif resource == "assets":
|
||||
if include_examples:
|
||||
example_asset = resource_dir / "example_asset.txt"
|
||||
example_asset.write_text(EXAMPLE_ASSET)
|
||||
print("[OK] Created assets/example_asset.txt")
|
||||
else:
|
||||
print("[OK] Created assets/")
|
||||
|
||||
|
||||
def init_skill(skill_name, path, resources, include_examples):
|
||||
"""
|
||||
Initialize a new skill directory with template SKILL.md.
|
||||
|
||||
Args:
|
||||
skill_name: Name of the skill
|
||||
path: Path where the skill directory should be created
|
||||
resources: Resource directories to create
|
||||
include_examples: Whether to create example files in resource directories
|
||||
|
||||
Returns:
|
||||
Path to created skill directory, or None if error
|
||||
"""
|
||||
# Determine skill directory path
|
||||
skill_dir = Path(path).resolve() / skill_name
|
||||
|
||||
# Check if directory already exists
|
||||
if skill_dir.exists():
|
||||
print(f"[ERROR] Skill directory already exists: {skill_dir}")
|
||||
return None
|
||||
|
||||
# Create skill directory
|
||||
try:
|
||||
skill_dir.mkdir(parents=True, exist_ok=False)
|
||||
print(f"[OK] Created skill directory: {skill_dir}")
|
||||
except Exception as e:
|
||||
print(f"[ERROR] Error creating directory: {e}")
|
||||
return None
|
||||
|
||||
# Create SKILL.md from template
|
||||
skill_title = title_case_skill_name(skill_name)
|
||||
skill_content = SKILL_TEMPLATE.format(skill_name=skill_name, skill_title=skill_title)
|
||||
|
||||
skill_md_path = skill_dir / "SKILL.md"
|
||||
try:
|
||||
skill_md_path.write_text(skill_content)
|
||||
print("[OK] Created SKILL.md")
|
||||
except Exception as e:
|
||||
print(f"[ERROR] Error creating SKILL.md: {e}")
|
||||
return None
|
||||
|
||||
# Create resource directories if requested
|
||||
if resources:
|
||||
try:
|
||||
create_resource_dirs(skill_dir, skill_name, skill_title, resources, include_examples)
|
||||
except Exception as e:
|
||||
print(f"[ERROR] Error creating resource directories: {e}")
|
||||
return None
|
||||
|
||||
# Print next steps
|
||||
print(f"\n[OK] Skill '{skill_name}' initialized successfully at {skill_dir}")
|
||||
print("\nNext steps:")
|
||||
print("1. Edit SKILL.md to complete the TODO items and update the description")
|
||||
if resources:
|
||||
if include_examples:
|
||||
print("2. Customize or delete the example files in scripts/, references/, and assets/")
|
||||
else:
|
||||
print("2. Add resources to scripts/, references/, and assets/ as needed")
|
||||
else:
|
||||
print("2. Create resource directories only if needed (scripts/, references/, assets/)")
|
||||
print("3. Run the validator when ready to check the skill structure")
|
||||
|
||||
return skill_dir
|
||||
|
||||
|
||||
def main():
|
||||
parser = argparse.ArgumentParser(
|
||||
description="Create a new skill directory with a SKILL.md template.",
|
||||
)
|
||||
parser.add_argument("skill_name", help="Skill name (normalized to hyphen-case)")
|
||||
parser.add_argument("--path", required=True, help="Output directory for the skill")
|
||||
parser.add_argument(
|
||||
"--resources",
|
||||
default="",
|
||||
help="Comma-separated list: scripts,references,assets",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--examples",
|
||||
action="store_true",
|
||||
help="Create example files inside the selected resource directories",
|
||||
)
|
||||
args = parser.parse_args()
|
||||
|
||||
raw_skill_name = args.skill_name
|
||||
skill_name = normalize_skill_name(raw_skill_name)
|
||||
if not skill_name:
|
||||
print("[ERROR] Skill name must include at least one letter or digit.")
|
||||
sys.exit(1)
|
||||
if len(skill_name) > MAX_SKILL_NAME_LENGTH:
|
||||
print(
|
||||
f"[ERROR] Skill name '{skill_name}' is too long ({len(skill_name)} characters). "
|
||||
f"Maximum is {MAX_SKILL_NAME_LENGTH} characters."
|
||||
)
|
||||
sys.exit(1)
|
||||
if skill_name != raw_skill_name:
|
||||
print(f"Note: Normalized skill name from '{raw_skill_name}' to '{skill_name}'.")
|
||||
|
||||
resources = parse_resources(args.resources)
|
||||
if args.examples and not resources:
|
||||
print("[ERROR] --examples requires --resources to be set.")
|
||||
sys.exit(1)
|
||||
|
||||
path = args.path
|
||||
|
||||
print(f"Initializing skill: {skill_name}")
|
||||
print(f" Location: {path}")
|
||||
if resources:
|
||||
print(f" Resources: {', '.join(resources)}")
|
||||
if args.examples:
|
||||
print(" Examples: enabled")
|
||||
else:
|
||||
print(" Resources: none (create as needed)")
|
||||
print()
|
||||
|
||||
result = init_skill(skill_name, path, resources, args.examples)
|
||||
|
||||
if result:
|
||||
sys.exit(0)
|
||||
else:
|
||||
sys.exit(1)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
|
|
@ -1,51 +0,0 @@
|
|||
#!/usr/bin/env python3
|
||||
"""
|
||||
Regression tests for skill initialization.
|
||||
"""
|
||||
|
||||
import shutil
|
||||
import sys
|
||||
import tempfile
|
||||
from contextlib import redirect_stdout
|
||||
from io import StringIO
|
||||
from pathlib import Path
|
||||
from unittest import TestCase, main
|
||||
|
||||
SCRIPT_DIR = Path(__file__).resolve().parent
|
||||
if str(SCRIPT_DIR) not in sys.path:
|
||||
sys.path.insert(0, str(SCRIPT_DIR))
|
||||
|
||||
import init_skill
|
||||
|
||||
|
||||
class TestInitSkill(TestCase):
|
||||
def setUp(self):
|
||||
self.temp_dir = Path(tempfile.mkdtemp(prefix="test_init_skill_"))
|
||||
|
||||
def tearDown(self):
|
||||
if self.temp_dir.exists():
|
||||
shutil.rmtree(self.temp_dir)
|
||||
|
||||
def test_generated_description_placeholder_is_yaml_string(self):
|
||||
with redirect_stdout(StringIO()):
|
||||
skill_dir = init_skill.init_skill("yaml-description-skill", self.temp_dir, [], False)
|
||||
|
||||
self.assertIsNotNone(skill_dir)
|
||||
content = (skill_dir / "SKILL.md").read_text(encoding="utf-8")
|
||||
frontmatter = content.split("---", 2)[1]
|
||||
|
||||
self.assertIn("description: '[TODO:", frontmatter)
|
||||
|
||||
try:
|
||||
import yaml
|
||||
except ImportError:
|
||||
self.skipTest("PyYAML is not installed")
|
||||
|
||||
parsed = yaml.safe_load(frontmatter)
|
||||
|
||||
self.assertIsInstance(parsed["description"], str)
|
||||
self.assertTrue(parsed["description"].startswith("[TODO: Complete"))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
Loading…
Add table
Add a link
Reference in a new issue