mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
fix(skill-creator): reject empty name and description in skill valida… (#83704)
Summary: - The PR makes skill-creator quick validation reject empty or whitespace-only `name` and `description` fields, adds regression tests, and records the fix in the changelog. - Reproducibility: yes. Source inspection on current main shows empty or whitespace-only values skip validation after `.strip()`, and the source PR includes before/after terminal output for the same path. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(skill-creator): reject empty name and description in skill valida… Validation: - ClawSweeper review passed for head0fb4555cb2. - Required merge gates passed before the squash merge. Prepared head SHA:0fb4555cb2Review: https://github.com/openclaw/openclaw/pull/83704#issuecomment-4479984760 Co-authored-by: jay <a1@ponys-Mac.local> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com>
This commit is contained in:
@@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai
|
||||
### Fixes
|
||||
|
||||
- Agents/image generation: allow distinct `image_generate` prompts to start separate session-backed background tasks while same-prompt retries still return the active task status. (#83614) Thanks @Elarwei001.
|
||||
- Skills: reject empty or whitespace-only skill names and descriptions during quick validation. (#27061)
|
||||
- Sessions: skip trailing custom transcript entries when checking tail assistant replies so embedded CLI gap-fill does not duplicate canonical assistant output. (#83635) Thanks @yaoyi1222.
|
||||
- Memory Wiki: keep `wiki_lint` tool output path-safe by reporting vault-internal lint reports as relative paths in tool text and details while preserving absolute report paths for CLI/file callers. (#83439) Thanks @LLagoon3.
|
||||
- Telegram: keep verbose tool progress visible without mirroring non-final progress into active session transcripts, preventing embedded provider replies from aborting mid-run. (#83631) Thanks @kurplunkin.
|
||||
|
||||
@@ -123,36 +123,38 @@ def validate_skill(skill_path):
|
||||
if not isinstance(name, str):
|
||||
return False, f"Name must be a string, got {type(name).__name__}"
|
||||
name = name.strip()
|
||||
if name:
|
||||
if not re.match(r"^[a-z0-9-]+$", name):
|
||||
return (
|
||||
False,
|
||||
f"Name '{name}' should be hyphen-case (lowercase letters, digits, and hyphens only)",
|
||||
)
|
||||
if name.startswith("-") or name.endswith("-") or "--" in name:
|
||||
return (
|
||||
False,
|
||||
f"Name '{name}' cannot start/end with hyphen or contain consecutive hyphens",
|
||||
)
|
||||
if len(name) > MAX_SKILL_NAME_LENGTH:
|
||||
return (
|
||||
False,
|
||||
f"Name is too long ({len(name)} characters). "
|
||||
f"Maximum is {MAX_SKILL_NAME_LENGTH} characters.",
|
||||
)
|
||||
if not name:
|
||||
return False, "Name must not be empty"
|
||||
if not re.match(r"^[a-z0-9-]+$", name):
|
||||
return (
|
||||
False,
|
||||
f"Name '{name}' should be hyphen-case (lowercase letters, digits, and hyphens only)",
|
||||
)
|
||||
if name.startswith("-") or name.endswith("-") or "--" in name:
|
||||
return (
|
||||
False,
|
||||
f"Name '{name}' cannot start/end with hyphen or contain consecutive hyphens",
|
||||
)
|
||||
if len(name) > MAX_SKILL_NAME_LENGTH:
|
||||
return (
|
||||
False,
|
||||
f"Name is too long ({len(name)} characters). "
|
||||
f"Maximum is {MAX_SKILL_NAME_LENGTH} characters.",
|
||||
)
|
||||
|
||||
description = frontmatter.get("description", "")
|
||||
if not isinstance(description, str):
|
||||
return False, f"Description must be a string, got {type(description).__name__}"
|
||||
description = description.strip()
|
||||
if description:
|
||||
if "<" in description or ">" in description:
|
||||
return False, "Description cannot contain angle brackets (< or >)"
|
||||
if len(description) > 1024:
|
||||
return (
|
||||
False,
|
||||
f"Description is too long ({len(description)} characters). Maximum is 1024 characters.",
|
||||
)
|
||||
if not description:
|
||||
return False, "Description must not be empty"
|
||||
if "<" in description or ">" in description:
|
||||
return False, "Description cannot contain angle brackets (< or >)"
|
||||
if len(description) > 1024:
|
||||
return (
|
||||
False,
|
||||
f"Description is too long ({len(description)} characters). Maximum is 1024 characters.",
|
||||
)
|
||||
|
||||
return True, "Skill is valid!"
|
||||
|
||||
|
||||
@@ -67,6 +67,50 @@ metadata: |
|
||||
|
||||
self.assertTrue(valid, message)
|
||||
|
||||
def test_rejects_empty_name(self):
|
||||
skill_dir = self.temp_dir / "empty-name-skill"
|
||||
skill_dir.mkdir(parents=True, exist_ok=True)
|
||||
content = '---\nname: ""\ndescription: a valid description\n---\n# Skill\n'
|
||||
(skill_dir / "SKILL.md").write_text(content, encoding="utf-8")
|
||||
|
||||
valid, message = quick_validate.validate_skill(skill_dir)
|
||||
|
||||
self.assertFalse(valid)
|
||||
self.assertEqual(message, "Name must not be empty")
|
||||
|
||||
def test_rejects_whitespace_only_name(self):
|
||||
skill_dir = self.temp_dir / "ws-name-skill"
|
||||
skill_dir.mkdir(parents=True, exist_ok=True)
|
||||
content = "---\nname: ' '\ndescription: a valid description\n---\n# Skill\n"
|
||||
(skill_dir / "SKILL.md").write_text(content, encoding="utf-8")
|
||||
|
||||
valid, message = quick_validate.validate_skill(skill_dir)
|
||||
|
||||
self.assertFalse(valid)
|
||||
self.assertEqual(message, "Name must not be empty")
|
||||
|
||||
def test_rejects_empty_description(self):
|
||||
skill_dir = self.temp_dir / "empty-desc-skill"
|
||||
skill_dir.mkdir(parents=True, exist_ok=True)
|
||||
content = '---\nname: valid-skill\ndescription: ""\n---\n# Skill\n'
|
||||
(skill_dir / "SKILL.md").write_text(content, encoding="utf-8")
|
||||
|
||||
valid, message = quick_validate.validate_skill(skill_dir)
|
||||
|
||||
self.assertFalse(valid)
|
||||
self.assertEqual(message, "Description must not be empty")
|
||||
|
||||
def test_rejects_whitespace_only_description(self):
|
||||
skill_dir = self.temp_dir / "ws-desc-skill"
|
||||
skill_dir.mkdir(parents=True, exist_ok=True)
|
||||
content = "---\nname: valid-skill\ndescription: ' '\n---\n# Skill\n"
|
||||
(skill_dir / "SKILL.md").write_text(content, encoding="utf-8")
|
||||
|
||||
valid, message = quick_validate.validate_skill(skill_dir)
|
||||
|
||||
self.assertFalse(valid)
|
||||
self.assertEqual(message, "Description must not be empty")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
|
||||
Reference in New Issue
Block a user