deer-flow/backend/tests/test_persistence_bootstrap_concurrency.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

151 lines
5.2 KiB
Python

"""Concurrency safety tests for ``bootstrap_schema``.
The contract: N concurrent callers against the same DB always converge to
``alembic_version == head`` without exceptions and without duplicate schema
mutations.
We model concurrency at the *async-task* level here (multiple coroutines
inside one process). SQLite is single-node by deployment, so within-process
serialisation -- which is what the per-engine ``_SQLITE_LOCKS`` entry
provides -- is the realistic boundary. Cross-process serialisation falls
through to SQLite's own write lock + ``PRAGMA busy_timeout`` plus the
idempotent revision helpers.
"""
from __future__ import annotations
import asyncio
from pathlib import Path
import pytest
import sqlalchemy as sa
from sqlalchemy.ext.asyncio import create_async_engine
import deerflow.persistence.models # noqa: F401
from deerflow.persistence import bootstrap as bootstrap_mod
from deerflow.persistence.bootstrap import bootstrap_schema
pytestmark = pytest.mark.asyncio
HEAD = "0006_agents"
def _url(tmp_path: Path) -> str:
return f"sqlite+aiosqlite:///{(tmp_path / 'concurrent.db').as_posix()}"
async def _alembic_version(engine) -> str | None:
async with engine.connect() as conn:
row = await conn.execute(sa.text("SELECT version_num FROM alembic_version"))
return row.scalar()
async def _runs_columns(engine) -> set[str]:
async with engine.connect() as conn:
return await conn.run_sync(lambda c: {col["name"] for col in sa.inspect(c).get_columns("runs")})
async def test_two_concurrent_bootstrap_callers_converge(tmp_path: Path) -> None:
engine = create_async_engine(_url(tmp_path))
try:
await asyncio.gather(
bootstrap_schema(engine, backend="sqlite"),
bootstrap_schema(engine, backend="sqlite"),
)
assert await _alembic_version(engine) == HEAD
assert "token_usage_by_model" in await _runs_columns(engine)
finally:
await engine.dispose()
async def test_five_concurrent_bootstrap_callers_converge(tmp_path: Path) -> None:
engine = create_async_engine(_url(tmp_path))
try:
await asyncio.gather(*(bootstrap_schema(engine, backend="sqlite") for _ in range(5)))
assert await _alembic_version(engine) == HEAD
finally:
await engine.dispose()
async def test_cancelled_caller_does_not_block_others(tmp_path: Path) -> None:
"""Cancelling one task mid-bootstrap must not strand the lock or the DB.
After the cancel, a subsequent ``bootstrap_schema`` call must still reach
head.
"""
engine = create_async_engine(_url(tmp_path))
try:
task = asyncio.create_task(bootstrap_schema(engine, backend="sqlite"))
# Give the event loop a turn so the task can start; then cancel.
await asyncio.sleep(0)
task.cancel()
# Cancelled task may have raced past the lock; swallow either outcome.
try:
await task
except (asyncio.CancelledError, Exception): # noqa: BLE001
pass
# Lock must be free for the next caller.
await bootstrap_schema(engine, backend="sqlite")
assert await _alembic_version(engine) == HEAD
finally:
await engine.dispose()
async def test_late_caller_after_head_is_noop(monkeypatch, tmp_path: Path) -> None:
"""When the first caller leaves the DB at head, the second observes
'versioned' and skips create_all / stamp -- it only runs upgrade head,
which is alembic-no-op.
We use a monkeypatched ``_upgrade`` counter to assert the second caller's
upgrade ran but did no real work (no new revision applied).
"""
engine = create_async_engine(_url(tmp_path))
try:
# First caller: empty branch.
await bootstrap_schema(engine, backend="sqlite")
first_version = await _alembic_version(engine)
assert first_version == HEAD
upgrade_calls: list[str] = []
original_upgrade = bootstrap_mod._upgrade
def counting_upgrade(cfg, rev: str) -> None:
upgrade_calls.append(rev)
original_upgrade(cfg, rev)
monkeypatch.setattr(bootstrap_mod, "_upgrade", counting_upgrade)
# Second caller: versioned branch -> calls _upgrade('head').
await bootstrap_schema(engine, backend="sqlite")
assert upgrade_calls == ["head"]
assert await _alembic_version(engine) == HEAD
finally:
await engine.dispose()
async def test_slow_upgrade_does_not_corrupt_concurrent_state(monkeypatch, tmp_path: Path) -> None:
"""Inject a delay into the upgrade path; concurrent callers must still
converge to head with no exceptions."""
engine = create_async_engine(_url(tmp_path))
try:
original_upgrade = bootstrap_mod._upgrade
def slow_upgrade(cfg, rev: str) -> None:
import time # noqa: PLC0415
time.sleep(0.2)
original_upgrade(cfg, rev)
monkeypatch.setattr(bootstrap_mod, "_upgrade", slow_upgrade)
await asyncio.gather(
bootstrap_schema(engine, backend="sqlite"),
bootstrap_schema(engine, backend="sqlite"),
bootstrap_schema(engine, backend="sqlite"),
)
assert await _alembic_version(engine) == HEAD
finally:
await engine.dispose()