fix(skills): install for a user keeps its custom-dir setup off the event loop (#5650)

* 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.
This commit is contained in:
lihongyuan99 2026-09-22 18:39:19 +08:00 committed by GitHub
parent 0ecb077fe7
commit 53352287a7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 42 additions and 2 deletions

View File

@ -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.

View File

@ -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)