mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-27 21:50:36 +00:00
4bab280250
* fix(skills): share skill cache invalidation across Gateway replicas SkillStorage.load_skills() rescans disk and re-reads extensions_config.json on every call, but the prompt layer in deerflow/agents/lead_agent/prompt.py caches the enabled-skills list, the per-(app_config, user_id) entries and the rendered <skill_system> section per process, and every skill mutation in routers/skills.py only refreshed the process that handled it. With several uvicorn workers or several Pods sharing one home volume, a skill installed, edited, deleted, rolled back or toggled through replica A kept being offered as the old skill set by replica B until B restarted, and POST /api/skills/reload had to be sent to every Pod separately. Reuse the MCP cache-reset mechanism. A new generic helper, deerflow.config.shared_reset_marker, owns a hidden JSON marker beside the resolved extensions config (.<config name>.<suffix>.json): publish() replaces it atomically with a random generation under extensions_config_write_lock + extensions_config_file_lock, chaining previous_generation and an optional user_id; SharedResetMarkerTracker polls it at most once per second (monotonic clock, never blocking behind a concurrent poller), adopts the current state silently on its first poll, and classifies a change as user-scoped only when exactly one publication carrying that user happened since the last poll. deerflow/mcp/cache.py now builds its .<config>.mcp-cache-reset.json marker through the same helper with no behavior change. prompt.py publishes .<config>.skills-cache-reset.json via publish_skills_cache_reset(user_id=...) and checks the marker on every cache lookup (global cache, per-config cache, prompt-section LRU, user scoped entries), calling _invalidate_enabled_skills_cache() or invalidate_user_skill_cache(). The publishing process remembers its own generation so it does not retire the caches it just rebuilt. Every mutation tail in routers/skills.py (install/upload, edit, delete, rollback, enable/disable, reload) refreshes locally and then publishes the marker off the event loop; /skills/reload answers scope=shared_config, or scope=process when no extensions config path resolves, and a marker publication failure is a 500 so a 200 never means only one process was refreshed. Docs: README, backend/docs/API.md, skills/config/mcp/gateway AGENTS.md, and frontend skills.mdx (en/zh; also fixes the reference to a non-existent skills/loader.py) describe the shared invalidation and drop the per-Pod guidance. CHANGELOG entry under Unreleased / Fixed. * docs(changelog): link the skills cache reset entry to #6495 * fix(skills): drain the reload tail and keep marker failures out of 404 Two review follow-ups on the shared skills cache reset (#6495). POST /skills/reload awaited the local refresh and the marker publication unshielded. CancelledError is a BaseException, so a caller that disconnected between the two slipped past the `except Exception -> 500` mapping, surfaced no error, and left every peer replica serving the old skill set -- the one outcome the operator hook for external mount writes exists to prevent. The refresh and the publication now run as one `await_drained(_drain_skill_mutation("reload", ...))` tail, the same discipline the install/edit/delete/rollback/toggle tails already use; the exception -> 500 mapping and the scope response are unchanged. Publication errors could surface as 404. resolve_shared_config_path() only swallows the resolution-time FileNotFoundError; a config directory that vanishes between resolution and the lock file / atomic replace still raised FileNotFoundError from the marker write, and the install, edit, delete and rollback handlers mapped that to HTTP 404 with the raw OS error text for a skill that had in fact been applied. publish_skills_cache_reset() now re-raises OSError (and a misconfigured project root's ValueError) as SkillCacheResetPublishError, a RuntimeError that no handler maps to 404/400, so every handler reports it through its generic 500 branch; the message names only the exception type, never a server path. The install handler gains an explicit clause that pins that contract beside its 404 mapping. Tests: cancelling reload after the refresh still publishes the marker; a FileNotFoundError from the marker write yields 500 (not 404) for install, edit, delete and rollback, 500 for the public toggle and the generic reload error; the wrapper type is not an OSError.
65 lines
2.5 KiB
Python
65 lines
2.5 KiB
Python
"""Regression anchor: the admin skills reload endpoint must not block ASGI."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from app.gateway.routers import skills as skills_router
|
|
from deerflow.agents.lead_agent import prompt as prompt_module
|
|
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
|
|
|
|
pytestmark = pytest.mark.asyncio
|
|
|
|
|
|
def _seed_skill(skills_root: Path) -> None:
|
|
skill_dir = skills_root / "public" / "reload-anchor"
|
|
skill_dir.mkdir(parents=True, exist_ok=True)
|
|
(skill_dir / "SKILL.md").write_text(
|
|
"---\nname: reload-anchor\ndescription: blocking IO regression anchor\n---\n# Reload anchor\n",
|
|
encoding="utf-8",
|
|
)
|
|
|
|
|
|
async def test_reload_skills_offloads_directory_scan(tmp_path: Path, monkeypatch) -> None:
|
|
await asyncio.to_thread(_seed_skill, tmp_path)
|
|
storage = await asyncio.to_thread(LocalSkillStorage, host_path=str(tmp_path))
|
|
|
|
async def _noop_admin(_request, **_kwargs) -> None:
|
|
return None
|
|
|
|
monkeypatch.setattr(skills_router, "require_admin_user", _noop_admin)
|
|
monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", lambda **_kwargs: storage)
|
|
monkeypatch.setattr(prompt_module, "resolve_shared_config_path", lambda: None)
|
|
|
|
response = await skills_router.reload_skills(request=None)
|
|
|
|
assert response.success is True
|
|
assert response.scope == "process"
|
|
|
|
|
|
async def test_reload_skills_publishes_shared_marker_off_the_loop(tmp_path: Path, monkeypatch) -> None:
|
|
"""Resolving the shared config path and writing the reset marker are filesystem work."""
|
|
await asyncio.to_thread(_seed_skill, tmp_path)
|
|
storage = await asyncio.to_thread(LocalSkillStorage, host_path=str(tmp_path))
|
|
config_path = tmp_path / "extensions_config.json"
|
|
await asyncio.to_thread(config_path.write_text, '{"mcpServers": {}, "skills": {}}', encoding="utf-8")
|
|
monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(config_path))
|
|
|
|
async def _noop_admin(_request, **_kwargs) -> None:
|
|
return None
|
|
|
|
monkeypatch.setattr(skills_router, "require_admin_user", _noop_admin)
|
|
monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", lambda **_kwargs: storage)
|
|
|
|
try:
|
|
response = await skills_router.reload_skills(request=None)
|
|
finally:
|
|
prompt_module._skills_cache_reset_tracker.reset()
|
|
|
|
assert response.success is True
|
|
assert response.scope == "shared_config"
|
|
assert await asyncio.to_thread(lambda: prompt_module.SKILLS_CACHE_RESET_MARKER.path_for(config_path).exists())
|