mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-26 07:57:57 +00:00
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.
This commit is contained in:
parent
807c3c5218
commit
a94ea9325b
@ -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()
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user