From 65ef398bb8aa7b815d9cdd8d9b494160bbe55bd5 Mon Sep 17 00:00:00 2001 From: ShenAC-SAC <142667174+ShenAC-SAC@users.noreply.github.com> Date: Fri, 8 May 2026 10:59:58 +0800 Subject: [PATCH] refactor(config): remove ACP singleton, expose acp_agents on AppConfig Drops the legacy load_acp_config_from_dict / get_acp_agents singleton API. acp_agents is now sourced exclusively from AppConfig.acp_agents, eliminating the dual source-of-truth between the global singleton and the validated config. - acp_config.py keeps only ACPAgentConfig pydantic model - app_config.py removes _validate_acp_agents (handled by pydantic) - tools.py reads config.acp_agents directly - lead_agent/prompt._build_acp_section drops singleton fallback - tests migrated to construct AppConfig with acp_agents addresses #2687 item 2 --- .../deerflow/agents/lead_agent/prompt.py | 11 +- .../harness/deerflow/config/acp_config.py | 30 ----- .../harness/deerflow/config/app_config.py | 18 +-- .../packages/harness/deerflow/tools/tools.py | 7 +- backend/tests/test_acp_config.py | 114 ++++++------------ backend/tests/test_app_config_reload.py | 2 - backend/tests/test_invoke_acp_agent_tool.py | 19 +-- backend/tests/test_lead_agent_prompt.py | 5 - 8 files changed, 45 insertions(+), 161 deletions(-) diff --git a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py index b255e962d4..0c7fb9dba8 100644 --- a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py +++ b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py @@ -719,16 +719,7 @@ def get_deferred_tools_prompt_section(*, app_config: AppConfig | None = None) -> def _build_acp_section(*, app_config: AppConfig | None = None) -> str: """Build the ACP agent prompt section, only if ACP agents are configured.""" - if app_config is None: - try: - from deerflow.config.acp_config import get_acp_agents - - agents = get_acp_agents() - except Exception: - return "" - else: - agents = getattr(app_config, "acp_agents", {}) or {} - + agents = getattr(app_config, "acp_agents", {}) if app_config is not None else {} if not agents: return "" diff --git a/backend/packages/harness/deerflow/config/acp_config.py b/backend/packages/harness/deerflow/config/acp_config.py index de4b1e89f4..36142404cb 100644 --- a/backend/packages/harness/deerflow/config/acp_config.py +++ b/backend/packages/harness/deerflow/config/acp_config.py @@ -1,12 +1,7 @@ """ACP (Agent Client Protocol) agent configuration loaded from config.yaml.""" -import logging -from collections.abc import Mapping - from pydantic import BaseModel, Field -logger = logging.getLogger(__name__) - class ACPAgentConfig(BaseModel): """Configuration for a single ACP-compatible agent.""" @@ -24,28 +19,3 @@ class ACPAgentConfig(BaseModel): "are denied — the agent must be configured to operate without requesting permissions." ), ) - - -_acp_agents: dict[str, ACPAgentConfig] = {} - - -def get_acp_agents() -> dict[str, ACPAgentConfig]: - """Get the currently configured ACP agents. - - Returns: - Mapping of agent name -> ACPAgentConfig. Empty dict if no ACP agents are configured. - """ - return _acp_agents - - -def load_acp_config_from_dict(config_dict: Mapping[str, Mapping[str, object]] | None) -> None: - """Load ACP agent configuration from a dictionary (typically from config.yaml). - - Args: - config_dict: Mapping of agent name -> config fields. - """ - global _acp_agents - if config_dict is None: - config_dict = {} - _acp_agents = {name: ACPAgentConfig(**cfg) for name, cfg in config_dict.items()} - logger.info("ACP config loaded: %d agent(s): %s", len(_acp_agents), list(_acp_agents.keys())) diff --git a/backend/packages/harness/deerflow/config/app_config.py b/backend/packages/harness/deerflow/config/app_config.py index d470d65586..3571a5d38f 100644 --- a/backend/packages/harness/deerflow/config/app_config.py +++ b/backend/packages/harness/deerflow/config/app_config.py @@ -1,6 +1,5 @@ import logging import os -from collections.abc import Mapping from contextvars import ContextVar from pathlib import Path from typing import Any, Self @@ -9,7 +8,7 @@ from dotenv import load_dotenv from pydantic import BaseModel, ConfigDict, Field -from deerflow.config.acp_config import ACPAgentConfig, load_acp_config_from_dict +from deerflow.config.acp_config import ACPAgentConfig from deerflow.config.agents_api_config import AgentsApiConfig, load_agents_api_config_from_dict from deerflow.config.checkpointer_config import CheckpointerConfig, load_checkpointer_config_from_dict from deerflow.config.database_config import DatabaseConfig @@ -169,21 +168,11 @@ def from_file(cls, config_path: str | None = None) -> Self: config_data["extensions"] = extensions_config.model_dump() result = cls.model_validate(config_data) - acp_agents = cls._validate_acp_agents(config_data.get("acp_agents", {})) - cls._apply_singleton_configs(result, acp_agents) + cls._apply_singleton_configs(result) return result @classmethod - def _validate_acp_agents( - cls, - config_data: Mapping[str, Mapping[str, object]] | None, - ) -> dict[str, ACPAgentConfig]: - if config_data is None: - config_data = {} - return {name: ACPAgentConfig(**cfg) for name, cfg in config_data.items()} - - @classmethod - def _apply_singleton_configs(cls, config: Self, acp_agents: dict[str, ACPAgentConfig]) -> None: + def _apply_singleton_configs(cls, config: Self) -> None: from deerflow.config.checkpointer_config import get_checkpointer_config previous_checkpointer_config = get_checkpointer_config() @@ -197,7 +186,6 @@ def _apply_singleton_configs(cls, config: Self, acp_agents: dict[str, ACPAgentCo load_guardrails_config_from_dict(config.guardrails.model_dump()) load_checkpointer_config_from_dict(config.checkpointer.model_dump() if config.checkpointer is not None else None) load_stream_bridge_config_from_dict(config.stream_bridge.model_dump() if config.stream_bridge is not None else None) - load_acp_config_from_dict({name: agent.model_dump() for name, agent in acp_agents.items()}) if previous_checkpointer_config != config.checkpointer: # These runtime singletons derive their backend from checkpointer config. diff --git a/backend/packages/harness/deerflow/tools/tools.py b/backend/packages/harness/deerflow/tools/tools.py index 14d93e65f8..45ebe21057 100644 --- a/backend/packages/harness/deerflow/tools/tools.py +++ b/backend/packages/harness/deerflow/tools/tools.py @@ -143,12 +143,7 @@ def get_available_tools( try: from deerflow.tools.builtins.invoke_acp_agent_tool import build_invoke_acp_agent_tool - if app_config is None: - from deerflow.config.acp_config import get_acp_agents - - acp_agents = get_acp_agents() - else: - acp_agents = getattr(config, "acp_agents", {}) or {} + acp_agents = getattr(config, "acp_agents", {}) or {} if acp_agents: acp_tools.append(build_invoke_acp_agent_tool(acp_agents)) logger.info(f"Including invoke_acp_agent tool ({len(acp_agents)} agent(s): {list(acp_agents.keys())})") diff --git a/backend/tests/test_acp_config.py b/backend/tests/test_acp_config.py index 16fbfad16c..8cb38b81bb 100644 --- a/backend/tests/test_acp_config.py +++ b/backend/tests/test_acp_config.py @@ -6,61 +6,10 @@ import yaml from pydantic import ValidationError -from deerflow.config.acp_config import ACPAgentConfig, get_acp_agents, load_acp_config_from_dict +from deerflow.config.acp_config import ACPAgentConfig from deerflow.config.app_config import AppConfig -def setup_function(): - """Reset ACP config before each test.""" - load_acp_config_from_dict({}) - - -def test_load_acp_config_sets_agents(): - load_acp_config_from_dict( - { - "claude_code": { - "command": "claude-code-acp", - "args": [], - "description": "Claude Code for coding tasks", - "model": None, - } - } - ) - agents = get_acp_agents() - assert "claude_code" in agents - assert agents["claude_code"].command == "claude-code-acp" - assert agents["claude_code"].description == "Claude Code for coding tasks" - assert agents["claude_code"].model is None - - -def test_load_acp_config_multiple_agents(): - load_acp_config_from_dict( - { - "claude_code": {"command": "claude-code-acp", "args": [], "description": "Claude Code"}, - "codex": {"command": "codex-acp", "args": ["--flag"], "description": "Codex CLI"}, - } - ) - agents = get_acp_agents() - assert len(agents) == 2 - assert agents["codex"].args == ["--flag"] - - -def test_load_acp_config_empty_clears_agents(): - load_acp_config_from_dict({"agent": {"command": "cmd", "args": [], "description": "desc"}}) - assert len(get_acp_agents()) == 1 - - load_acp_config_from_dict({}) - assert len(get_acp_agents()) == 0 - - -def test_load_acp_config_none_clears_agents(): - load_acp_config_from_dict({"agent": {"command": "cmd", "args": [], "description": "desc"}}) - assert len(get_acp_agents()) == 1 - - load_acp_config_from_dict(None) - assert get_acp_agents() == {} - - def test_acp_agent_config_defaults(): cfg = ACPAgentConfig(command="my-agent", description="My agent") assert cfg.args == [] @@ -79,21 +28,6 @@ def test_acp_agent_config_env_default_is_empty(): assert cfg.env == {} -def test_load_acp_config_preserves_env(): - load_acp_config_from_dict( - { - "codex": { - "command": "codex-acp", - "args": [], - "description": "Codex CLI", - "env": {"OPENAI_API_KEY": "$OPENAI_API_KEY", "FOO": "bar"}, - } - } - ) - cfg = get_acp_agents()["codex"] - assert cfg.env == {"OPENAI_API_KEY": "$OPENAI_API_KEY", "FOO": "bar"} - - def test_acp_agent_config_with_model(): cfg = ACPAgentConfig(command="my-agent", description="desc", model="claude-opus-4") assert cfg.model == "claude-opus-4" @@ -115,10 +49,40 @@ def test_acp_agent_config_missing_description_raises(): ACPAgentConfig(command="my-agent") -def test_get_acp_agents_returns_empty_by_default(): - """After clearing, should return empty dict.""" - load_acp_config_from_dict({}) - assert get_acp_agents() == {} +def test_app_config_parses_acp_agents(): + config = AppConfig.model_validate( + { + "sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider"}, + "models": [ + {"name": "test-model", "use": "langchain_openai:ChatOpenAI", "model": "gpt-test"}, + ], + "acp_agents": { + "claude_code": {"command": "claude-code-acp", "args": [], "description": "Claude Code"}, + "codex": { + "command": "codex-acp", + "args": ["--flag"], + "description": "Codex CLI", + "env": {"OPENAI_API_KEY": "$OPENAI_API_KEY"}, + }, + }, + } + ) + assert set(config.acp_agents) == {"claude_code", "codex"} + assert config.acp_agents["claude_code"].command == "claude-code-acp" + assert config.acp_agents["codex"].args == ["--flag"] + assert config.acp_agents["codex"].env == {"OPENAI_API_KEY": "$OPENAI_API_KEY"} + + +def test_app_config_acp_agents_default_empty(): + config = AppConfig.model_validate( + { + "sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider"}, + "models": [ + {"name": "test-model", "use": "langchain_openai:ChatOpenAI", "model": "gpt-test"}, + ], + } + ) + assert config.acp_agents == {} def test_app_config_reload_without_acp_agents_clears_previous_state(tmp_path, monkeypatch): @@ -157,9 +121,9 @@ def test_app_config_reload_without_acp_agents_clears_previous_state(tmp_path, mo monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(extensions_path)) config_path.write_text(yaml.safe_dump(config_with_acp), encoding="utf-8") - AppConfig.from_file(str(config_path)) - assert set(get_acp_agents()) == {"codex"} + cfg1 = AppConfig.from_file(str(config_path)) + assert set(cfg1.acp_agents) == {"codex"} config_path.write_text(yaml.safe_dump(config_without_acp), encoding="utf-8") - AppConfig.from_file(str(config_path)) - assert get_acp_agents() == {} + cfg2 = AppConfig.from_file(str(config_path)) + assert cfg2.acp_agents == {} diff --git a/backend/tests/test_app_config_reload.py b/backend/tests/test_app_config_reload.py index 3f744aee2f..e98c0dc891 100644 --- a/backend/tests/test_app_config_reload.py +++ b/backend/tests/test_app_config_reload.py @@ -9,7 +9,6 @@ from pydantic import ValidationError import deerflow.config.app_config as app_config_module -from deerflow.config.acp_config import load_acp_config_from_dict from deerflow.config.agents_api_config import get_agents_api_config, load_agents_api_config_from_dict from deerflow.config.app_config import AppConfig, get_app_config, reset_app_config from deerflow.config.checkpointer_config import get_checkpointer_config, load_checkpointer_config_from_dict @@ -34,7 +33,6 @@ def _reset_config_singletons() -> None: load_guardrails_config_from_dict({}) load_checkpointer_config_from_dict(None) load_stream_bridge_config_from_dict(None) - load_acp_config_from_dict({}) reset_checkpointer() reset_store() reset_app_config() diff --git a/backend/tests/test_invoke_acp_agent_tool.py b/backend/tests/test_invoke_acp_agent_tool.py index 8c44403b88..0b943cc71c 100644 --- a/backend/tests/test_invoke_acp_agent_tool.py +++ b/backend/tests/test_invoke_acp_agent_tool.py @@ -669,23 +669,12 @@ def method_not_found(method): def test_get_available_tools_includes_invoke_acp_agent_when_agents_configured(monkeypatch): - from deerflow.config.acp_config import load_acp_config_from_dict - - load_acp_config_from_dict( - { - "codex": { - "command": "codex-acp", - "args": [], - "description": "Codex CLI", - } - } - ) - fake_config = SimpleNamespace( tools=[], models=[], tool_search=SimpleNamespace(enabled=False), get_model_config=lambda name: None, + acp_agents={"codex": ACPAgentConfig(command="codex-acp", description="Codex CLI")}, ) monkeypatch.setattr("deerflow.tools.tools.get_app_config", lambda: fake_config) monkeypatch.setattr( @@ -696,8 +685,6 @@ def test_get_available_tools_includes_invoke_acp_agent_when_agents_configured(mo tools = get_available_tools(include_mcp=True, subagent_enabled=False) assert "invoke_acp_agent" in [tool.name for tool in tools] - load_acp_config_from_dict({}) - def test_get_available_tools_uses_explicit_app_config_for_acp_agents(monkeypatch): explicit_agents = {"codex": ACPAgentConfig(command="codex-acp", description="Codex CLI")} @@ -712,15 +699,11 @@ def test_get_available_tools_uses_explicit_app_config_for_acp_agents(monkeypatch sentinel_tool = SimpleNamespace(name="invoke_acp_agent") captured: dict[str, object] = {} - def fail_get_acp_agents(): - raise AssertionError("ambient get_acp_agents() must not be used when app_config is explicit") - def fake_build_invoke_acp_agent_tool(agents): captured["agents"] = agents return sentinel_tool monkeypatch.setattr("deerflow.tools.tools.is_host_bash_allowed", lambda config=None: True) - monkeypatch.setattr("deerflow.config.acp_config.get_acp_agents", fail_get_acp_agents) monkeypatch.setattr("deerflow.tools.builtins.invoke_acp_agent_tool.build_invoke_acp_agent_tool", fake_build_invoke_acp_agent_tool) tools = get_available_tools(include_mcp=False, subagent_enabled=False, app_config=explicit_config) diff --git a/backend/tests/test_lead_agent_prompt.py b/backend/tests/test_lead_agent_prompt.py index 28c8518e59..2f0360c69e 100644 --- a/backend/tests/test_lead_agent_prompt.py +++ b/backend/tests/test_lead_agent_prompt.py @@ -178,11 +178,6 @@ def fail_get_subagents_app_config(): def test_build_acp_section_uses_explicit_app_config_without_global_config(monkeypatch): explicit_config = SimpleNamespace(acp_agents={"codex": object()}) - def fail_get_acp_agents(): - raise AssertionError("ambient get_acp_agents() must not be used when app_config is explicit") - - monkeypatch.setattr("deerflow.config.acp_config.get_acp_agents", fail_get_acp_agents) - section = prompt_module._build_acp_section(app_config=explicit_config) assert "ACP Agent Tasks" in section