diff --git a/README.md b/README.md index 92cdc8ff59..ead9314a4b 100644 --- a/README.md +++ b/README.md @@ -2018,6 +2018,11 @@ the stable English `name`. API callers can pass `display_name` to agent creation or update requests; an omitted update preserves it and `null` clears it. The same optional field is supported in the agent's `config.yaml`. +Custom Agents migrated to `agent_storage.backend: db` remain editable through +Agent settings and `update_agent` while the importer retains their old files +for rollback. See [Agent Storage](backend/docs/CONFIGURATION.md#agent-storage) +for the migration procedure. + Custom Agents can also be moved between DeerFlow users or deployments from the Agents gallery. **Export Agent** downloads a versioned `*.deerflow-agent.json` package containing the portable definition and diff --git a/backend/app/gateway/routers/AGENTS.md b/backend/app/gateway/routers/AGENTS.md index 9904a14366..cb3f9e6564 100644 --- a/backend/app/gateway/routers/AGENTS.md +++ b/backend/app/gateway/routers/AGENTS.md @@ -16,6 +16,11 @@ create-only: collisions return 409, never overwrite or upsert. Ordinary creation and import share `_persist_new_agent`, which uses `run_drained_write` so a cancelled request still drains the owned persistence worker. +Custom Agent update guards apply legacy shared-file checks only when the +resolved store is `FileAgentStore`, including the `update_agent` tool. DB +imports retain source files for rollback; those files cannot veto SQL updates. +Keep store resolution and filesystem checks off-loop in the HTTP route. + Committed MCP reconciliation failures are logged once by `fail_mcp_reconciliation`, with only the underlying exception type. `_run_drained_mcp_apply` passes `_McpCommittedReconciliationError` as an expected diff --git a/backend/app/gateway/routers/agents.py b/backend/app/gateway/routers/agents.py index 459f520aa5..5415de97b9 100644 --- a/backend/app/gateway/routers/agents.py +++ b/backend/app/gateway/routers/agents.py @@ -26,6 +26,7 @@ from deerflow.config.app_config import get_app_config from deerflow.config.paths import get_paths from deerflow.knowledge_scope import KnowledgeScope, canonicalize_knowledge_scope from deerflow.persistence.agents import AgentDeleteOutcome, AgentExistsError, get_agent_store +from deerflow.persistence.agents.file import FileAgentStore from deerflow.runtime.user_context import get_effective_user_id logger = logging.getLogger(__name__) @@ -532,15 +533,18 @@ async def update_agent(name: str, body: AgentUpdateRequest, request: Request) -> raise HTTPException(status_code=404, detail=f"Agent '{name}' not found") def _is_legacy_only_layout() -> bool: + # The DB importer retains legacy source files for rollback. They say + # nothing about the ownership of the active SQL definition. + if not isinstance(get_agent_store(), FileAgentStore): + return False # 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). The db backend has no legacy - # shared layout, so this file-only guard is a no-op there. The .exists() - # probes are filesystem IO, so they run off the event loop. + # resolve_agent_dir's guard, see #3390). Store resolution and the + # filesystem probes both run off the event loop. paths = get_paths() agent_dir = paths.user_agent_dir(user_id, name) legacy_dir = paths.agent_dir(name) diff --git a/backend/docs/CONFIGURATION.md b/backend/docs/CONFIGURATION.md index aafba34b00..f1d9296e6a 100644 --- a/backend/docs/CONFIGURATION.md +++ b/backend/docs/CONFIGURATION.md @@ -684,6 +684,10 @@ uv run --no-sync --project backend python backend/scripts/migrate_agents_to_db.p The importer is idempotent (already-present agents are skipped) and leaves the source files untouched, so reverting `agent_storage.backend` to `file` is a clean rollback. Agent *memory* (`memory.json`) is unaffected by this switch. +With `backend: db`, retained legacy files do not block HTTP or `update_agent` +updates to the imported SQL definitions. With `backend: file`, shared legacy +agents still require the user-isolation migration before they can be updated. + ### Tools Configure specific tools available to the agent: 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 3474f616bd..b9682468f4 100644 --- a/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py @@ -38,6 +38,7 @@ from deerflow.config.agents_config import load_agent_config, preserve_non_manage from deerflow.config.app_config import get_app_config from deerflow.config.paths import get_paths from deerflow.persistence.agents import get_agent_store +from deerflow.persistence.agents.file import FileAgentStore from deerflow.runtime.user_context import resolve_runtime_user_id from deerflow.tools.types import Runtime @@ -158,19 +159,16 @@ def update_agent( if model is not None and get_app_config().get_model_config(model) is None: return _err(f"Unknown model '{model}'. Pass a model name that exists in config.yaml's models section.") - paths = get_paths() - agent_dir = paths.user_agent_dir(user_id, agent_name) - 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.") + # SQL definitions are already owner-scoped; the importer leaves their + # file-backed sources in place for rollback (#6743). + if isinstance(get_agent_store(), FileAgentStore): + paths = get_paths() + agent_dir = paths.user_agent_dir(user_id, agent_name) + legacy_dir = paths.agent_dir(agent_name) + # A memory-only user directory must not shadow the shared definition + # or allow a silent per-user fork (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: existing_cfg = load_agent_config(agent_name, user_id=user_id) diff --git a/backend/tests/test_agent_storage_backend.py b/backend/tests/test_agent_storage_backend.py index 206f8d11dc..14db9bcb13 100644 --- a/backend/tests/test_agent_storage_backend.py +++ b/backend/tests/test_agent_storage_backend.py @@ -7,11 +7,16 @@ from types import SimpleNamespace import pytest import yaml +from fastapi import FastAPI +from fastapi.testclient import TestClient from sqlalchemy import create_engine +from app.gateway.auth_middleware import AuthMiddleware from app.gateway.deps import _validate_agent_storage +from app.gateway.routers import agents as agents_router from deerflow.config.agent_storage_config import AgentStorageConfig -from deerflow.config.app_config import reset_app_config +from deerflow.config.agents_api_config import load_agents_api_config_from_dict +from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config from deerflow.config.database_config import DatabaseConfig from deerflow.config.extensions_config import reset_extensions_config from deerflow.persistence.agents import get_agent_store, make_agent_store @@ -23,6 +28,7 @@ from deerflow.persistence.managed_subagents.base import ManagedSubagentDefinitio from deerflow.persistence.managed_subagents.file import FileManagedSubagentStore from deerflow.persistence.managed_subagents.model import ManagedSubagentRow from deerflow.persistence.managed_subagents.sql import SqlManagedSubagentStore +from deerflow.tools.builtins.update_agent_tool import update_agent def _cfg(agent_backend: str, db_backend: str, sqlite_dir: str = "/tmp/agent-store-test") -> SimpleNamespace: @@ -156,6 +162,78 @@ def test_importer_dry_run_writes_nothing(file_home, monkeypatch): assert SqlManagedSubagentStore(cfg.database.app_sync_sqlalchemy_url).list() == [] +@pytest.mark.parametrize("backend", ["file", "db"]) +@pytest.mark.parametrize("memory_only", [False, True], ids=["no-user-dir", "memory-only-user-dir"]) +@pytest.mark.parametrize("surface", ["http", "tool"]) +def test_legacy_source_only_blocks_file_backed_updates(tmp_path, monkeypatch, backend, memory_only, surface): + """Retained migration sources must not override the active SQL store (#6743).""" + monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path)) + monkeypatch.setattr("deerflow.config.paths._paths", None) + monkeypatch.setenv("DEER_FLOW_AUTH_DISABLED", "1") + monkeypatch.setenv("DEER_FLOW_ENV", "test") + monkeypatch.setenv("ENVIRONMENT", "test") + legacy = tmp_path / "agents" / "reviewer" + legacy.mkdir(parents=True) + original_config = "name: reviewer\ndescription: original\n" + (legacy / "config.yaml").write_text(original_config, encoding="utf-8") + (legacy / "SOUL.md").write_text("Review carefully.", encoding="utf-8") + user_dir = tmp_path / "users" / "default" / "agents" / "reviewer" + if memory_only: + user_dir.mkdir(parents=True) + (user_dir / "memory.json").write_text("{}", encoding="utf-8") + + cfg = AppConfig( + models=[], + sandbox={"use": "deerflow.sandbox.local:LocalSandboxProvider"}, + database={"backend": "sqlite", "sqlite_dir": str(tmp_path / "db")}, + agent_storage={"backend": backend}, + ) + set_app_config(cfg) + load_agents_api_config_from_dict({"enabled": True}) + try: + if backend == "db": + # Real file discovery, import and SQL writes; only the unrelated + # schema bootstrap is replaced by the fixture's real SQLite DDL. + importer = _patch_importer(monkeypatch, cfg) + monkeypatch.setattr(sys, "argv", ["migrate_agents_to_db"]) + assert importer.main() == 0 + store = get_agent_store() + assert store.get("reviewer", user_id="default").description == "original" + if backend == "db": + store.create("reviewer", {"name": "reviewer", "description": "other owner"}, "Other soul", user_id="other-user") + + if surface == "http": + app = FastAPI() + app.add_middleware(AuthMiddleware) + app.include_router(agents_router.router) + with TestClient(app) as client: + assert client.get("/api/agents/reviewer").status_code == 200 + response = client.put("/api/agents/reviewer", json={"description": "updated"}) + assert response.status_code == (200 if backend == "db" else 409), response.text + if backend == "file": + assert "legacy shared layout" in response.json()["detail"] + else: + runtime = SimpleNamespace(context={"agent_name": "reviewer", "user_id": "default"}, tool_call_id="update-1") + message = update_agent.func(runtime=runtime, description="updated").update["messages"][0] + assert message.status == ("success" if backend == "db" else "error"), message.content + if backend == "file": + assert "legacy shared layout" in message.content + + assert store.get("reviewer", user_id="default").description == ("updated" if backend == "db" else "original") + assert store.get_soul("reviewer", user_id="default") == "Review carefully." + if backend == "db": + assert store.get("reviewer", user_id="other-user").description == "other owner" + assert store.get_soul("reviewer", user_id="other-user") == "Other soul" + assert (legacy / "config.yaml").read_text(encoding="utf-8") == original_config + assert (legacy / "SOUL.md").read_text(encoding="utf-8") == "Review carefully." + assert not (user_dir / "config.yaml").exists() + if memory_only: + assert (user_dir / "memory.json").read_text(encoding="utf-8") == "{}" + finally: + load_agents_api_config_from_dict({}) + reset_app_config() + + def test_importer_runs_when_only_managed_subagents_exist(tmp_path, monkeypatch): monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path)) from deerflow.config import paths as paths_module