From f8738d1e3e42f685706f379b075b4156e73268a0 Mon Sep 17 00:00:00 2001 From: greatmengqi Date: Thu, 16 Apr 2026 23:57:11 +0800 Subject: [PATCH] refactor(config): DeerFlowClient captures config in constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- backend/packages/harness/deerflow/client.py | 42 +++++++--- backend/tests/test_client.py | 62 +++++++-------- backend/tests/test_client_multi_isolation.py | 82 ++++++++++++++++++++ 3 files changed, 144 insertions(+), 42 deletions(-) create mode 100644 backend/tests/test_client_multi_isolation.py diff --git a/backend/packages/harness/deerflow/client.py b/backend/packages/harness/deerflow/client.py index 1308e51af..bb52a5931 100644 --- a/backend/packages/harness/deerflow/client.py +++ b/backend/packages/harness/deerflow/client.py @@ -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, diff --git a/backend/tests/test_client.py b/backend/tests/test_client.py index f87df7e9f..b01502817 100644 --- a/backend/tests/test_client.py +++ b/backend/tests/test_client.py @@ -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) diff --git a/backend/tests/test_client_multi_isolation.py b/backend/tests/test_client_multi_isolation.py new file mode 100644 index 000000000..39ede0a78 --- /dev/null +++ b/backend/tests/test_client_multi_isolation.py @@ -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"}