diff --git a/alembic/versions/0148_plugin_migration_hash.py b/alembic/versions/0148_plugin_migration_hash.py new file mode 100644 index 0000000..59aa36e --- /dev/null +++ b/alembic/versions/0148_plugin_migration_hash.py @@ -0,0 +1,41 @@ +"""Add content_hash to plugin_migrations (F40/Astra). + +The migration runner previously tracked migrations by FILENAME only — +editing an already-applied migration stayed unnoticed (the #389 bug +class: a broken migration was fixed, but the runner silently skipped +it because the filename was already tracked). + +Now every applied migration records the SHA-256 of its SQL content. +On subsequent runs the runner compares hashes and logs a loud warning +when an applied migration was modified (repaired) — the skip stays +idempotent, but drift becomes VISIBLE. + +Revision ID: 0148 +Revises: 0147 +""" + +import sqlalchemy as sa + +from alembic import op + +revision = "0148" +down_revision = "0147" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "plugin_migrations", + sa.Column("content_hash", sa.String(64), nullable=True), + ) + op.create_index( + "ix_plugin_migrations_content_hash", + "plugin_migrations", + ["content_hash"], + ) + + +def downgrade() -> None: + op.drop_index("ix_plugin_migrations_content_hash", table_name="plugin_migrations") + op.drop_column("plugin_migrations", "content_hash") diff --git a/app/models/plugin.py b/app/models/plugin.py index 81485c8..69de3f2 100644 --- a/app/models/plugin.py +++ b/app/models/plugin.py @@ -58,3 +58,7 @@ class PluginMigration(Base, TimestampMixin): plugin_name: Mapped[str] = mapped_column(String(80), nullable=False) migration_file: Mapped[str] = mapped_column(String(255), nullable=False) status: Mapped[str] = mapped_column(String(20), nullable=False, default="applied") + # F40 (Astra): SHA-256 of the applied SQL content. The runner compares + # this on subsequent runs — a modified already-applied migration + # (repaired) becomes VISIBLE instead of being silently skipped. + content_hash: Mapped[str | None] = mapped_column(String(64), nullable=True) diff --git a/app/plugins/migration_runner.py b/app/plugins/migration_runner.py index d1c7de3..7e65a5f 100644 --- a/app/plugins/migration_runner.py +++ b/app/plugins/migration_runner.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging import os from pathlib import Path from typing import Any @@ -11,6 +12,8 @@ from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession from app.models.plugin import PluginMigration +logger = logging.getLogger(__name__) + class MigrationValidationError(Exception): """Raised when a plugin migration creates a table without tenant_id column.""" @@ -124,12 +127,20 @@ class MigrationRunner: plugin_name=plugin_name, migration_file=migration_filename, status="applied", + content_hash=self._hash_sql(sql_content), # F40 (Astra): record applied content ) db.add(migration_record) await db.flush() return migration_record + @staticmethod + def _hash_sql(sql_content: str) -> str: + """F40: SHA-256 of migration SQL content for drift detection.""" + import hashlib + + return hashlib.sha256(sql_content.encode("utf-8")).hexdigest() + async def run_all_migrations( self, db: AsyncSession, @@ -150,11 +161,34 @@ class MigrationRunner: PluginMigration.status == "applied", ) ) - applied_files = {row.migration_file for row in result.scalars().all()} + applied_rows = {row.migration_file: row for row in result.scalars().all()} + applied_files = set(applied_rows.keys()) records: list[PluginMigration] = [] for filename in migration_files: if filename in applied_files: + # F40 (Astra): hash check — a modified already-applied + # migration must be VISIBLE, not silently skipped. This is + # exactly the #389 bug class: a broken migration was fixed + # on disk, but the runner skipped it by filename forever. + record = applied_rows[filename] + try: + path = self._resolve_migration_path(filename, plugin_name) + current_hash = self._hash_sql(path.read_text(encoding="utf-8")) + except FileNotFoundError: + continue # file gone (plugin removed it) — nothing to compare + if record.content_hash is not None and record.content_hash != current_hash: + logger.warning( + "F40 DRIFT: plugin '%s' migration '%s' was MODIFIED after being " + "applied (recorded=%s current=%s). The runner keeps the skip " + "(idempotent), but the schema may no longer match the file. " + "If the change is intentional, verify the schema and consider " + "a follow-up migration instead of editing an applied one.", + plugin_name, + filename, + (record.content_hash or "")[:12], + current_hash[:12], + ) continue # Already applied — idempotent skip record = await self.run_migration(db, plugin_name, filename, tenant_id) records.append(record)