fix(skills): rollback reads its history file off the event loop (#5729)

* fix(skills): rollback reads its history off the event loop

rollback_custom_skill constructed the user-scoped storage, probed whether the
skill exists and parsed custom/.history/<name>.jsonl inline, while the sibling
get_custom_skill_history two lines above already offloads exactly those three
steps and documents them as "blocking filesystem IO that must stay off the
event loop". The history file grows one entry per edit carrying full previous
and new content, so a rollback request parsed the whole edit history on the
Gateway loop; the strict Blockbuster gate catches the construction as a blocking
os.getcwd() through SkillsConfig.get_skills_path() -> project_root().

Offload the three steps through the same worker-thread closure and anchor both
pre-scan branches under tests/blocking_io/.

* test(skills): address rollback anchor review feedback

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

backend/app/gateway/routers/skills.py

@@ -618,10 +618,19 @@ async def get_custom_skill_history(skill_name: str, request: Request, config: Ap
 async def rollback_custom_skill(skill_name: str, body: SkillRollbackRequest, request: Request, config: AppConfig = Depends(get_config)) -> CustomSkillContentResponse:
     await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
     try:
-        storage = _get_user_skill_storage(config)
-        if not storage.custom_skill_exists(skill_name) and not storage.get_skill_history_file(skill_name).exists():
+
+        def _read_rollback_history() -> tuple[SkillStorage, list[dict] | None]:
+            # Worker thread: storage construction, the existence probes, and the
+            # history-file read are blocking filesystem IO that must stay off the
+            # event loop — the same rule get_custom_skill_history applies above.
+            storage = _get_user_skill_storage(config)
+            if not storage.custom_skill_exists(skill_name) and not storage.get_skill_history_file(skill_name).exists():
+                return storage, None
+            return storage, storage.read_history(skill_name)
+
+        storage, history = await asyncio.to_thread(_read_rollback_history)
+        if history is None:
             raise HTTPException(status_code=404, detail=f"Custom skill '{skill_name}' not found")
-        history = storage.read_history(skill_name)
         if not history:
             raise HTTPException(status_code=400, detail=f"Custom skill '{skill_name}' has no history")
         record = history[body.history_index]

backend/tests/blocking_io/test_skills_custom_router.py

@@ -0,0 +1,86 @@
+"""Regression anchor: the custom-skill rollback route must keep its history reads off the loop.
+
+``rollback_custom_skill`` offloads storage construction, existence probes and
+the ``custom/.history/<name>.jsonl`` read through ``asyncio.to_thread``, matching
+the adjacent ``get_custom_skill_history`` handler. History entries contain the
+full previous and new skill content, so reading and parsing the entire history
+on the Gateway loop would stall other requests. The post-scan current-content
+read is outside this anchor's coverage and remains a separate blocking-IO fix.
+
+The two branches driven here both return before the awaited security scan, so
+the anchor needs no scanner or model stub: the 404 branch covers construction
+plus the existence probes, and the out-of-range branch covers the full
+history-file read and parse.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import json
+from pathlib import Path
+from types import SimpleNamespace
+from uuid import UUID
+
+import pytest
+from fastapi import HTTPException, Request
+
+from app.gateway.routers.skills import SkillRollbackRequest, rollback_custom_skill
+from deerflow.config.app_config import AppConfig
+from deerflow.config.paths import get_paths
+from deerflow.runtime.user_context import get_effective_user_id
+
+pytestmark = pytest.mark.asyncio
+
+_SKILL_NAME = "loop-rollback-skill"
+_SKILL_MD = f"---\nname: {_SKILL_NAME}\ndescription: Anchor fixture skill.\n---\n\n# {_SKILL_NAME}\n"
+
+
+def _custom_dir() -> Path:
+    return get_paths().user_custom_skills_dir(get_effective_user_id())
+
+
+@pytest.fixture(autouse=True)
+def _isolate_paths(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path))
+    monkeypatch.setattr("deerflow.config.paths._paths", None)
+
+
+def _admin_request() -> Request:
+    # AuthMiddleware normally supplies this state; keep the real admin check.
+    user = SimpleNamespace(id=UUID("11111111-2222-3333-4444-555555555555"), system_role="admin")
+    return Request({"type": "http", "headers": [], "state": {"user": user}})
+
+
+def _install_skill() -> None:
+    skill_dir = _custom_dir() / _SKILL_NAME
+    skill_dir.mkdir(parents=True, exist_ok=True)
+    (skill_dir / "SKILL.md").write_text(_SKILL_MD, encoding="utf-8")
+
+
+def _write_history(records: list[dict]) -> None:
+    history_dir = _custom_dir() / ".history"
+    history_dir.mkdir(parents=True, exist_ok=True)
+    (history_dir / f"{_SKILL_NAME}.jsonl").write_text("".join(json.dumps(r) + "\n" for r in records), encoding="utf-8")
+
+
+async def test_rollback_missing_skill_does_not_block_event_loop() -> None:
+    """The 404 branch must not resolve paths or probe the filesystem from the loop."""
+    config = AppConfig.model_validate({"sandbox": {"use": "test"}})
+
+    with pytest.raises(HTTPException) as excinfo:
+        await rollback_custom_skill(_SKILL_NAME, SkillRollbackRequest(history_index=0), _admin_request(), config)
+
+    assert excinfo.value.status_code == 404
+
+
+async def test_rollback_history_read_does_not_block_event_loop() -> None:
+    """The out-of-range branch reads and parses the whole history file first."""
+    await asyncio.to_thread(_install_skill)
+    await asyncio.to_thread(_write_history, [{"action": "edit", "ts": 1, "prev_content": _SKILL_MD, "new_content": _SKILL_MD}])
+    config = AppConfig.model_validate({"sandbox": {"use": "test"}})
+
+    with pytest.raises(HTTPException) as excinfo:
+        await rollback_custom_skill(_SKILL_NAME, SkillRollbackRequest(history_index=99), _admin_request(), config)
+
+    assert excinfo.value.status_code == 400
+    assert "history_index is out of range" in str(excinfo.value.detail)