From 013dca6352b639447f5c885b03c431fa9782598f Mon Sep 17 00:00:00 2001 From: wutongyuonce <147830929+wutongyuonce@users.noreply.github.com> Date: Tue, 25 Aug 2026 19:46:04 +0800 Subject: [PATCH] fix(skills): refresh user storage after config reload (#4972) Co-authored-by: Willem Jiang --- .../deerflow/skills/storage/__init__.py | 28 +++++++++++-------- backend/tests/test_skill_storage_lifecycle.py | 19 +++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/backend/packages/harness/deerflow/skills/storage/__init__.py b/backend/packages/harness/deerflow/skills/storage/__init__.py index 1d8940140..0ecd51528 100644 --- a/backend/packages/harness/deerflow/skills/storage/__init__.py +++ b/backend/packages/harness/deerflow/skills/storage/__init__.py @@ -29,7 +29,7 @@ _MAX_USER_SCOPED_STORAGES = 64 # OrderedDict so that LRU eviction can remove the least-recently-used entry # via ``move_to_end`` + ``popitem(last=False)`` when the cache exceeds # ``_MAX_USER_SCOPED_STORAGES``. -_user_scoped_storages: OrderedDict[str, UserScopedSkillStorage] = OrderedDict() +_user_scoped_storages: OrderedDict[str, tuple[object, UserScopedSkillStorage]] = OrderedDict() _user_scoped_storage_lock = threading.Lock() @@ -113,34 +113,40 @@ def get_or_new_user_skill_storage(user_id: str, **kwargs) -> SkillStorage: are safely bucketed before reaching :class:`UserScopedSkillStorage`, which calls :func:`_validate_user_id` internally. - Instances are cached by the *normalised* ``user_id`` with double-check - locking to prevent concurrent creation races. When the cache exceeds - ``_MAX_USER_SCOPED_STORAGES``, the least-recently-accessed entry is - evicted (true LRU, not FIFO). + Instances are cached by the *normalised* ``user_id`` and current + ``AppConfig`` identity with double-check locking to prevent concurrent + creation races. When the cache exceeds ``_MAX_USER_SCOPED_STORAGES``, the + least-recently-accessed entry is evicted (true LRU, not FIFO). """ + from deerflow.config import get_app_config from deerflow.config.paths import make_safe_user_id safe_id = make_safe_user_id(user_id) + app_config = kwargs.get("app_config") + if app_config is None: + app_config = get_app_config() + kwargs["app_config"] = app_config # Always acquire lock so move_to_end is safe — makes this a true LRU # cache instead of FIFO. The overhead is negligible since dict ops are # fast and this function is called once per agent-creation cycle. with _user_scoped_storage_lock: cached = _user_scoped_storages.get(safe_id) - if cached is not None: + if cached is not None and cached[0] is app_config: _user_scoped_storages.move_to_end(safe_id) - return cached + return cached[1] - cached = UserScopedSkillStorage(safe_id, **kwargs) - _user_scoped_storages[safe_id] = cached + storage = UserScopedSkillStorage(safe_id, **kwargs) + _user_scoped_storages[safe_id] = (app_config, storage) + _user_scoped_storages.move_to_end(safe_id) # Evict least-recently-used entry if cache exceeds the ceiling. # Since we just moved the current user_id to the end, popitem(last=False) # will evict the oldest/least-recently-accessed entry (never the # one we just created). while len(_user_scoped_storages) > _MAX_USER_SCOPED_STORAGES: - evicted_key, evicted_val = _user_scoped_storages.popitem(last=False) + evicted_key, _ = _user_scoped_storages.popitem(last=False) logger.info("Evicted user-scoped skill storage for safe_id=%s (cache ceiling %d)", evicted_key, _MAX_USER_SCOPED_STORAGES) - return cached + return storage def user_should_see_legacy_skills(user_id: str, **kwargs) -> bool: diff --git a/backend/tests/test_skill_storage_lifecycle.py b/backend/tests/test_skill_storage_lifecycle.py index cab3966c8..926c39543 100644 --- a/backend/tests/test_skill_storage_lifecycle.py +++ b/backend/tests/test_skill_storage_lifecycle.py @@ -284,6 +284,25 @@ def test_different_users_get_different_storages(monkeypatch): skill_storage.reset_skill_storage() +def test_user_storage_is_rebuilt_when_app_config_changes(monkeypatch): + """A hot-reloaded config must not reuse storage bound to the old paths.""" + skill_storage.reset_skill_storage() + SlowUserSkillStorage.instances_created = 0 + _patch_user_storage_resolution(monkeypatch) + + first_config = _AppConfig() + second_config = _AppConfig() + first = skill_storage.get_or_new_user_skill_storage("alice", app_config=first_config) + second = skill_storage.get_or_new_user_skill_storage("alice", app_config=second_config) + + try: + assert second is not first + assert skill_storage.get_or_new_user_skill_storage("alice", app_config=second_config) is second + assert SlowUserSkillStorage.instances_created == 2 + finally: + skill_storage.reset_skill_storage() + + def test_reset_user_skill_storage_only_clears_target_user(monkeypatch): """Resetting alice's storage must not invalidate bob's.""" skill_storage.reset_skill_storage()