deer-flow/backend/tests/test_persistence_bootstrap_regression.py
Aari 0d4d0cb17d
feat(agents): database-backed storage for custom agent definitions (#4359)
* feat(agents): database-backed storage for custom agent definitions

Add an agent_storage.backend switch (default file, behaviour-unchanged) with a
db backend that stores each custom agent as a row in the shared SQL persistence
layer, so a multi-instance deployment sees the same agents on every node
(#4331, #4357). Introduces an AgentStore interface routing all read/write
surfaces, an agents table + migration 0006, startup validation, and a file->db
importer. Follows the thread_meta store / run_events backend-switch /
0003_scheduled_tasks migration patterns; no new dependency.

* fix(agents): make db storage path production-ready (review round 1)

Addresses review feedback on the db/sync agent-storage path:

- sql.py: mirror the async engine's per-connection SQLite PRAGMAs on the sync
  engine (busy_timeout=30000, synchronous=NORMAL, foreign_keys=ON, WAL) so both
  engines behave identically against the shared DB; guard the engine cache with
  a lock (double-checked) so concurrent first-touch cannot build duplicate
  engines or register the connect listener twice.
- routers/agents.py + routers/assistants_compat.py: offload the sync-store reads
  that ran on the event loop (list/get/check, update's pre-read + legacy guard +
  refresh, and assistants_compat's four list routes) via asyncio.to_thread — on
  db+postgres each was a network round trip stalling the loop. Writes were
  already offloaded.
- file.py: translate the create() mkdir(exist_ok=False) race FileExistsError
  into AgentExistsError (router 409, matching SqlAgentStore's IntegrityError
  path); correct the _write docstring — per-file atomic replace, two commits
  sequential not transactional.

Tests: sync-engine PRAGMA + engine-cache reuse assertions; file create-race ->
AgentExistsError; strict Blockbuster anchor over the read endpoints so a
regression back onto the loop fails CI.

* fix(agents): address round-2 review on the db store path

- update_agent tool: align the docstring/inline comment with FileAgentStore._write.
  Cross-field write atomicity is db-only; the file backend commits config then
  soul via two sequential os.replace (a crash between them can leave a fresh
  config.yaml beside a stale SOUL.md). The dropped partial-write *reporting* is
  an intentional tradeoff — the stage-then-replace safety is preserved
  (test_update_agent_soul_failure_does_not_replace_config still holds).
- SqlAgentStore.update(): true upsert. Catch IntegrityError on the
  insert-on-missing branch, re-fetch and apply, so two concurrent first-time
  writes (e.g. two setup_agent handshakes) converge instead of surfacing a raw
  UNIQUE(user_id, name) violation as a 500. Symmetric with create().
- get_agent_store(): document the graph-subprocess config-resolution invariant
  (the except->file fallback is a genuine no-config path, not a mask for a
  misconfigured graph process) and pin it with two tests driving the real
  get_app_config() file resolution: db resolves from an on-disk config.yaml,
  file fallback when config is unresolvable.

* test(agents): cover SqlAgentStore.update() write-race upsert recovery

Mandatory-TDD test for the round-2 fix in 0680340a: two concurrent first-time
update()s where the loser's insert hits UNIQUE(user_id, name). Deterministically
forces the IntegrityError recovery path by making the first _row probe miss the
committed winner, and asserts last-writer-wins instead of a surfaced 500.
2026-07-23 08:03:21 +08:00

122 lines
4.8 KiB
Python

"""Regression test for GitHub issue #3682.
End-to-end shape:
1. Hand-build a SQLite DB that mirrors a real pre-#3658 deployment -- the
``runs`` table is missing the ``token_usage_by_model`` column, mirroring
what every existing user's DB looked like after the upgrade that triggered
the issue.
2. Run ``init_engine`` (the entry point used by the FastAPI Gateway
lifespan), which now routes through ``bootstrap_schema``.
3. Confirm a real ``SELECT`` against the column succeeds, demonstrating the
500 from the original issue is gone.
The pre-fix codepath would have raised
``sqlalchemy.exc.OperationalError: no such column: runs.token_usage_by_model``
on step 3.
"""
from __future__ import annotations
import sqlite3
from pathlib import Path
from uuid import uuid4
import pytest
import sqlalchemy as sa
import deerflow.persistence.models # noqa: F401 -- registers ORM models
from deerflow.persistence.base import Base
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
from deerflow.persistence.run import RunRepository
pytestmark = pytest.mark.asyncio
def _seed_pre_3658_database(db_path: Path) -> None:
"""Build a DB that looks like a pre-PR-#3658 deployment.
Uses the synchronous ``sqlite3`` driver so the seed is independent of the
async engine under test.
"""
db_path.parent.mkdir(parents=True, exist_ok=True)
# Easiest way to get the legacy shape exactly right: create_all then
# ALTER away the new column.
sync_url = f"sqlite:///{db_path.as_posix()}"
sync_engine = sa.create_engine(sync_url)
try:
Base.metadata.create_all(sync_engine)
with sync_engine.begin() as conn:
conn.execute(sa.text("ALTER TABLE runs DROP COLUMN token_usage_by_model"))
finally:
sync_engine.dispose()
async def test_legacy_database_recovers_token_usage_column(tmp_path: Path) -> None:
db_path = tmp_path / "legacy.db"
_seed_pre_3658_database(db_path)
# Sanity: confirm we did indeed land in the buggy pre-fix shape before
# init_engine touches the file.
with sqlite3.connect(db_path) as raw:
cols = {row[1] for row in raw.execute("PRAGMA table_info(runs)").fetchall()}
assert "run_id" in cols
assert "token_usage_by_model" not in cols
version_table_count = raw.execute("SELECT count(*) FROM sqlite_master WHERE type='table' AND name='alembic_version'").fetchone()[0]
assert version_table_count == 0
# Run the same init_engine path FastAPI lifespan uses on startup.
url = f"sqlite+aiosqlite:///{db_path.as_posix()}"
await init_engine(backend="sqlite", url=url, sqlite_dir=str(tmp_path))
try:
# The column must now be present.
with sqlite3.connect(db_path) as raw:
cols = {row[1] for row in raw.execute("PRAGMA table_info(runs)").fetchall()}
assert "token_usage_by_model" in cols
version_row = raw.execute("SELECT version_num FROM alembic_version").fetchone()
assert version_row[0] == "0006_agents"
# And the read path that originally 500'd must now succeed.
sf = get_session_factory()
assert sf is not None
repo = RunRepository(sf)
# No rows yet -- the point is just that the SELECT does not raise
# ``no such column: runs.token_usage_by_model``.
result = await repo.aggregate_tokens_by_thread(thread_id=str(uuid4()))
assert result["total_tokens"] == 0
assert result["by_model"] == {}
finally:
await close_engine()
async def test_legacy_database_with_manual_alter_still_bootstraps(tmp_path: Path) -> None:
"""User-side workaround scenario: someone already applied the manual
``ALTER TABLE runs ADD COLUMN token_usage_by_model JSON`` from the issue
write-up. The hybrid bootstrap must just stamp head, not double-add the
column, and not error.
"""
db_path = tmp_path / "manual_altered.db"
db_path.parent.mkdir(parents=True, exist_ok=True)
sync_engine = sa.create_engine(f"sqlite:///{db_path.as_posix()}")
try:
Base.metadata.create_all(sync_engine)
# Don't strip the column -- this is the "user already ran the
# workaround" case where create_all already produced it.
finally:
sync_engine.dispose()
url = f"sqlite+aiosqlite:///{db_path.as_posix()}"
await init_engine(backend="sqlite", url=url, sqlite_dir=str(tmp_path))
try:
with sqlite3.connect(db_path) as raw:
cols = [row[1] for row in raw.execute("PRAGMA table_info(runs)").fetchall()]
# No duplicate column -- list, not set, to catch dupes.
assert cols.count("token_usage_by_model") == 1
version_row = raw.execute("SELECT version_num FROM alembic_version").fetchone()
assert version_row[0] == "0006_agents"
finally:
await close_engine()