Alexander Whitestone
65b2e53440
Some checks failed
Forge CI / smoke-and-build (pull_request) Failing after 1m14s
Add tests/test_skill_manager_pokayoke.py
299 lines
12 KiB
Python
299 lines
12 KiB
Python
"""Tests for poka-yoke skill edit revert and validate action."""
|
|
|
|
import json
|
|
import os
|
|
import shutil
|
|
import tempfile
|
|
from pathlib import Path
|
|
from unittest.mock import patch
|
|
|
|
import pytest
|
|
|
|
|
|
@pytest.fixture()
|
|
def isolated_skills_dir(tmp_path, monkeypatch):
|
|
"""Point SKILLS_DIR at a temp directory for test isolation."""
|
|
skills_dir = tmp_path / "skills"
|
|
skills_dir.mkdir()
|
|
monkeypatch.setattr("tools.skill_manager_tool.SKILLS_DIR", skills_dir)
|
|
monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", skills_dir)
|
|
# Also patch skill discovery so _find_skill and validate look in our temp dir
|
|
monkeypatch.setattr(
|
|
"agent.skill_utils.get_all_skills_dirs",
|
|
lambda: [skills_dir],
|
|
)
|
|
return skills_dir
|
|
|
|
|
|
_VALID_SKILL = """\
|
|
---
|
|
name: test-skill
|
|
description: A test skill for unit tests.
|
|
---
|
|
|
|
# Test Skill
|
|
|
|
Instructions here.
|
|
"""
|
|
|
|
|
|
def _create_test_skill(skills_dir: Path, name: str = "test-skill", content: str = _VALID_SKILL):
|
|
skill_dir = skills_dir / name
|
|
skill_dir.mkdir(parents=True, exist_ok=True)
|
|
(skill_dir / "SKILL.md").write_text(content)
|
|
return skill_dir
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# _edit_skill revert on failure
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestEditRevert:
|
|
def test_edit_preserves_original_on_invalid_frontmatter(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
bad_content = "---\nname: test-skill\n---\n" # missing description
|
|
result = json.loads(skill_manage("edit", "test-skill", content=bad_content))
|
|
assert result["success"] is False
|
|
assert "Original file preserved" in result["error"]
|
|
# Original should be untouched
|
|
original = (isolated_skills_dir / "test-skill" / "SKILL.md").read_text()
|
|
assert "A test skill" in original
|
|
|
|
def test_edit_preserves_original_on_empty_body(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
bad_content = "---\nname: test-skill\ndescription: ok\n---\n"
|
|
result = json.loads(skill_manage("edit", "test-skill", content=bad_content))
|
|
assert result["success"] is False
|
|
assert "Original file preserved" in result["error"]
|
|
original = (isolated_skills_dir / "test-skill" / "SKILL.md").read_text()
|
|
assert "Instructions here" in original
|
|
|
|
def test_edit_reverts_on_write_error(self, isolated_skills_dir, monkeypatch):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
|
|
def boom(*a, **kw):
|
|
raise OSError("disk full")
|
|
|
|
monkeypatch.setattr("tools.skill_manager_tool._atomic_write_text", boom)
|
|
result = json.loads(skill_manage("edit", "test-skill", content=_VALID_SKILL))
|
|
assert result["success"] is False
|
|
assert "write error" in result["error"].lower()
|
|
assert "Original file preserved" in result["error"]
|
|
|
|
def test_edit_reverts_on_security_scan_block(self, isolated_skills_dir, monkeypatch):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
monkeypatch.setattr(
|
|
"tools.skill_manager_tool._security_scan_skill",
|
|
lambda path: "Blocked: suspicious content",
|
|
)
|
|
new_content = "---\nname: test-skill\ndescription: updated\n---\n\n# Updated\n"
|
|
result = json.loads(skill_manage("edit", "test-skill", content=new_content))
|
|
assert result["success"] is False
|
|
assert "Original file preserved" in result["error"]
|
|
original = (isolated_skills_dir / "test-skill" / "SKILL.md").read_text()
|
|
assert "A test skill" in original
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# _patch_skill revert on failure
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestPatchRevert:
|
|
def test_patch_preserves_original_on_no_match(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
result = json.loads(skill_manage(
|
|
"patch", "test-skill",
|
|
old_string="NONEXISTENT_TEXT",
|
|
new_string="replacement",
|
|
))
|
|
assert result["success"] is False
|
|
assert "Original file preserved" in result["error"]
|
|
original = (isolated_skills_dir / "test-skill" / "SKILL.md").read_text()
|
|
assert "Instructions here" in original
|
|
|
|
def test_patch_preserves_original_on_broken_frontmatter(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
# Patch that would remove the frontmatter closing ---
|
|
result = json.loads(skill_manage(
|
|
"patch", "test-skill",
|
|
old_string="description: A test skill for unit tests.",
|
|
new_string="", # removing description
|
|
))
|
|
assert result["success"] is False
|
|
assert "Original file preserved" in result["error"]
|
|
original = (isolated_skills_dir / "test-skill" / "SKILL.md").read_text()
|
|
assert "A test skill" in original
|
|
|
|
def test_patch_reverts_on_write_error(self, isolated_skills_dir, monkeypatch):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
|
|
def boom(*a, **kw):
|
|
raise OSError("disk full")
|
|
|
|
monkeypatch.setattr("tools.skill_manager_tool._atomic_write_text", boom)
|
|
result = json.loads(skill_manage(
|
|
"patch", "test-skill",
|
|
old_string="Instructions here.",
|
|
new_string="New instructions.",
|
|
))
|
|
assert result["success"] is False
|
|
assert "write error" in result["error"].lower()
|
|
assert "Original file preserved" in result["error"]
|
|
|
|
def test_patch_reverts_on_security_scan_block(self, isolated_skills_dir, monkeypatch):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
monkeypatch.setattr(
|
|
"tools.skill_manager_tool._security_scan_skill",
|
|
lambda path: "Blocked: malicious code",
|
|
)
|
|
result = json.loads(skill_manage(
|
|
"patch", "test-skill",
|
|
old_string="Instructions here.",
|
|
new_string="New instructions.",
|
|
))
|
|
assert result["success"] is False
|
|
assert "Original file preserved" in result["error"]
|
|
original = (isolated_skills_dir / "test-skill" / "SKILL.md").read_text()
|
|
assert "Instructions here" in original
|
|
|
|
def test_patch_successful_writes_new_content(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
result = json.loads(skill_manage(
|
|
"patch", "test-skill",
|
|
old_string="Instructions here.",
|
|
new_string="Updated instructions.",
|
|
))
|
|
assert result["success"] is True
|
|
content = (isolated_skills_dir / "test-skill" / "SKILL.md").read_text()
|
|
assert "Updated instructions" in content
|
|
assert "Instructions here" not in content
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# _write_file revert on failure
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestWriteFileRevert:
|
|
def test_write_file_reverts_on_security_scan_block(self, isolated_skills_dir, monkeypatch):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
monkeypatch.setattr(
|
|
"tools.skill_manager_tool._security_scan_skill",
|
|
lambda path: "Blocked: malicious",
|
|
)
|
|
result = json.loads(skill_manage(
|
|
"write_file", "test-skill",
|
|
file_path="references/notes.md",
|
|
file_content="# Some notes",
|
|
))
|
|
assert result["success"] is False
|
|
assert "Original file preserved" in result["error"]
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# validate action
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestValidateAction:
|
|
def test_validate_passes_on_good_skill(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
result = json.loads(skill_manage("validate", "test-skill"))
|
|
assert result["success"] is True
|
|
assert result["errors"] == 0
|
|
assert result["results"][0]["valid"] is True
|
|
|
|
def test_validate_finds_missing_description(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
bad = "---\nname: bad-skill\n---\n\nBody here.\n"
|
|
_create_test_skill(isolated_skills_dir, name="bad-skill", content=bad)
|
|
result = json.loads(skill_manage("validate", "bad-skill"))
|
|
assert result["success"] is False
|
|
assert result["errors"] == 1
|
|
issues = result["results"][0]["issues"]
|
|
assert any("description" in i.lower() for i in issues)
|
|
|
|
def test_validate_finds_empty_body(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
empty_body = "---\nname: empty-skill\ndescription: test\n---\n"
|
|
_create_test_skill(isolated_skills_dir, name="empty-skill", content=empty_body)
|
|
result = json.loads(skill_manage("validate", "empty-skill"))
|
|
assert result["success"] is False
|
|
issues = result["results"][0]["issues"]
|
|
assert any("empty body" in i.lower() for i in issues)
|
|
|
|
def test_validate_all_skills(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
_create_test_skill(isolated_skills_dir, name="good-1")
|
|
_create_test_skill(isolated_skills_dir, name="good-2")
|
|
bad = "---\nname: bad\n---\n\nBody.\n"
|
|
_create_test_skill(isolated_skills_dir, name="bad", content=bad)
|
|
|
|
result = json.loads(skill_manage("validate", ""))
|
|
assert result["total"] == 3
|
|
assert result["errors"] == 1
|
|
|
|
def test_validate_nonexistent_skill(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage
|
|
|
|
result = json.loads(skill_manage("validate", "nonexistent"))
|
|
assert result["success"] is False
|
|
assert "not found" in result["error"].lower()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Modification log
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestModificationLog:
|
|
def test_edit_logs_on_success(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage, _MOD_LOG_FILE
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
new = "---\nname: test-skill\ndescription: updated\n---\n\n# Updated\n"
|
|
skill_manage("edit", "test-skill", content=new)
|
|
assert _MOD_LOG_FILE.exists()
|
|
lines = _MOD_LOG_FILE.read_text().strip().split("\n")
|
|
entry = json.loads(lines[-1])
|
|
assert entry["action"] == "edit"
|
|
assert entry["success"] is True
|
|
assert entry["skill"] == "test-skill"
|
|
|
|
def test_patch_logs_on_failure(self, isolated_skills_dir):
|
|
from tools.skill_manager_tool import skill_manage, _MOD_LOG_FILE
|
|
|
|
_create_test_skill(isolated_skills_dir)
|
|
monkeypatch = None # just use no-match to trigger failure
|
|
skill_manage(
|
|
"patch", "test-skill",
|
|
old_string="NONEXISTENT",
|
|
new_string="replacement",
|
|
)
|
|
# Failure before write — no log entry expected since file never changed
|
|
# But the failure path in patch returns early before logging
|
|
# (the log only fires on write-side errors, not match errors)
|
|
# This is correct behavior — no write happened, nothing to log
|