fix(persistence): repair run-change clock schema skipped by the 0023 insertion (#5517)

0023_run_change_seq was chained 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: the
run_change_clock table and runs.change_seq column are permanently missing
and the first thread deletion fails with 'no such table:
run_change_clock' (#5516). 0025_repair_run_change_seq 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.

Fixes #5516

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
hataa 2026-09-18 10:48:26 +08:00 committed by GitHub
parent 16f154f32b
commit db6130861d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 239 additions and 13 deletions

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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",
]

View File

@ -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:

View File

@ -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()