diff --git a/backend/app/gateway/routers/agents.py b/backend/app/gateway/routers/agents.py index ad8274ca2..0b28f503e 100644 --- a/backend/app/gateway/routers/agents.py +++ b/backend/app/gateway/routers/agents.py @@ -301,7 +301,15 @@ async def update_agent(name: str, request: AgentUpdateRequest) -> AgentResponse: paths = get_paths() agent_dir = paths.user_agent_dir(user_id, name) - if not agent_dir.exists() and paths.agent_dir(name).exists(): + legacy_dir = paths.agent_dir(name) + # Require config.yaml, not bare directory existence — a per-user agent + # directory can exist containing only memory.json (written the first + # time this user chats with a legacy shared agent, before this route + # is ever called). Bare .exists() would miss that case and let this + # fall through to a silent fork of a brand-new config.yaml/SOUL.md + # into the memory-only directory instead of blocking (mirrors + # resolve_agent_dir's guard, see #3390). + if not (agent_dir / "config.yaml").exists() and (legacy_dir / "config.yaml").exists(): raise HTTPException( status_code=409, detail=(f"Agent '{name}' only exists in the legacy shared layout and is not scoped to a user. Run scripts/migrate_user_isolation.py to move legacy agents into the per-user layout before updating."), diff --git a/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py b/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py index 0975a841f..e438579a9 100644 --- a/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py @@ -170,7 +170,16 @@ def update_agent( paths = get_paths() agent_dir = paths.user_agent_dir(user_id, agent_name) - if not agent_dir.exists() and paths.agent_dir(agent_name).exists(): + legacy_dir = paths.agent_dir(agent_name) + # Require config.yaml, not bare directory existence — a per-user agent + # directory can exist containing only memory.json (written the first + # time this user chats with a legacy shared agent, before update_agent + # is ever called). Bare .exists() would miss that case and let this + # fall through to load_agent_config, which correctly resolves through + # to the legacy shared config via resolve_agent_dir, silently forking + # a brand-new config.yaml/SOUL.md into the memory-only directory + # instead of blocking (mirrors resolve_agent_dir's guard, see #3390). + if not (agent_dir / "config.yaml").exists() and (legacy_dir / "config.yaml").exists(): return _err(f"Agent '{agent_name}' only exists in the legacy shared layout and is not scoped to a user. Run scripts/migrate_user_isolation.py to move legacy agents into the per-user layout before updating.") try: diff --git a/backend/tests/test_custom_agent.py b/backend/tests/test_custom_agent.py index f8d34bde6..c56dc5143 100644 --- a/backend/tests/test_custom_agent.py +++ b/backend/tests/test_custom_agent.py @@ -695,6 +695,32 @@ class TestAgentsAPI: ], } + def test_update_memory_only_user_dir_with_legacy_agent_returns_409(self, agent_client, tmp_path): + """Regression for #3390's PUT /api/agents/{name} guard. + + A per-user agent directory can exist containing only memory.json + (written the first time this user chats with a legacy shared + agent). The stale guard checked bare directory existence and + missed this case, letting the route silently fork a brand-new + config.yaml/SOUL.md into the memory-only directory instead of + blocking with the migration-script guidance. + """ + legacy_dir = tmp_path / "agents" / "legacy-agent" + legacy_dir.mkdir(parents=True) + (legacy_dir / "config.yaml").write_text("name: legacy-agent\ndescription: legacy\n", encoding="utf-8") + (legacy_dir / "SOUL.md").write_text("legacy soul", encoding="utf-8") + + user_agent_dir = tmp_path / "users" / "test-user-autouse" / "agents" / "legacy-agent" + user_agent_dir.mkdir(parents=True) + (user_agent_dir / "memory.json").write_text("{}", encoding="utf-8") + + response = agent_client.put("/api/agents/legacy-agent", json={"soul": "should not write"}) + + assert response.status_code == 409 + assert not (user_agent_dir / "config.yaml").exists() + assert not (user_agent_dir / "SOUL.md").exists() + assert (user_agent_dir / "memory.json").exists(), "the user's existing memory must be left untouched" + def test_update_missing_agent_404(self, agent_client): response = agent_client.put("/api/agents/ghost-agent", json={"soul": "new"}) assert response.status_code == 404 diff --git a/backend/tests/test_update_agent_tool.py b/backend/tests/test_update_agent_tool.py index 7eb2dc97d..594becfbd 100644 --- a/backend/tests/test_update_agent_tool.py +++ b/backend/tests/test_update_agent_tool.py @@ -124,6 +124,37 @@ def test_update_agent_rejects_unknown_agent(tmp_path, patched_paths): assert not _user_agent_dir(tmp_path, "ghost").exists() +def test_update_agent_rejects_legacy_agent_when_user_dir_has_only_memory(tmp_path, patched_paths): + """Regression for #3390's update_agent guard. + + A per-user agent directory can exist containing only memory.json — + written automatically the first time this user chats with a legacy + shared agent, before update_agent is ever called. The stale guard + checked bare directory existence, so it missed this case, fell + through to load_agent_config (which correctly resolves through to + the legacy shared config via resolve_agent_dir), and then silently + forked a brand-new config.yaml/SOUL.md into the memory-only + directory — splitting the agent for just this user with no warning. + """ + legacy_dir = tmp_path / "agents" / "legacy-agent" + legacy_dir.mkdir(parents=True) + (legacy_dir / "config.yaml").write_text(yaml.safe_dump({"name": "legacy-agent", "description": "legacy"}), encoding="utf-8") + (legacy_dir / "SOUL.md").write_text("legacy soul", encoding="utf-8") + + user_agent_dir = _user_agent_dir(tmp_path, "legacy-agent") + user_agent_dir.mkdir(parents=True) + (user_agent_dir / "memory.json").write_text("{}", encoding="utf-8") + + result = update_agent.func(runtime=_runtime(agent_name="legacy-agent"), soul="should not write") + + msg = result.update["messages"][0] + assert "only exists in the legacy shared layout" in msg.content + assert msg.status == "error" + assert not (user_agent_dir / "config.yaml").exists() + assert not (user_agent_dir / "SOUL.md").exists() + assert (user_agent_dir / "memory.json").exists(), "the user's existing memory must be left untouched" + + def test_update_agent_requires_at_least_one_field(tmp_path, patched_paths): _seed_agent(tmp_path)