mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(skills): strengthen authoring guidance and accept invocation frontmatter (#121955)
Rewrite the bundled skill-creator SKILL.md workflow-first with checkable completion criteria, tighten the runtime skill-authoring standards prompt, dedupe keep/drop policy out of the collection-only skill_workshop tool description (collection-review.ts remains the sole owner), and teach quick_validate.py the documented invocation frontmatter keys (disable-model-invocation, command-dispatch, command-tool, command-arg-mode) it previously rejected. Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
@@ -1,84 +1,40 @@
|
||||
---
|
||||
name: skill-creator
|
||||
description: "Create, edit, audit, tidy, validate, or restructure AgentSkills and SKILL.md files."
|
||||
description: "Author or review AgentSkills: create, repair, validate, or restructure SKILL.md files and bundled resources."
|
||||
---
|
||||
|
||||
# Skill Creator
|
||||
|
||||
Skills are compact triggerable workflows. Metadata is always visible; body loads only after trigger; references/scripts/assets load only as needed.
|
||||
|
||||
## 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`.
|
||||
- Frontmatter needs `name` + `description`; local OpenClaw skills may also use `metadata`, `homepage`, `allowed-tools`, `user-invocable`, `license`.
|
||||
- Prefer noun-phrase descriptions; short generic trigger phrase, not full workflow.
|
||||
- Move long examples/docs to `references/`; scripts to `scripts/`; templates/media to `assets/`.
|
||||
- No extra README/changelog/setup docs inside a skill unless they are actual task references.
|
||||
- Validate YAML frontmatter after edits.
|
||||
|
||||
## Shape
|
||||
|
||||
```text
|
||||
skill-name/
|
||||
SKILL.md
|
||||
scripts/ optional deterministic helpers
|
||||
references/ optional docs loaded only when needed
|
||||
assets/ optional output resources/templates
|
||||
agents/ optional UI metadata
|
||||
```
|
||||
|
||||
## Good SKILL.md
|
||||
|
||||
```markdown
|
||||
---
|
||||
name: pdf-tools
|
||||
description: "Inspect, split, merge, OCR, redact, or convert PDFs with local CLI tools."
|
||||
---
|
||||
|
||||
# PDF tools
|
||||
|
||||
Use for PDF manipulation. Prefer deterministic scripts for page edits.
|
||||
|
||||
## Workflow
|
||||
|
||||
1. Inspect file/page count.
|
||||
2. Choose exact operation.
|
||||
3. Write output beside input unless user asked otherwise.
|
||||
4. Render/verify changed pages.
|
||||
```
|
||||
1. Establish the contract.
|
||||
- Read the existing skill and its resources, or collect concrete requests for a new skill.
|
||||
- Separate actual workflow branches from synonyms for the same branch.
|
||||
- **Done when:** every branch has a concrete trigger, expected outcome, and persistence target.
|
||||
|
||||
## Edit workflow
|
||||
2. Choose invocation.
|
||||
- Model-discoverable: write a model-facing `description`; omit `disable-model-invocation`.
|
||||
- Manual-only: set `disable-model-invocation: true`; write a human-facing summary.
|
||||
- Direct tool command: add `command-dispatch: tool`, `command-tool`, and `command-arg-mode` only when the command bypasses the model.
|
||||
- **Done when:** frontmatter matches how the skill will actually be reached.
|
||||
|
||||
1. Read existing skill and nearby resource names.
|
||||
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.
|
||||
3. Structure the skill.
|
||||
- Map shared ordered procedure to `SKILL.md`; end every step with a checkable completion criterion and finish with verification.
|
||||
- Keep routing conditions in `description`; start the body with execution.
|
||||
- Map branch-only detail to `references/`, deterministic helpers to `scripts/`, output resources to `assets/`, and optional UI metadata to `agents/`.
|
||||
- **Done when:** every planned resource has one purpose and a direct pointer from `SKILL.md`.
|
||||
|
||||
## Validation
|
||||
4. Draft and persist.
|
||||
- Live workspace skill: use `skill_workshop` to create or revise a pending proposal; keep live files unchanged until apply.
|
||||
- Repository-owned skill source: use the repository's normal edit and review workflow.
|
||||
- **Done when:** the proposal or source diff implements every branch from step 1 and contains every resource from step 3.
|
||||
|
||||
```bash
|
||||
python skills/skill-creator/scripts/quick_validate.py skills/<name>
|
||||
python - <<'PY'
|
||||
from pathlib import Path
|
||||
import yaml
|
||||
for p in Path("skills").glob("*/SKILL.md"):
|
||||
text=p.read_text()
|
||||
if not text.startswith("---\n"):
|
||||
raise SystemExit(f"missing frontmatter: {p}")
|
||||
fm=text.split("---",2)[1]
|
||||
yaml.safe_load(fm)
|
||||
print("ok")
|
||||
PY
|
||||
```
|
||||
5. Validate.
|
||||
- Run `python {baseDir}/scripts/quick_validate.py <skill-directory>` and execute every touched helper's focused test.
|
||||
- **Done when:** frontmatter passes, resource pointers resolve, and every touched helper passes its focused test.
|
||||
|
||||
`quick_validate.py` is conservative; repo-local frontmatter may allow keys beyond public skill bundles.
|
||||
## Frontmatter
|
||||
|
||||
Required: `name`, `description`.
|
||||
|
||||
OpenClaw also supports `metadata`, `homepage`, `license`, `allowed-tools`, `user-invocable`, `disable-model-invocation`, `command-dispatch`, `command-tool`, and `command-arg-mode`. Add optional fields only when they change runtime behavior or discovery.
|
||||
|
||||
@@ -102,6 +102,10 @@ def validate_skill(skill_path):
|
||||
"license",
|
||||
"allowed-tools",
|
||||
"user-invocable",
|
||||
"disable-model-invocation",
|
||||
"command-dispatch",
|
||||
"command-tool",
|
||||
"command-arg-mode",
|
||||
"metadata",
|
||||
}
|
||||
|
||||
|
||||
@@ -111,6 +111,26 @@ metadata: |
|
||||
self.assertFalse(valid)
|
||||
self.assertEqual(message, "Description must not be empty")
|
||||
|
||||
def test_accepts_openclaw_invocation_frontmatter(self):
|
||||
skill_dir = self.temp_dir / "invocable-skill"
|
||||
skill_dir.mkdir(parents=True, exist_ok=True)
|
||||
content = """---
|
||||
name: invocable-skill
|
||||
description: A manually invoked skill
|
||||
user-invocable: true
|
||||
disable-model-invocation: true
|
||||
command-dispatch: tool
|
||||
command-tool: example_tool
|
||||
command-arg-mode: raw
|
||||
---
|
||||
# Skill
|
||||
"""
|
||||
(skill_dir / "SKILL.md").write_text(content, encoding="utf-8")
|
||||
|
||||
valid, message = quick_validate.validate_skill(skill_dir)
|
||||
|
||||
self.assertTrue(valid, message)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
|
||||
@@ -8,7 +8,7 @@ export function buildSkillWorkshopToolDescription(params: {
|
||||
collectionOnly: boolean;
|
||||
}): string {
|
||||
if (params.collectionOnly) {
|
||||
return `Read every current writable skill, then replace the collection with one reconcile call. Keep distinct useful skills. Never drop a skill only because it is specialized; a narrow trigger is useful when it routes reliably. Rewrite weak skills. Merge overlap by writing the strongest result and dropping the redundant skills. Drop only clear junk, task artifacts, unusable stale fragments, duplicates, or skills fully preserved by a surviving skill. Every current skill needs exactly one keep, write, or drop decision.\n\n${SKILL_AUTHORING_STANDARDS_PROMPT}`;
|
||||
return `Read every current writable skill, then replace the collection with one reconcile call. Every current skill needs exactly one keep, write, or drop decision.\n\n${SKILL_AUTHORING_STANDARDS_PROMPT}`;
|
||||
}
|
||||
if (!params.proposalOnly) {
|
||||
const repairPolicy =
|
||||
|
||||
@@ -49,8 +49,8 @@ describe("skill_workshop tool", () => {
|
||||
});
|
||||
|
||||
expect(JSON.stringify(tool.parameters)).toContain('"enum":["read","reconcile"]');
|
||||
expect(tool.description).toContain("Never drop a skill only because it is specialized");
|
||||
expect(tool.description).not.toContain("Drop narrow");
|
||||
expect(tool.description).toContain("exactly one keep, write, or drop decision");
|
||||
expect(tool.description).toContain(SKILL_AUTHORING_STANDARDS_PROMPT);
|
||||
await tool.execute("read", { action: "read", skill_name: "duplicate" });
|
||||
await tool.execute("reconcile", {
|
||||
action: "reconcile",
|
||||
|
||||
@@ -7,9 +7,12 @@ import { SKILL_AUTHORING_STANDARDS_PROMPT } from "./skill-authoring-standards.js
|
||||
describe("skill authoring standards", () => {
|
||||
it("defines routing, naming, body, token, evidence, and durable-fix requirements", () => {
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("first ~60 characters");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("one trigger per actual branch");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("notes, helpers, or workflows");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("class-level name");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("exact procedure steps");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("checkable completion criterion");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("direct pointer");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("one source for each meaning");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("Every sentence must earn its tokens");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("never invent flags");
|
||||
expect(SKILL_AUTHORING_STANDARDS_PROMPT).toContain("capture the working fix");
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
export const SKILL_AUTHORING_STANDARDS_PROMPT = [
|
||||
"Skill authoring standards:",
|
||||
"- Description: write one sentence. Lead with concrete trigger phrases or the task class in the first ~60 characters so the skill index can route the request before loading the body. Do not use generic filler; notes, helpers, or workflows cannot be the sole descriptor.",
|
||||
"- Description: write one sentence. Lead with concrete trigger phrases or the task class in the first ~60 characters so the skill index can route the request before loading the body. Keep one trigger per actual branch and collapse synonyms; notes, helpers, or workflows cannot be the sole descriptor.",
|
||||
"- Name: choose a lowercase-hyphenated class-level name that will still identify the task a month later. Reject names tied to one session, run ID, incident ID, calendar date, or other temporary artifact.",
|
||||
"- Body: state when to use the skill, give exact procedure steps, name evidenced pitfalls, and include an evidence-backed verification step.",
|
||||
"- Token-efficient language: skills load into model context. Use compact imperative language, short lines, and no narration, filler, or obvious restatement. Every sentence must earn its tokens.",
|
||||
"- Invocation: preserve the current policy unless evidence calls for a change. A model-discoverable skill omits `disable-model-invocation`; a manual-only skill sets it to `true`.",
|
||||
"- Procedure: put shared ordered steps before reference material. End every step with a checkable completion criterion. Move branch-only detail into a bundled resource with a direct pointer from the body, and reference every bundled resource.",
|
||||
"- Language: use compact positive imperatives and short lines. State the target behavior; reserve prohibitions for hard guardrails and pair them with the target. Keep one source for each meaning. Every sentence must earn its tokens.",
|
||||
"- Evidence: never invent flags, commands, paths, APIs, tool behavior, or requirements that the source material does not establish. Omit unsupported details or mark them as unknown.",
|
||||
"- Durable learning: capture the working fix, recovery, or procedure. Never preserve a standalone claim that something does not work after the problem may be gone.",
|
||||
].join("\n");
|
||||
|
||||
Reference in New Issue
Block a user