mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 21:26:39 +00:00
fix(agents): allow SQL updates with retained legacy files (#6747)
This commit is contained in:
parent
16a4f780c1
commit
5b2a1352cd
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user