mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-04-25 11:18:22 +00:00
refactor(config): DeerFlowClient captures config in constructor
Phase 2 Task P2-3 (Category H): the client now owns its AppConfig for its lifetime via self._app_config, instead of routing through the process-global AppConfig.current() / AppConfig.init() path. Constructor resolution priority: 1. Explicit config= parameter (new; enables multi-client isolation) 2. Explicit config_path= parameter (reads file directly, no process-global write) 3. AppConfig.current() (legacy fallback; removed in P2-10) All 7 method-level AppConfig.current() reads are replaced with self._app_config. _reload_config() no longer mutates the process-global — only the client's own config is rebuilt, so concurrent clients' configs remain isolated. New test: test_client_multi_isolation.py pins the key invariant — two clients with different configs coexist without contention. Before this refactor the second client's init() would clobber the first. Test updates: ~8 test_client.py sites that formerly relied on patch.object(AppConfig, "current", ...) now set client._app_config directly to the mock. test_custom_config_path no longer asserts AppConfig.init() was called because the client no longer touches it. All 141 test_client.py + 46 test_client_e2e.py/multi_isolation tests pass.
This commit is contained in:
parent
70323e052a
commit
f8738d1e3e
@ -117,6 +117,7 @@ class DeerFlowClient:
|
||||
config_path: str | None = None,
|
||||
checkpointer=None,
|
||||
*,
|
||||
config: AppConfig | None = None,
|
||||
model_name: str | None = None,
|
||||
thinking_enabled: bool = True,
|
||||
subagent_enabled: bool = False,
|
||||
@ -131,9 +132,14 @@ class DeerFlowClient:
|
||||
|
||||
Args:
|
||||
config_path: Path to config.yaml. Uses default resolution if None.
|
||||
Ignored when ``config`` is provided.
|
||||
checkpointer: LangGraph checkpointer instance for state persistence.
|
||||
Required for multi-turn conversations on the same thread_id.
|
||||
Without a checkpointer, each call is stateless.
|
||||
config: Optional pre-constructed AppConfig. When provided, it takes
|
||||
precedence over ``config_path`` and no file is read. Enables
|
||||
multi-client isolation: two clients with different configs can
|
||||
coexist in the same process without touching process-global state.
|
||||
model_name: Override the default model name from config.
|
||||
thinking_enabled: Enable model's extended thinking.
|
||||
subagent_enabled: Enable subagent delegation.
|
||||
@ -142,9 +148,19 @@ class DeerFlowClient:
|
||||
available_skills: Optional set of skill names to make available. If None (default), all scanned skills are available.
|
||||
middlewares: Optional list of custom middlewares to inject into the agent.
|
||||
"""
|
||||
if config_path is not None:
|
||||
AppConfig.init(AppConfig.from_file(config_path))
|
||||
self._app_config = AppConfig.current()
|
||||
# Constructor-captured config: the client owns its AppConfig for its lifetime.
|
||||
# Multiple clients with different configs do not contend.
|
||||
#
|
||||
# Priority: explicit ``config=`` > explicit ``config_path=`` > AppConfig.current().
|
||||
# The third tier preserves backward compatibility with callers that relied on
|
||||
# the process-global (tests via the conftest autouse fixture). After P2-10
|
||||
# removes AppConfig.current(), this fallback will require an explicit choice.
|
||||
if config is not None:
|
||||
self._app_config = config
|
||||
elif config_path is not None:
|
||||
self._app_config = AppConfig.from_file(config_path)
|
||||
else:
|
||||
self._app_config = AppConfig.current()
|
||||
|
||||
if agent_name is not None and not AGENT_NAME_PATTERN.match(agent_name):
|
||||
raise ValueError(f"Invalid agent name '{agent_name}'. Must match pattern: {AGENT_NAME_PATTERN.pattern}")
|
||||
@ -173,9 +189,13 @@ class DeerFlowClient:
|
||||
self._agent_config_key = None
|
||||
|
||||
def _reload_config(self) -> None:
|
||||
"""Reload config from file and refresh the cached reference."""
|
||||
AppConfig.init(AppConfig.from_file())
|
||||
self._app_config = AppConfig.current()
|
||||
"""Reload config from file and refresh the cached reference.
|
||||
|
||||
Only the client's own ``_app_config`` is rebuilt. Other clients
|
||||
and the process-global are untouched, so multi-client coexistence
|
||||
survives reload.
|
||||
"""
|
||||
self._app_config = AppConfig.from_file()
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Internal helpers
|
||||
@ -821,7 +841,7 @@ class DeerFlowClient:
|
||||
Dict with "mcp_servers" key mapping server name to config,
|
||||
matching the Gateway API ``McpConfigResponse`` schema.
|
||||
"""
|
||||
ext = AppConfig.current().extensions
|
||||
ext = self._app_config.extensions
|
||||
return {"mcp_servers": {name: server.model_dump() for name, server in ext.mcp_servers.items()}}
|
||||
|
||||
def update_mcp_config(self, mcp_servers: dict[str, dict]) -> dict:
|
||||
@ -844,7 +864,7 @@ class DeerFlowClient:
|
||||
if config_path is None:
|
||||
raise FileNotFoundError("Cannot locate extensions_config.json. Set DEER_FLOW_EXTENSIONS_CONFIG_PATH or ensure it exists in the project root.")
|
||||
|
||||
current_ext = AppConfig.current().extensions
|
||||
current_ext = self._app_config.extensions
|
||||
|
||||
config_data = {
|
||||
"mcpServers": mcp_servers,
|
||||
@ -856,7 +876,7 @@ class DeerFlowClient:
|
||||
self._agent = None
|
||||
self._agent_config_key = None
|
||||
self._reload_config()
|
||||
reloaded = AppConfig.current().extensions
|
||||
reloaded = self._app_config.extensions
|
||||
return {"mcp_servers": {name: server.model_dump() for name, server in reloaded.mcp_servers.items()}}
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
@ -910,7 +930,7 @@ class DeerFlowClient:
|
||||
if config_path is None:
|
||||
raise FileNotFoundError("Cannot locate extensions_config.json. Set DEER_FLOW_EXTENSIONS_CONFIG_PATH or ensure it exists in the project root.")
|
||||
|
||||
ext = AppConfig.current().extensions
|
||||
ext = self._app_config.extensions
|
||||
ext.skills[name] = SkillStateConfig(enabled=enabled)
|
||||
|
||||
config_data = {
|
||||
@ -1005,7 +1025,7 @@ class DeerFlowClient:
|
||||
Returns:
|
||||
Memory config dict.
|
||||
"""
|
||||
config = AppConfig.current().memory
|
||||
config = self._app_config.memory
|
||||
return {
|
||||
"enabled": config.enabled,
|
||||
"storage_path": config.storage_path,
|
||||
|
||||
@ -85,14 +85,12 @@ class TestClientInit:
|
||||
DeerFlowClient(agent_name="../path/traversal")
|
||||
|
||||
def test_custom_config_path(self, mock_app_config):
|
||||
with (
|
||||
patch.object(AppConfig, "from_file", return_value=mock_app_config) as mock_from_file,
|
||||
patch.object(AppConfig, "init") as mock_init,
|
||||
patch.object(AppConfig, "current", return_value=mock_app_config),
|
||||
):
|
||||
DeerFlowClient(config_path="/tmp/custom.yaml")
|
||||
# Phase 2: DeerFlowClient stores config locally in self._app_config
|
||||
# rather than touching AppConfig.init() / process-global state.
|
||||
with patch.object(AppConfig, "from_file", return_value=mock_app_config) as mock_from_file:
|
||||
client = DeerFlowClient(config_path="/tmp/custom.yaml")
|
||||
mock_from_file.assert_called_once_with("/tmp/custom.yaml")
|
||||
mock_init.assert_called_once_with(mock_app_config)
|
||||
assert client._app_config is mock_app_config
|
||||
|
||||
def test_checkpointer_stored(self, mock_app_config):
|
||||
cp = MagicMock()
|
||||
@ -1092,8 +1090,9 @@ class TestMcpConfig:
|
||||
ext_config = MagicMock()
|
||||
ext_config.mcp_servers = {"github": server}
|
||||
|
||||
with patch.object(AppConfig, "current", return_value=MagicMock(extensions=ext_config)):
|
||||
result = client.get_mcp_config()
|
||||
# Phase 2: client reads from self._app_config, not AppConfig.current()
|
||||
client._app_config = MagicMock(extensions=ext_config)
|
||||
result = client.get_mcp_config()
|
||||
|
||||
assert "mcp_servers" in result
|
||||
assert "github" in result["mcp_servers"]
|
||||
@ -1117,8 +1116,8 @@ class TestMcpConfig:
|
||||
# Pre-set agent to verify it gets invalidated
|
||||
client._agent = MagicMock()
|
||||
|
||||
# Set initial AppConfig with current extensions
|
||||
AppConfig.init(MagicMock(extensions=current_config))
|
||||
# Phase 2: initial config is stored on the client, not process-global
|
||||
client._app_config = MagicMock(extensions=current_config)
|
||||
|
||||
with (
|
||||
patch("deerflow.client.ExtensionsConfig.resolve_config_path", return_value=tmp_path),
|
||||
@ -1180,11 +1179,11 @@ class TestSkillsManagement:
|
||||
try:
|
||||
# Pre-set agent to verify it gets invalidated
|
||||
client._agent = MagicMock()
|
||||
client._app_config = MagicMock(extensions=ext_config)
|
||||
|
||||
with (
|
||||
patch("deerflow.skills.loader.load_skills", side_effect=[[skill], [updated_skill]]),
|
||||
patch("deerflow.client.ExtensionsConfig.resolve_config_path", return_value=tmp_path),
|
||||
patch.object(AppConfig, "current", return_value=MagicMock(extensions=ext_config)),
|
||||
patch("deerflow.config.app_config.AppConfig.from_file", return_value=MagicMock()),
|
||||
):
|
||||
result = client.update_skill("test-skill", enabled=False)
|
||||
@ -1331,8 +1330,9 @@ class TestMemoryManagement:
|
||||
app_cfg = MagicMock()
|
||||
app_cfg.memory = mem_config
|
||||
|
||||
with patch.object(AppConfig, "current", return_value=app_cfg):
|
||||
result = client.get_memory_config()
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
result = client.get_memory_config()
|
||||
|
||||
assert result["enabled"] is True
|
||||
assert result["max_facts"] == 100
|
||||
@ -1351,10 +1351,9 @@ class TestMemoryManagement:
|
||||
app_cfg.memory = mem_config
|
||||
data = {"version": "1.0", "facts": []}
|
||||
|
||||
with (
|
||||
patch.object(AppConfig, "current", return_value=app_cfg),
|
||||
patch("deerflow.agents.memory.updater.get_memory_data", return_value=data),
|
||||
):
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
with patch("deerflow.agents.memory.updater.get_memory_data", return_value=data):
|
||||
result = client.get_memory_status()
|
||||
|
||||
assert "config" in result
|
||||
@ -2031,10 +2030,9 @@ class TestScenarioMemoryWorkflow:
|
||||
|
||||
app_cfg = MagicMock()
|
||||
app_cfg.memory = config
|
||||
with (
|
||||
patch.object(AppConfig, "current", return_value=app_cfg),
|
||||
patch("deerflow.agents.memory.updater.get_memory_data", return_value=updated_data),
|
||||
):
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
with patch("deerflow.agents.memory.updater.get_memory_data", return_value=updated_data):
|
||||
status = client.get_memory_status()
|
||||
assert status["config"]["enabled"] is True
|
||||
assert len(status["data"]["facts"]) == 2
|
||||
@ -2317,8 +2315,9 @@ class TestGatewayConformance:
|
||||
ext_config = MagicMock()
|
||||
ext_config.mcp_servers = {"test": server}
|
||||
|
||||
with patch.object(AppConfig, "current", return_value=MagicMock(extensions=ext_config)):
|
||||
result = client.get_mcp_config()
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = MagicMock(extensions=ext_config)
|
||||
result = client.get_mcp_config()
|
||||
|
||||
parsed = McpConfigResponse(**result)
|
||||
assert "test" in parsed.mcp_servers
|
||||
@ -2342,8 +2341,9 @@ class TestGatewayConformance:
|
||||
config_file = tmp_path / "extensions_config.json"
|
||||
config_file.write_text("{}")
|
||||
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = MagicMock(extensions=ext_config)
|
||||
with (
|
||||
patch.object(AppConfig, "current", return_value=MagicMock(extensions=ext_config)),
|
||||
patch("deerflow.client.ExtensionsConfig.resolve_config_path", return_value=config_file),
|
||||
patch("deerflow.config.app_config.AppConfig.from_file", return_value=MagicMock(extensions=ext_config)),
|
||||
):
|
||||
@ -2379,8 +2379,9 @@ class TestGatewayConformance:
|
||||
app_cfg = MagicMock()
|
||||
app_cfg.memory = mem_cfg
|
||||
|
||||
with patch.object(AppConfig, "current", return_value=app_cfg):
|
||||
result = client.get_memory_config()
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
result = client.get_memory_config()
|
||||
|
||||
parsed = MemoryConfigResponse(**result)
|
||||
assert parsed.enabled is True
|
||||
@ -2414,10 +2415,9 @@ class TestGatewayConformance:
|
||||
"facts": [],
|
||||
}
|
||||
|
||||
with (
|
||||
patch.object(AppConfig, "current", return_value=app_cfg),
|
||||
patch("deerflow.agents.memory.updater.get_memory_data", return_value=memory_data),
|
||||
):
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
with patch("deerflow.agents.memory.updater.get_memory_data", return_value=memory_data):
|
||||
result = client.get_memory_status()
|
||||
|
||||
parsed = MemoryStatusResponse(**result)
|
||||
|
||||
82
backend/tests/test_client_multi_isolation.py
Normal file
82
backend/tests/test_client_multi_isolation.py
Normal file
@ -0,0 +1,82 @@
|
||||
"""Multi-client isolation regression test.
|
||||
|
||||
Phase 2 Task P2-3: ``DeerFlowClient`` now captures its ``AppConfig`` in the
|
||||
constructor instead of going through process-global ``AppConfig.current()``.
|
||||
This test pins the resulting invariant: two clients with different configs
|
||||
can coexist without contending over shared state.
|
||||
|
||||
Before P2-3, the shared ``AppConfig._global`` caused the second client's
|
||||
``init()`` to clobber the first client's config.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from deerflow.client import DeerFlowClient
|
||||
from deerflow.config.app_config import AppConfig
|
||||
from deerflow.config.memory_config import MemoryConfig
|
||||
from deerflow.config.sandbox_config import SandboxConfig
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def disable_agent_creation(monkeypatch):
|
||||
"""Prevent lazy agent creation — we only care about config access."""
|
||||
monkeypatch.setattr(DeerFlowClient, "_get_or_create_agent", MagicMock(), raising=False)
|
||||
|
||||
|
||||
def test_two_clients_do_not_clobber_each_other(disable_agent_creation):
|
||||
"""Two clients with distinct configs keep their own AppConfig."""
|
||||
cfg_a = AppConfig(
|
||||
sandbox=SandboxConfig(use="test"),
|
||||
memory=MemoryConfig(enabled=True),
|
||||
)
|
||||
cfg_b = AppConfig(
|
||||
sandbox=SandboxConfig(use="test"),
|
||||
memory=MemoryConfig(enabled=False),
|
||||
)
|
||||
|
||||
client_a = DeerFlowClient(config=cfg_a)
|
||||
client_b = DeerFlowClient(config=cfg_b)
|
||||
|
||||
# Identity: each client retains its own instance, not a shared ref
|
||||
assert client_a._app_config is cfg_a
|
||||
assert client_b._app_config is cfg_b
|
||||
|
||||
# Semantic: memory flag differs
|
||||
assert client_a._app_config.memory.enabled is True
|
||||
assert client_b._app_config.memory.enabled is False
|
||||
|
||||
|
||||
def test_client_config_precedes_path(disable_agent_creation, tmp_path):
|
||||
"""When both config= and config_path= are given, config= wins."""
|
||||
cfg = AppConfig(sandbox=SandboxConfig(use="test"), log_level="debug")
|
||||
|
||||
# config_path points at a file that doesn't exist — proves it's unused
|
||||
bogus_path = str(tmp_path / "nope.yaml")
|
||||
client = DeerFlowClient(config_path=bogus_path, config=cfg)
|
||||
|
||||
assert client._app_config is cfg
|
||||
assert client._app_config.log_level == "debug"
|
||||
|
||||
|
||||
def test_multi_client_gateway_dict_returns_distinct(disable_agent_creation):
|
||||
"""get_mcp_config() reads from self._app_config, not process-global."""
|
||||
from deerflow.config.extensions_config import ExtensionsConfig, McpServerConfig
|
||||
|
||||
ext_a = ExtensionsConfig(mcp_servers={"server-a": McpServerConfig(enabled=True)})
|
||||
ext_b = ExtensionsConfig(mcp_servers={"server-b": McpServerConfig(enabled=True)})
|
||||
|
||||
cfg_a = AppConfig(sandbox=SandboxConfig(use="test"), extensions=ext_a)
|
||||
cfg_b = AppConfig(sandbox=SandboxConfig(use="test"), extensions=ext_b)
|
||||
|
||||
client_a = DeerFlowClient(config=cfg_a)
|
||||
client_b = DeerFlowClient(config=cfg_b)
|
||||
|
||||
servers_a = client_a.get_mcp_config()["mcp_servers"]
|
||||
servers_b = client_b.get_mcp_config()["mcp_servers"]
|
||||
|
||||
assert set(servers_a.keys()) == {"server-a"}
|
||||
assert set(servers_b.keys()) == {"server-b"}
|
||||
Loading…
x
Reference in New Issue
Block a user