diff --git a/CHANGELOG.md b/CHANGELOG.md index 2de8373bd..2ba5a4aeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -941,6 +941,18 @@ This release closes that milestone with **765 merged pull requests**. ### Fixed +- **persistence:** Heal databases that silently skipped the run-change clock + schema. `0023_run_change_seq` was inserted ahead of the already-shipped + `0023_user_preferences` revision, so databases stamped at that revision (or + later) treat it as an applied ancestor and never execute it — leaving the + `run_change_clock` table and the `runs.change_seq` column permanently + missing, and the first thread deletion (any run-store change-clock bump) + fails with `no such table: run_change_clock`. The new + `0025_repair_run_change_seq` revision re-applies the same guarded DDL on + upgrade and no-ops on healthy shapes. `RunChangeClockRow` and + `UserPreferenceRow` are also registered in the ORM model registry so + `create_all` and autogenerate see every table through explicit imports + instead of module side effects. - **nginx:** Extend the 600-second read timeout to the two remaining locations whose routes wait on the Gateway, both left on nginx's 60-second default by the thread-route fix. Behind the `/api/` catch-all, the stateless diff --git a/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md b/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md index ee757fe13..3886a582f 100644 --- a/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md +++ b/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md @@ -2,7 +2,7 @@ DeerFlow's application tables (`runs`, `threads_meta`, `feedback`, `users`, `run_events`, plus the four `channel_*` tables) are owned by alembic via a **hybrid bootstrap** strategy. LangGraph's checkpointer tables (`checkpoints`, `checkpoint_blobs`, `checkpoint_writes`, `checkpoint_migrations`) live in the same database but are owned by LangGraph and excluded from alembic's view via `migrations/_env_filters.py::include_object`. -**Convention**: every ORM model change (new column, new table, new index) MUST ship as an alembic revision under `migrations/versions/`. The Gateway runs `alembic upgrade head` automatically on startup; routine production upgrades do not require manual Alembic commands. The audited offline recovery below is an exception for the out-of-tree incarnation revision. +**Convention**: every ORM model change (new column, new table, new index) MUST ship as an alembic revision under `migrations/versions/`. The Gateway runs `alembic upgrade head` automatically on startup; routine production upgrades do not require manual Alembic commands. The audited offline recovery below is an exception for the out-of-tree incarnation revision. A new revision must always chain onto the current head — never insert one ahead of an already-shipped revision (re-parenting it): alembic only walks forward from a database's stamped revision, so every database already stamped at or past the insertion point treats the inserted revision as an applied ancestor and never executes it. Inserting `0023_run_change_seq` ahead of the shipped `0023_user_preferences` stranded those databases without the run-change clock schema until `0025_repair_run_change_seq` re-applied it (#5516). **Hybrid bootstrap** (`persistence/bootstrap.py::bootstrap_schema`, invoked from `persistence/engine.py::init_engine`): @@ -24,7 +24,7 @@ The empty-DB path keeps using `create_all` because `Base.metadata` is the only a `0020_threads_meta_project_id` → `0021_batch_acceptance` → `0019_thread_incarnations` → `0022_scheduled_occurrence_seq` → `0023_run_change_seq` → `0023_user_preferences` → -`0024_project_documents` (current head). The preference +`0024_project_documents` → `0025_repair_run_change_seq` (current head). The preference revision adds a separate owner/key table with a cascading users foreign key and does not alter users; the project-documents revision adds a new owner-scoped shelf table, so the bootstrap forward-compat floor is unchanged. @@ -161,6 +161,7 @@ on installs that never enabled it. The convention is: - `migrations/versions/0022_scheduled_occurrence_seq.py` — adds the per-task `last_occurrence_seq` high-water mark, nullable occurrence `occurrence_seq` and `launch_accounted`, and a unique `(task_id, occurrence_seq)` index. New occurrences allocate their sequence under the existing parent lock; launch accounting is recorded atomically with the count so an older recovered occurrence cannot be counted twice. Legacy child columns remain NULL without guessed ordering or accounting backfill. All three fields are internal and omitted from repository responses. Both once-task recovery paths lock the parent, defer while any occurrence row is active (sequenced or not), and otherwise project only from the highest sequence (`can_project`), the same rule as the launch, completion, and queue-failure writes. Chains after `0019_thread_incarnations`. - `migrations/versions/0023_run_change_seq.py` — adds `runs.change_seq`, its global singleton allocation clock, and owner-aware cursor indexes. Legacy rows remain at zero and page by run id; lifecycle, cancellation, and model-name mutations allocate monotonically increasing positions in their own transaction. Atomic replacement uses one position for every affected row. Progress snapshots and lease heartbeats do not advance the clock. `0023_user_preferences` follows this revision. Its migration test verifies membership in the single-head chain and the expected predecessor rather than pinning the latest head, so later migrations can extend the chain. - `migrations/versions/0024_project_documents.py` — creates the `project_documents` shelf table (id/project_id/user_id/name/stored_relpath/sha256/size_bytes, nullable promotion provenance and trash fields, timestamps) with indexes on project_id, user_id, sha256 and trashed_at; no DB-level foreign key on project_id by design (project delete trashes the shelf inside its own transaction). New table, so the bootstrap forward-compat floor is unchanged; chains after `0023_user_preferences` (renumbered from 0023 after the rebase) +- `migrations/versions/0025_repair_run_change_seq.py` — heals databases that skipped `0023_run_change_seq` because it was inserted ahead of the already-shipped `0023_user_preferences` (#5516): re-applies the guarded `run_change_clock` table, `runs.change_seq` column, and cursor indexes on upgrade; no-ops on healthy shapes. Its downgrade is a deliberate no-op — the schema and allocated clock positions are owned by ancestor 0023, and dropping them on a repair rollback would recreate the skipped-revision hole and discard cursor data; downgrading to 0023 itself removes the schema through that revision's own downgrade. Chains after `0024_project_documents` - `persistence/bootstrap.py` — `bootstrap_schema(engine, backend=...)`, the three-branch provisioning decision, locked revision validation, and the narrow 0019 forward-compatibility exception - `extensions/loader.py::load_extensions` — registers each spec's `table_prefix` with `register_extension_table_prefix()` -- Tests: `tests/test_persistence_bootstrap.py` (branches), `tests/test_persistence_bootstrap_concurrency.py` (concurrency), `tests/test_persistence_bootstrap_regression.py` (issue #3682), `tests/test_persistence_migrations_env.py` (filter, including extension-owned tables), `tests/test_extension_loader.py::TestTablePrefixRegistration` (spec-to-filter wiring), `tests/blocking_io/test_persistence_bootstrap.py` (asyncio.to_thread anchor), `tests/test_migration_0004_run_ownership_dedupe.py` + `tests/test_migration_0007_scheduled_run_active_dedupe.py` (dedupe-before-unique-index pre-steps) +- Tests: `tests/test_persistence_bootstrap.py` (branches), `tests/test_persistence_bootstrap_concurrency.py` (concurrency), `tests/test_persistence_bootstrap_regression.py` (issue #3682), `tests/test_persistence_migrations_env.py` (filter, including extension-owned tables), `tests/test_extension_loader.py::TestTablePrefixRegistration` (spec-to-filter wiring), `tests/blocking_io/test_persistence_bootstrap.py` (asyncio.to_thread anchor), `tests/test_migration_0004_run_ownership_dedupe.py` + `tests/test_migration_0007_scheduled_run_active_dedupe.py` (dedupe-before-unique-index pre-steps), `tests/test_migration_0025_repair_run_change_seq.py` (issue #5516 skipped-revision heal) diff --git a/backend/packages/harness/deerflow/persistence/migrations/versions/0025_repair_run_change_seq.py b/backend/packages/harness/deerflow/persistence/migrations/versions/0025_repair_run_change_seq.py new file mode 100644 index 000000000..cbda94e64 --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/migrations/versions/0025_repair_run_change_seq.py @@ -0,0 +1,70 @@ +"""Repair run-change clock schema skipped by the 0023 insertion (#5516). + +``0023_run_change_seq`` was inserted between ``0022_scheduled_occurrence_seq`` +and the already-shipped ``0023_user_preferences`` revision. Alembic only walks +forward from a database's stamped revision, so every database that had already +reached ``0023_user_preferences`` (or later) before the insertion treats +``0023_run_change_seq`` as an applied ancestor and never executes it. Those +databases permanently lack the ``run_change_clock`` table, the +``runs.change_seq`` column, and their indexes, and the first run-store +operation that bumps the change clock (e.g. thread deletion via +``create_thread_operation_atomic``) fails with ``no such table: +run_change_clock``. Restarting never heals it because the stamped revision is +already at or past 0023. + +This revision re-applies the same idempotent DDL as ``0023_run_change_seq`` +for every database that upgrades past it, restoring those skipped schemas. +Fresh and legacy databases that ran 0023 itself are untouched: every step is +guarded exactly like 0023 and no-ops on the healthy shape. Its downgrade is +a deliberate no-op — the schema and the allocated clock positions are owned +by ancestor 0023, and removing them here would leave a database stamped at +0024 without 0023's schema, recreating exactly the hole this revision heals. + +Revision ID: 0025_repair_run_change_seq +Revises: 0024_project_documents +""" + +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "0025_repair_run_change_seq" +down_revision: str | Sequence[str] | None = "0024_project_documents" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + from deerflow.persistence.migrations._helpers import safe_add_column + + safe_add_column( + "runs", + sa.Column("change_seq", sa.BigInteger(), nullable=False, server_default=sa.text("0")), + ) + inspector = sa.inspect(op.get_bind()) + tables = set(inspector.get_table_names()) + if "run_change_clock" not in tables: + op.create_table( + "run_change_clock", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("value", sa.BigInteger(), nullable=False, server_default=sa.text("0")), + ) + indexes = {index["name"] for index in sa.inspect(op.get_bind()).get_indexes("runs")} + if "ix_runs_change_seq" not in indexes: + op.create_index("ix_runs_change_seq", "runs", ["change_seq", "run_id"]) + if "ix_runs_user_change_seq" not in indexes: + op.create_index("ix_runs_user_change_seq", "runs", ["user_id", "change_seq", "run_id"]) + + +def downgrade() -> None: + # Deliberate no-op: the change-clock schema and its allocated positions + # are owned by ancestor 0023_run_change_seq, not by this repair. Dropping + # them here would leave a database stamped at 0024 without the schema its + # revision history claims is applied -- recreating exactly the #5516 hole + # this revision heals -- and would permanently discard cursor values. + # Downgrading to 0023 itself is what removes the schema, through that + # revision's own downgrade. + return None diff --git a/backend/packages/harness/deerflow/persistence/models/__init__.py b/backend/packages/harness/deerflow/persistence/models/__init__.py index 74a104a93..56da9abda 100644 --- a/backend/packages/harness/deerflow/persistence/models/__init__.py +++ b/backend/packages/harness/deerflow/persistence/models/__init__.py @@ -27,12 +27,12 @@ from deerflow.persistence.mcp_tasks.model import McpTaskRow from deerflow.persistence.models.run_event import RunEventRow from deerflow.persistence.personal_access_tokens.model import PersonalAccessTokenRow from deerflow.persistence.projects.model import ProjectDocumentRow, ProjectRow -from deerflow.persistence.run.model import RunRow +from deerflow.persistence.run.model import RunChangeClockRow, RunRow from deerflow.persistence.scheduled_task_runs.model import ScheduledTaskRunRow from deerflow.persistence.scheduled_tasks.model import ScheduledTaskRow from deerflow.persistence.subagent_batches.model import SubagentBatchItemRow, SubagentBatchRow from deerflow.persistence.thread_meta.model import ThreadMetaRow -from deerflow.persistence.user.model import UserRow +from deerflow.persistence.user.model import UserPreferenceRow, UserRow from deerflow.persistence.webhook_delivery.model import WebhookDeliveryRow __all__ = [ @@ -48,12 +48,14 @@ __all__ = [ "ProjectDocumentRow", "ProjectRow", "RunEventRow", + "RunChangeClockRow", "RunRow", "ScheduledTaskRow", "ScheduledTaskRunRow", "SubagentBatchRow", "SubagentBatchItemRow", "ThreadMetaRow", + "UserPreferenceRow", "UserRow", "WebhookDeliveryRow", ] diff --git a/backend/tests/test_migration_0024_project_documents.py b/backend/tests/test_migration_0024_project_documents.py index e9e9d8e36..3e477decd 100644 --- a/backend/tests/test_migration_0024_project_documents.py +++ b/backend/tests/test_migration_0024_project_documents.py @@ -1,8 +1,8 @@ """Migration tests for 0024_project_documents (Phase-2 spec §6.1). -Pins the table shape and the four indexes on upgrade, the clean downgrade, -and the chain head (the forward-revision-compat pin moved here with 0023, -forwarded to 0024 after the rebase onto 0023_user_preferences). +Pins the table shape and the four indexes on upgrade, and the clean downgrade +(the chain-head pin moved on to 0025_repair_run_change_seq with that +revision). """ from __future__ import annotations @@ -13,7 +13,7 @@ import pytest import sqlalchemy as sa from alembic import command -from deerflow.persistence.bootstrap import _get_alembic_config, _get_head_revision +from deerflow.persistence.bootstrap import _get_alembic_config from deerflow.persistence.engine import close_engine, init_engine pytestmark = pytest.mark.asyncio @@ -67,10 +67,6 @@ async def _inspect(engine): return await conn.run_sync(_read) -async def test_0024_is_the_chain_head(): - assert _get_head_revision() == REVISION - - async def test_0024_upgrade_creates_table_and_indexes(tmp_path): engine = await _engine(tmp_path) try: diff --git a/backend/tests/test_migration_0025_repair_run_change_seq.py b/backend/tests/test_migration_0025_repair_run_change_seq.py new file mode 100644 index 000000000..28c125a81 --- /dev/null +++ b/backend/tests/test_migration_0025_repair_run_change_seq.py @@ -0,0 +1,145 @@ +"""Migration tests for 0025_repair_run_change_seq (#5516). + +``0023_run_change_seq`` was inserted ahead of the already-shipped +``0023_user_preferences`` revision, so databases stamped at that revision (or +later) never executed it and permanently lack the ``run_change_clock`` table +and ``runs.change_seq`` column. 0025 re-applies the same guarded DDL on +upgrade, heals those databases, and no-ops on healthy shapes. +""" + +from __future__ import annotations + +import asyncio +import sqlite3 + +import pytest +import sqlalchemy as sa +from alembic import command + +import deerflow.persistence.models # noqa: F401 -- registers ORM models +from deerflow.persistence.base import Base +from deerflow.persistence.bootstrap import _get_alembic_config, _get_head_revision +from deerflow.persistence.engine import close_engine, get_session_factory, init_engine +from deerflow.persistence.run import RunRepository + +pytestmark = pytest.mark.asyncio + +REVISION = "0025_repair_run_change_seq" +PREVIOUS = "0024_project_documents" +STAMP_BEFORE_INSERTION = "0023_user_preferences" + + +def _seed_database_that_skipped_0023(db_path) -> None: + """Build the #5516 shape: stamped past 0023_run_change_seq without running it. + + Mirrors a deployment that reached ``0023_user_preferences`` before + ``0023_run_change_seq`` was inserted ahead of it: the version row says the + revision is applied, so alembic never runs it, and the schema it owns is + missing. Uses the synchronous ``sqlite3``-backed engine so the seed is + independent of the async engine under test. + """ + 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) + with sync_engine.begin() as conn: + # Remove everything 0023_run_change_seq owns. + conn.execute(sa.text("DROP INDEX IF EXISTS ix_runs_change_seq")) + conn.execute(sa.text("DROP INDEX IF EXISTS ix_runs_user_change_seq")) + conn.execute(sa.text("ALTER TABLE runs DROP COLUMN change_seq")) + conn.execute(sa.text("DROP TABLE IF EXISTS run_change_clock")) + # 0024 had not run at this stamp either. + conn.execute(sa.text("DROP TABLE IF EXISTS project_documents")) + # Stamp the position such a deployment sat at. + conn.execute(sa.text("CREATE TABLE alembic_version (version_num VARCHAR(32) NOT NULL)")) + conn.execute(sa.text(f"INSERT INTO alembic_version (version_num) VALUES ('{STAMP_BEFORE_INSERTION}')")) + finally: + sync_engine.dispose() + + +def _table_and_column_state(db_path) -> tuple[bool, bool, set[str], str | None]: + with sqlite3.connect(db_path) as raw: + tables = {row[0] for row in raw.execute("SELECT name FROM sqlite_master WHERE type='table'").fetchall()} + run_columns = {row[1] for row in raw.execute("PRAGMA table_info(runs)").fetchall()} + run_indexes = {row[1] for row in raw.execute("PRAGMA index_list(runs)").fetchall()} + version_row = raw.execute("SELECT version_num FROM alembic_version").fetchone() + return "run_change_clock" in tables, "change_seq" in run_columns, run_indexes, version_row[0] if version_row else None + + +async def test_0025_is_the_chain_head(): + assert _get_head_revision() == REVISION + + +async def test_0025_repairs_schema_skipped_by_the_0023_insertion(tmp_path): + db_path = tmp_path / "skipped-0023.db" + _seed_database_that_skipped_0023(db_path) + + has_table, has_column, _, version = _table_and_column_state(db_path) + assert not has_table + assert not has_column + assert version == STAMP_BEFORE_INSERTION + + url = f"sqlite+aiosqlite:///{db_path.as_posix()}" + await init_engine(backend="sqlite", url=url, sqlite_dir=str(tmp_path)) + try: + has_table, has_column, run_indexes, version = _table_and_column_state(db_path) + assert has_table + assert has_column + assert {"ix_runs_change_seq", "ix_runs_user_change_seq"} <= run_indexes + assert version == _get_head_revision() + + # The exact call that 500'd in #5516: bumping the change clock. + sf = get_session_factory() + assert sf is not None + async with sf() as session: + assert await RunRepository._next_change_seq(session) == 1 + finally: + await close_engine() + + +async def test_0025_downgrade_preserves_ancestor_owned_schema_and_data(tmp_path): + """Rolling back only this repair must not remove schema owned by 0023. + + The change-clock schema belongs to ancestor ``0023_run_change_seq``; a + repair downgrade that dropped it would leave the database stamped at 0024 + without 0023's schema — recreating the #5516 hole — and would discard + allocated clock positions. + """ + db_path = tmp_path / "downgrade.db" + url = f"sqlite+aiosqlite:///{db_path.as_posix()}" + await init_engine("sqlite", url=url, sqlite_dir=str(tmp_path)) + try: + from deerflow.persistence.engine import get_engine + + engine = get_engine() + assert engine is not None + cfg = _get_alembic_config(engine) + + # Allocate a clock position so the data-preservation claim is real. + sf = get_session_factory() + assert sf is not None + async with sf() as session: + assert await RunRepository._next_change_seq(session) == 1 + await session.commit() + + await asyncio.to_thread(command.downgrade, cfg, PREVIOUS) + has_table, has_column, run_indexes, version = _table_and_column_state(db_path) + assert has_table + assert has_column + assert {"ix_runs_change_seq", "ix_runs_user_change_seq"} <= run_indexes + assert version == PREVIOUS + with sqlite3.connect(db_path) as raw: + assert raw.execute("SELECT value FROM run_change_clock WHERE id = 1").fetchone()[0] == 1 + + # Downgrade is idempotent; re-upgrading re-runs the guarded repair as a no-op. + await asyncio.to_thread(command.downgrade, cfg, PREVIOUS) + await asyncio.to_thread(command.upgrade, cfg, REVISION) + has_table, has_column, run_indexes, version = _table_and_column_state(db_path) + assert has_table + assert has_column + assert {"ix_runs_change_seq", "ix_runs_user_change_seq"} <= run_indexes + assert version == REVISION + with sqlite3.connect(db_path) as raw: + assert raw.execute("SELECT value FROM run_change_clock WHERE id = 1").fetchone()[0] == 1 + finally: + await close_engine()