From 53352287a7197cc609379536e9d2ee5580740add Mon Sep 17 00:00:00 2001 From: lihongyuan99 <64824864+lihongyuan99@users.noreply.github.com> Date: Tue, 22 Sep 2026 18:39:19 +0800 Subject: [PATCH] fix(skills): install for a user keeps its custom-dir setup off the event loop (#5650) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(skills): install for a user keeps its custom-dir setup off the event loop UserScopedSkillStorage.ainstall_skill_from_archive re-implemented the install pipeline and created the per-user custom directory inline, so the await behind POST /api/skills/install blocked the Gateway loop on os.mkdir. The base class offloads every filesystem phase around its LLM scan — and says so — but the override's setup step sat above that comment and escaped it, and the existing blocking-IO anchor drove only the host-scoped class. Offload the mkdir through the same worker thread and anchor the override itself under the strict Blockbuster gate. * test(blocking-io): pin per-user staging of supporting skill files Review follow-up on #5650: the user-scoped anchor asserted only the staged SKILL.md, so a regression that mis-stages the references/ tree under the per-user root would pass both anchors. Assert the supporting file against the per-user layout as well, mirroring the host-scoped twin. --- .../storage/user_scoped_skill_storage.py | 6 ++- .../tests/blocking_io/test_skills_install.py | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py b/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py index e7a652039..8e2a45986 100644 --- a/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py +++ b/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py @@ -329,8 +329,10 @@ class UserScopedSkillStorage(LocalSkillStorage): path = Path(archive_path) custom_dir = self._user_custom_root - # Ensure user custom directory exists - custom_dir.mkdir(parents=True, exist_ok=True) + # Ensure user custom directory exists. This is filesystem work too, so + # it goes through the same worker-thread discipline as the phases below + # — the install route awaits this coroutine on the Gateway event loop. + await asyncio.to_thread(custom_dir.mkdir, parents=True, exist_ok=True) # The per-file security scan is an async LLM call and must stay on the # event loop; every filesystem phase around it runs in a worker thread. diff --git a/backend/tests/blocking_io/test_skills_install.py b/backend/tests/blocking_io/test_skills_install.py index 9af5e66bd..17d510c0b 100644 --- a/backend/tests/blocking_io/test_skills_install.py +++ b/backend/tests/blocking_io/test_skills_install.py @@ -15,6 +15,10 @@ archive, extraction, validation, and staging all run against the real local filesystem. Test-side setup IO is itself offloaded with ``asyncio.to_thread`` (matching ``test_agents_router``) so only the production path is exercised on the loop. + +``UserScopedSkillStorage`` overrides the same entry point and is what the route +actually awaits once per-user skill isolation is on, so it gets its own anchor +rather than inheriting the host-scoped one. """ from __future__ import annotations @@ -71,3 +75,37 @@ async def test_install_skill_archive_does_not_block_event_loop(tmp_path: Path, m installed_md = tmp_path / "skills" / "custom" / "loop-skill" / "SKILL.md" assert await asyncio.to_thread(installed_md.exists) assert await asyncio.to_thread((tmp_path / "skills" / "custom" / "loop-skill" / "references" / "usage.md").exists) + + +async def test_user_scoped_install_skill_archive_does_not_block_event_loop(tmp_path: Path, monkeypatch) -> None: + """The override the install route actually awaits must offload its own setup too. + + ``UserScopedSkillStorage.ainstall_skill_from_archive`` re-implements the + pipeline with a per-user custom root and creates that root inside the + coroutine, so the host-scoped anchor above cannot catch a regression there. + """ + from deerflow.config import paths as paths_mod + from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage + + archive = tmp_path / "loop-skill.skill" + await asyncio.to_thread(_build_archive, archive) + + async def _allow_scan(content: str, *, executable: bool = False, location: str = "SKILL.md", app_config=None, static_findings=None): + return SimpleNamespace(decision="allow", reason="anchor stub") + + monkeypatch.setattr("deerflow.skills.installer.scan_skill_content", _allow_scan) + monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path)) + monkeypatch.setattr(paths_mod, "_paths", None) + + # Constructor resolves per-user paths; offloaded so the only production + # work on the loop is the install pipeline itself. + storage = await asyncio.to_thread(UserScopedSkillStorage, "anchoruser", host_path=str(tmp_path / "skills")) + + result = await storage.ainstall_skill_from_archive(archive) + + assert result["success"] is True + assert result["skill_name"] == "loop-skill" + installed_md = tmp_path / "users" / "anchoruser" / "skills" / "custom" / "loop-skill" / "SKILL.md" + assert await asyncio.to_thread(installed_md.is_file) + installed_support = tmp_path / "users" / "anchoruser" / "skills" / "custom" / "loop-skill" / "references" / "usage.md" + assert await asyncio.to_thread(installed_support.is_file)