mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-25 07:28:07 +00:00
fix(memory): follow config reload in get_memory_config (#4216)
* fix(memory): follow config reload in get_memory_config get_memory_config() returned the _memory_config singleton directly, and that singleton is only refreshed as a side effect of get_app_config() reloading (via _apply_singleton_configs -> load_memory_config_from_dict). A reader that reaches memory config without going through get_app_config() first -- e.g. the agent factory deciding whether to bind the memory tools via `feat.memory_config or get_memory_config()` -- therefore saw a stale memory.mode after a config.yaml edit, even though memory.* is documented as hot-reloadable. The agent kept the old mode until some unrelated get_app_config() call happened to refresh the singleton. Trigger the same signature-checked reload inside get_memory_config() so the singleton follows the config file, mirroring how the other config getters already resolve through get_app_config(). When no config file is on disk (tests, or a minimal deployment running purely on defaults / set_memory_config) get_app_config() raises FileNotFoundError; swallow it and fall back to the in-memory singleton so the accessor is never more fragile than before. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * test(memory-config): pin malformed-config behavior for get_memory_config Address review feedback: the narrow except FileNotFoundError means a config file that exists but fails validation surfaces through get_memory_config() rather than being swallowed, consistent with every other unguarded get_app_config() caller. Add a regression test pinning that a malformed edit raises ValidationError and leaves the last-good singleton intact (not half-applied), and document the contract on the guard. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> --------- Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
This commit is contained in:
parent
92c8f2f03b
commit
cd0f1e2212
@ -121,7 +121,31 @@ _memory_config: MemoryConfig = MemoryConfig()
|
||||
|
||||
|
||||
def get_memory_config() -> MemoryConfig:
|
||||
"""Get the current memory configuration."""
|
||||
"""Get the current memory configuration.
|
||||
|
||||
``_memory_config`` is only refreshed as a side effect of ``get_app_config()``
|
||||
reloading (via ``_apply_singleton_configs`` -> ``load_memory_config_from_dict``).
|
||||
A reader that reaches memory config without going through ``get_app_config()``
|
||||
first -- e.g. the agent factory deciding whether to bind the memory tools --
|
||||
would otherwise see a stale ``memory.mode`` after a ``config.yaml`` edit, even
|
||||
though ``memory.*`` is documented as hot-reloadable. Trigger the same
|
||||
signature-checked reload here so the singleton follows the config file.
|
||||
|
||||
If no config file is on disk (tests, or a minimal deployment running purely
|
||||
on defaults / ``set_memory_config``), there is nothing to reload from, so we
|
||||
keep the pre-existing behavior of returning the in-memory singleton instead of
|
||||
becoming more fragile than before. The catch is deliberately narrow: a config
|
||||
file that exists but fails to load (malformed YAML, failed validation) is a
|
||||
real error and is surfaced here just as it is at every other unguarded
|
||||
``get_app_config()`` call site, leaving the last-good singleton untouched.
|
||||
"""
|
||||
# Lazy import: app_config imports this module, so a top-level import cycles.
|
||||
from .app_config import get_app_config
|
||||
|
||||
try:
|
||||
get_app_config()
|
||||
except FileNotFoundError:
|
||||
pass
|
||||
return _memory_config
|
||||
|
||||
|
||||
|
||||
@ -530,3 +530,75 @@ def test_get_app_config_does_not_mutate_singletons_when_reload_validation_fails(
|
||||
assert get_store() is initial_store
|
||||
finally:
|
||||
_reset_config_singletons()
|
||||
|
||||
|
||||
def test_get_memory_config_follows_config_file_reload(tmp_path, monkeypatch):
|
||||
"""get_memory_config() picks up a config.yaml memory.mode edit on its own (#4204).
|
||||
|
||||
``_memory_config`` used to refresh only as a side effect of ``get_app_config()``,
|
||||
so a reader reaching memory config directly (e.g. the agent factory deciding
|
||||
whether to bind the memory tools) saw a stale mode after a config edit even
|
||||
though ``memory.*`` is documented as hot-reloadable.
|
||||
"""
|
||||
config_path = tmp_path / "config.yaml"
|
||||
extensions_path = tmp_path / "extensions_config.json"
|
||||
_write_extensions_config(extensions_path)
|
||||
_write_config_with_sections(config_path, {"memory": {"enabled": True, "mode": "middleware"}})
|
||||
|
||||
monkeypatch.setenv("DEER_FLOW_CONFIG_PATH", str(config_path))
|
||||
monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(extensions_path))
|
||||
_reset_config_singletons()
|
||||
|
||||
try:
|
||||
assert get_memory_config().mode == "middleware"
|
||||
|
||||
_write_config_with_sections(config_path, {"memory": {"enabled": True, "mode": "tool"}})
|
||||
next_mtime = config_path.stat().st_mtime + 5
|
||||
os.utime(config_path, (next_mtime, next_mtime))
|
||||
|
||||
# No explicit get_app_config() call: get_memory_config() must reflect the
|
||||
# new mode on its own.
|
||||
assert get_memory_config().mode == "tool"
|
||||
finally:
|
||||
_reset_config_singletons()
|
||||
|
||||
|
||||
def test_get_memory_config_surfaces_malformed_config_edit(tmp_path, monkeypatch):
|
||||
"""A malformed config edit surfaces through get_memory_config() rather than
|
||||
being swallowed.
|
||||
|
||||
The ``except FileNotFoundError`` guard is deliberately narrow: it covers only
|
||||
the genuinely-absent file (tests, or a defaults-only deployment) where there
|
||||
is nothing to reload from. A file that exists but fails validation is a real
|
||||
error and propagates -- consistent with every other unguarded
|
||||
``get_app_config()`` caller -- and the last-good singleton is left intact
|
||||
rather than cleared or half-applied.
|
||||
"""
|
||||
import deerflow.config.memory_config as memory_config_module
|
||||
|
||||
config_path = tmp_path / "config.yaml"
|
||||
extensions_path = tmp_path / "extensions_config.json"
|
||||
_write_extensions_config(extensions_path)
|
||||
_write_config_with_sections(config_path, {"memory": {"enabled": True, "mode": "middleware"}})
|
||||
|
||||
monkeypatch.setenv("DEER_FLOW_CONFIG_PATH", str(config_path))
|
||||
monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(extensions_path))
|
||||
_reset_config_singletons()
|
||||
|
||||
try:
|
||||
assert get_memory_config().mode == "middleware"
|
||||
|
||||
# Same file, now invalid: `title: false` fails AppConfig validation while
|
||||
# the edit also flips memory.mode -- so a half-applied reload would leak
|
||||
# "tool" into the singleton.
|
||||
_write_config_with_sections(config_path, {"memory": {"enabled": True, "mode": "tool"}, "title": False})
|
||||
next_mtime = config_path.stat().st_mtime + 5
|
||||
os.utime(config_path, (next_mtime, next_mtime))
|
||||
|
||||
with pytest.raises(ValidationError):
|
||||
get_memory_config()
|
||||
|
||||
# Failed reload: the last-good singleton stays "middleware", not "tool".
|
||||
assert memory_config_module._memory_config.mode == "middleware"
|
||||
finally:
|
||||
_reset_config_singletons()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user