From a94ea9325b5660e4ea75d3b8911d7a7660bc0c1f Mon Sep 17 00:00:00 2001 From: qin-chenghan Date: Mon, 13 Jul 2026 21:13:33 +0800 Subject: [PATCH] fix(agents): load SOUL.md from agent dirs without config.yaml (#4136) * fix(agents): load SOUL.md from agent dirs without config.yaml (#4135) resolve_agent_dir requires config.yaml to be present in an agent directory (added in #3481 to fix #3390, where memory-only directories were mistaken for agent directories). However, SOUL.md loading does not depend on config.yaml. When an agent is configured externally (e.g. via DEER_FLOW_CONFIG_PATH) and the agent directory contains SOUL.md but no config.yaml, resolve_agent_dir skips the directory and load_agent_soul returns None. Add a fallback in load_agent_soul: if the resolved directory does not contain SOUL.md, check the per-user and legacy directories directly for SOUL.md. This preserves the #3390 fix for resolve_agent_dir (which is also used by load_agent_config) while allowing SOUL.md to load independently of config.yaml. * fix(agents): gate SOUL.md fallback on missing config.yaml per review Address review feedback on PR #4136: 1. Gate the fallback condition on not (agent_dir / config.yaml).exists() so it only fires when resolve_agent_dir returned its default path (no agent dir qualified), not when a properly-resolved per-user agent simply lacks SOUL.md. This preserves the per-user shadowing invariant. 2. Fix test_loads_soul_from_user_dir_without_config_yaml to actually exercise the fallback path: per-user memory-only dir (no config.yaml, no SOUL.md) + legacy dir with SOUL.md (no config.yaml) -> fallback finds legacy SOUL.md. 3. Add test_soul_not_leaked_from_legacy_when_per_user_has_config to verify the gate prevents legacy SOUL.md leaking into a per-user agent. 4. Patch get_effective_user_id in test_loads_soul_without_config_yaml for consistency and resilience. --- .../harness/deerflow/config/agents_config.py | 25 ++++++- backend/tests/test_custom_agent.py | 67 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/backend/packages/harness/deerflow/config/agents_config.py b/backend/packages/harness/deerflow/config/agents_config.py index 0715811a8..7e36b9f2f 100644 --- a/backend/packages/harness/deerflow/config/agents_config.py +++ b/backend/packages/harness/deerflow/config/agents_config.py @@ -299,9 +299,32 @@ def load_agent_soul(agent_name: str | None, *, user_id: str | None = None) -> st """ if agent_name: agent_dir = resolve_agent_dir(agent_name, user_id=user_id) + soul_path = agent_dir / SOUL_FILENAME + # Fallback: resolve_agent_dir requires config.yaml to be present + # (see #3390), but SOUL.md loading does not depend on config.yaml. + # If the resolved dir doesn't have config.yaml (meaning the resolver + # returned its default path because no agent dir qualified) and also + # lacks SOUL.md, check the per-user and legacy directories directly + # so that agents configured via DEER_FLOW_CONFIG_PATH (or any setup + # where the agent dir has SOUL.md but no config.yaml) can still load + # their soul (#4135). The config.yaml guard ensures this fallback + # only fires for dirs the resolver couldn't resolve, not for a + # properly-resolved per-user agent that simply lacks SOUL.md - + # preserving the "per-user entries fully shadow legacy entries" + # invariant (agents_config.py:3-7, list_custom_agents). + if not soul_path.exists() and not (agent_dir / "config.yaml").exists(): + paths = get_paths() + effective_user = user_id or get_effective_user_id() + for candidate in ( + paths.user_agent_dir(effective_user, agent_name), + paths.agent_dir(agent_name), + ): + if (candidate / SOUL_FILENAME).exists(): + soul_path = candidate / SOUL_FILENAME + break else: agent_dir = get_paths().base_dir - soul_path = agent_dir / SOUL_FILENAME + soul_path = agent_dir / SOUL_FILENAME if not soul_path.exists(): return None content = soul_path.read_text(encoding="utf-8").strip() diff --git a/backend/tests/test_custom_agent.py b/backend/tests/test_custom_agent.py index 0872c529c..f8d34bde6 100644 --- a/backend/tests/test_custom_agent.py +++ b/backend/tests/test_custom_agent.py @@ -322,6 +322,73 @@ class TestLoadAgentSoul: assert soul is None + def test_loads_soul_without_config_yaml(self, tmp_path): + """SOUL.md should load even when the agent dir has no config.yaml (#4135).""" + agent_dir = tmp_path / "agents" / "soul-only" + agent_dir.mkdir(parents=True) + # Deliberately no config.yaml – the agent is configured externally + (agent_dir / "SOUL.md").write_text("You are a brave agent.", encoding="utf-8") + + with patch("deerflow.config.agents_config.get_paths", return_value=_make_paths(tmp_path)), patch("deerflow.config.agents_config.get_effective_user_id", return_value="default"): + from deerflow.config.agents_config import load_agent_soul + + soul = load_agent_soul("soul-only") + + assert soul == "You are a brave agent." + + def test_loads_soul_from_user_dir_without_config_yaml(self, tmp_path): + """Fallback should find SOUL.md when resolver returns a default dir without it (#4135). + + Setup: per-user agent 'foo' exists as a memory-only directory + (no config.yaml, no SOUL.md). Legacy agent 'foo' has SOUL.md but + no config.yaml. resolve_agent_dir returns the per-user path as + default (neither dir has config.yaml). The fallback then finds + SOUL.md in the legacy directory. + """ + # Per-user dir: memory-only (no config.yaml, no SOUL.md) + user_dir = tmp_path / "users" / "test-user" / "agents" / "foo" + user_dir.mkdir(parents=True) + (user_dir / "memory.json").write_text("{}", encoding="utf-8") + + # Legacy dir: has SOUL.md but no config.yaml + legacy_dir = tmp_path / "agents" / "foo" + legacy_dir.mkdir(parents=True) + (legacy_dir / "SOUL.md").write_text("You are a legacy agent.", encoding="utf-8") + + with patch("deerflow.config.agents_config.get_paths", return_value=_make_paths(tmp_path)), patch("deerflow.config.agents_config.get_effective_user_id", return_value="test-user"): + from deerflow.config.agents_config import load_agent_soul + + soul = load_agent_soul("foo") + + assert soul == "You are a legacy agent." + + def test_soul_not_leaked_from_legacy_when_per_user_has_config(self, tmp_path): + """Per-user agent with config.yaml but no SOUL.md should NOT fall back to legacy SOUL.md. + + This verifies the gated condition: fallback only fires when the + resolved dir lacks config.yaml. A properly-resolved per-user agent + that simply has no SOUL.md returns None, preserving the + "per-user entries fully shadow legacy entries" invariant. + """ + # Legacy dir: has SOUL.md + legacy_dir = tmp_path / "agents" / "foo" + legacy_dir.mkdir(parents=True) + (legacy_dir / "config.yaml").write_text("name: foo\n") + (legacy_dir / "SOUL.md").write_text("You are a legacy agent.", encoding="utf-8") + + # Per-user dir: has config.yaml (resolver returns this) but no SOUL.md + user_dir = tmp_path / "users" / "test-user" / "agents" / "foo" + user_dir.mkdir(parents=True) + (user_dir / "config.yaml").write_text("name: foo\n") + # No SOUL.md in per-user dir + + with patch("deerflow.config.agents_config.get_paths", return_value=_make_paths(tmp_path)), patch("deerflow.config.agents_config.get_effective_user_id", return_value="test-user"): + from deerflow.config.agents_config import load_agent_soul + + soul = load_agent_soul("foo") + + assert soul is None + # =========================================================================== # 5. list_custom_agents