fix(skills): avoid blocking custom skill deletion on readonly history writes (#2197)

This commit is contained in:
sqsge 2026-04-14 09:00:29 +08:00 committed by GitHub
parent a7e7c6d667
commit 053e18e1a6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 87 additions and 12 deletions

View File

@ -1,3 +1,4 @@
import errno
import json
import logging
import shutil
@ -201,18 +202,23 @@ async def delete_custom_skill(skill_name: str) -> dict[str, bool]:
ensure_custom_skill_is_editable(skill_name)
skill_dir = get_custom_skill_dir(skill_name)
prev_content = read_custom_skill_content(skill_name)
append_history(
skill_name,
{
"action": "human_delete",
"author": "human",
"thread_id": None,
"file_path": "SKILL.md",
"prev_content": prev_content,
"new_content": None,
"scanner": {"decision": "allow", "reason": "Deletion requested."},
},
)
try:
append_history(
skill_name,
{
"action": "human_delete",
"author": "human",
"thread_id": None,
"file_path": "SKILL.md",
"prev_content": prev_content,
"new_content": None,
"scanner": {"decision": "allow", "reason": "Deletion requested."},
},
)
except OSError as e:
if not isinstance(e, PermissionError) and e.errno not in {errno.EACCES, errno.EPERM, errno.EROFS}:
raise
logger.warning("Skipping delete history write for custom skill %s due to readonly/permission failure; continuing with skill directory removal: %s", skill_name, e)
shutil.rmtree(skill_dir)
await refresh_skills_system_prompt_cache_async()
return {"success": True}

View File

@ -1,3 +1,4 @@
import errno
import json
from pathlib import Path
from types import SimpleNamespace
@ -164,6 +165,74 @@ def test_custom_skill_delete_preserves_history_and_allows_restore(monkeypatch, t
assert refresh_calls == ["refresh", "refresh"]
def test_custom_skill_delete_continues_when_history_write_is_readonly(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
custom_dir = skills_root / "custom" / "demo-skill"
custom_dir.mkdir(parents=True, exist_ok=True)
(custom_dir / "SKILL.md").write_text(_skill_content("demo-skill"), encoding="utf-8")
config = SimpleNamespace(
skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills"),
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
monkeypatch.setattr("deerflow.skills.manager.get_app_config", lambda: config)
refresh_calls = []
async def _refresh():
refresh_calls.append("refresh")
def _readonly_history(*args, **kwargs):
raise OSError(errno.EROFS, "Read-only file system", str(skills_root / "custom" / ".history"))
monkeypatch.setattr("app.gateway.routers.skills.append_history", _readonly_history)
monkeypatch.setattr("app.gateway.routers.skills.refresh_skills_system_prompt_cache_async", _refresh)
app = FastAPI()
app.include_router(skills_router.router)
with TestClient(app) as client:
delete_response = client.delete("/api/skills/custom/demo-skill")
assert delete_response.status_code == 200
assert delete_response.json() == {"success": True}
assert not custom_dir.exists()
assert refresh_calls == ["refresh"]
def test_custom_skill_delete_fails_when_skill_dir_removal_fails(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
custom_dir = skills_root / "custom" / "demo-skill"
custom_dir.mkdir(parents=True, exist_ok=True)
(custom_dir / "SKILL.md").write_text(_skill_content("demo-skill"), encoding="utf-8")
config = SimpleNamespace(
skills=SimpleNamespace(get_skills_path=lambda: skills_root, container_path="/mnt/skills"),
skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
)
monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
monkeypatch.setattr("deerflow.skills.manager.get_app_config", lambda: config)
refresh_calls = []
async def _refresh():
refresh_calls.append("refresh")
def _fail_rmtree(*args, **kwargs):
raise PermissionError(errno.EACCES, "Permission denied", str(custom_dir))
monkeypatch.setattr("app.gateway.routers.skills.shutil.rmtree", _fail_rmtree)
monkeypatch.setattr("app.gateway.routers.skills.refresh_skills_system_prompt_cache_async", _refresh)
app = FastAPI()
app.include_router(skills_router.router)
with TestClient(app) as client:
delete_response = client.delete("/api/skills/custom/demo-skill")
assert delete_response.status_code == 500
assert "Failed to delete custom skill" in delete_response.json()["detail"]
assert custom_dir.exists()
assert refresh_calls == []
def test_update_skill_refreshes_prompt_cache_before_return(monkeypatch, tmp_path):
config_path = tmp_path / "extensions_config.json"
enabled_state = {"value": True}