diff --git a/backend/app/gateway/routers/skills.py b/backend/app/gateway/routers/skills.py index 6d29b4582..005efb3b1 100644 --- a/backend/app/gateway/routers/skills.py +++ b/backend/app/gateway/routers/skills.py @@ -505,12 +505,22 @@ async def get_custom_skill(skill_name: str, request: Request, config: AppConfig async def _read_custom_skill_response(skill_name: str, config: AppConfig) -> CustomSkillContentResponse: try: skill_name = skill_name.replace("\r\n", "").replace("\n", "") - storage = _get_user_skill_storage(config) - skills = storage.load_skills(enabled_only=False) - skill = next((s for s in skills if s.name == skill_name and s.category == SkillCategory.CUSTOM), None) + + def _load_response_parts() -> tuple[Skill | None, str | None]: + # Worker thread: load_skills walks every skill directory and + # read_custom_skill opens SKILL.md — blocking filesystem IO that + # scales with the number of installed skills (#5747). + storage = _get_user_skill_storage(config) + skills = storage.load_skills(enabled_only=False) + skill = next((s for s in skills if s.name == skill_name and s.category == SkillCategory.CUSTOM), None) + if skill is None: + return None, None + return skill, storage.read_custom_skill(skill_name) + + skill, content = await asyncio.to_thread(_load_response_parts) if skill is None: raise HTTPException(status_code=404, detail=f"Custom skill '{skill_name}' not found") - return CustomSkillContentResponse(**_skill_to_response(skill).model_dump(), content=storage.read_custom_skill(skill_name)) + return CustomSkillContentResponse(**_skill_to_response(skill).model_dump(), content=content) except HTTPException: raise except Exception as e: @@ -637,11 +647,17 @@ async def rollback_custom_skill(skill_name: str, body: SkillRollbackRequest, req target_content = record.get("prev_content") if target_content is None: raise HTTPException(status_code=400, detail="Selected history entry has no previous content to roll back to") - storage.validate_skill_markdown_content(skill_name, target_content) + await asyncio.to_thread(storage.validate_skill_markdown_content, skill_name, target_content) static_findings = await _scan_static_skill_markdown_or_raise(skill_name, target_content, app_config=config) scan = await scan_skill_content(target_content, executable=False, location=f"{skill_name}/{SKILL_MD_FILE}", app_config=config, static_findings=static_findings) - skill_file = storage.get_custom_skill_file(skill_name) - current_content = skill_file.read_text(encoding="utf-8") if skill_file.exists() else None + + def _read_current_content() -> str | None: + # Worker thread: the post-scan read of the file being replaced is + # blocking filesystem IO (#5747), same rule as the history read. + skill_file = storage.get_custom_skill_file(skill_name) + return skill_file.read_text(encoding="utf-8") if skill_file.exists() else None + + current_content = await asyncio.to_thread(_read_current_content) history_entry = { "action": "rollback", "author": "human", diff --git a/backend/tests/blocking_io/test_skills_custom_router.py b/backend/tests/blocking_io/test_skills_custom_router.py index e85ac4aa1..582d2b445 100644 --- a/backend/tests/blocking_io/test_skills_custom_router.py +++ b/backend/tests/blocking_io/test_skills_custom_router.py @@ -19,6 +19,7 @@ import asyncio import json from pathlib import Path from types import SimpleNamespace +from unittest.mock import AsyncMock from uuid import UUID import pytest @@ -84,3 +85,23 @@ async def test_rollback_history_read_does_not_block_event_loop() -> None: assert excinfo.value.status_code == 400 assert "history_index is out of range" in str(excinfo.value.detail) + + +async def test_rollback_accepted_path_does_not_block_event_loop(monkeypatch) -> None: + """The accepted-rollback path (validate → scan → current-content read → + write → append → response read) must keep every filesystem operation off + the loop (#5747).""" + await asyncio.to_thread(_install_skill) + await asyncio.to_thread( + _write_history, + [{"action": "edit", "ts": 1, "prev_content": _SKILL_MD, "new_content": _SKILL_MD}], + ) + monkeypatch.setattr( + "app.gateway.routers.skills.scan_skill_content", + AsyncMock(return_value=SimpleNamespace(decision="allow", reason="ok", static_findings=[])), + ) + config = AppConfig.model_validate({"sandbox": {"use": "test"}}) + + response = await rollback_custom_skill(_SKILL_NAME, SkillRollbackRequest(history_index=0), _admin_request(), config) + + assert response.content == _SKILL_MD