diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d53321..0b7e1a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Post-v1.2.0 commits on `master`. No version bump yet. +### Added +- New external dependency on `civiccore` (pinned to `git+https://github.com/CivicSuite/civiccore.git@e7c5570` during Phase 1; will become a versioned wheel pin before v1.3.0). +- Backend migrations now run the `civiccore` shared-schema baseline before records' own chain via `backend/alembic/env.py`. See [ADR-0003](https://github.com/CivicSuite/civicsuite/blob/main/docs/architecture/ADR-0003-civiccore-alembic-baseline-strategy.md) for the rationale and gate contract. +- 3 migration gate tests at `backend/tests/test_civiccore_migration_gates.py` covering fresh-install, v1.2.x upgrade, and reapplication idempotency scenarios. + +### Changed +- `Dockerfile.backend` now installs `git` during image build so pip can resolve the temporary `git+https` civiccore dependency. +- 14 records migrations updated to use `civiccore.migrations.guards.idempotent_*` helpers, making them safe to re-apply on databases where the civiccore baseline has already created the shared tables (users, service_accounts, audit_log, data_sources, documents, document_chunks, model_registry, exemption_rules, connector_templates, departments, system_catalog, city_profile, notification_templates, prompt_templates, sync_run_log, sync_failures). + ## [1.2.0] - 2026-04-23 Tier 5 installer/onboarding/seeding/model-picker/portal-mode slices and Tier 6 at-rest encryption (ENG-001 closed) tagged together. CI green on `d556904` (run 24853147133). Unsigned Windows `.exe` installer produced on tag push via Inno Setup 6.x. diff --git a/Dockerfile.backend b/Dockerfile.backend index b6961a8..fe7205e 100644 --- a/Dockerfile.backend +++ b/Dockerfile.backend @@ -4,7 +4,11 @@ WORKDIR /app RUN apt-get update && apt-get install -y --no-install-recommends \ build-essential libpq-dev curl \ + git \ && rm -rf /var/lib/apt/lists/* +# git is required by pip to fetch the civiccore dependency from +# git+https://github.com/CivicSuite/civiccore.git@ until civiccore +# 0.1.0 is published to PyPI. Once published, this line can drop git. COPY backend/pyproject.toml . RUN pip install --no-cache-dir ".[dev]" diff --git a/README.md b/README.md index 020d3e5..8426792 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,12 @@ bash install.sh 4. Click **Ingest Now** — documents are parsed, chunked, and indexed automatically 5. Go to **Search** — type a natural language query and get cited results +### Phase 1 migration layer + +CivicRecords AI backend installs `civiccore` (the shared CivicSuite schema + migration runtime) as a dependency. During the Phase 1 transition period `civiccore` is pinned by git+SHA rather than PyPI version; see `backend/pyproject.toml`. Because of this, the backend Docker build requires `git` inside the image (already wired in `Dockerfile.backend`). + +Migrations run in two layers: `civiccore` first (creates/updates the 16 shared tables), then this repo's Alembic chain on top. See [ADR-0003](https://github.com/CivicSuite/civicsuite/blob/main/docs/architecture/ADR-0003-civiccore-alembic-baseline-strategy.md) for the full gate contract. + ## Architecture ``` diff --git a/backend/alembic/env.py b/backend/alembic/env.py index 80a062d..45691d7 100644 --- a/backend/alembic/env.py +++ b/backend/alembic/env.py @@ -9,7 +9,16 @@ config = context.config if config.config_file_name is not None: - fileConfig(config.config_file_name) + # Phase 1 Part B: programmatic alembic invocation (test_civiccore_migration_gates) + # runs env.py in the pytest parent process. fileConfig's default + # disable_existing_loggers=True sets .disabled=True on every pre-existing + # app.* logger, which survives beyond the migration call. Subsequent tests + # that rely on caplog (e.g. test_structured_log_on_fetch_failure) then see + # empty caplog.records because pytest's caplog.at_level does NOT reset the + # .disabled attribute — it only adjusts .level and logging.disable(). + # disable_existing_loggers=False is the correct posture for alembic invoked + # from within a Python app that owns its own logger hierarchy. + fileConfig(config.config_file_name, disable_existing_loggers=False) target_metadata = Base.metadata @@ -26,6 +35,28 @@ def run_migrations_offline() -> None: def do_run_migrations(connection): + # Phase-1 (civiccore extraction): bring civiccore migrations to head BEFORE + # records' own chain. See ADR-0003 §3. + # + # Civiccore is invoked in a subprocess rather than inline because alembic's + # `context._proxy` is process-global, not stacked. Calling civiccore's + # `command.upgrade` (even on a separate connection) would tear down records' + # active context proxy on civiccore's __exit__, causing + # `AttributeError: 'NoneType' object has no attribute 'configure'` on the + # next records env.py line. A subprocess gives civiccore a clean alembic + # process. Civiccore opens its own DB connection from DATABASE_URL. + import subprocess + import sys + + subprocess.run( + [ + sys.executable, + "-c", + "from civiccore.migrations.runner import upgrade_to_head; upgrade_to_head()", + ], + check=True, + ) + context.configure(connection=connection, target_metadata=target_metadata) with context.begin_transaction(): context.run_migrations() diff --git a/backend/alembic/versions/001_initial.py b/backend/alembic/versions/001_initial.py index 9445d84..dbf9f4d 100644 --- a/backend/alembic/versions/001_initial.py +++ b/backend/alembic/versions/001_initial.py @@ -11,6 +11,11 @@ import sqlalchemy as sa from sqlalchemy.dialects import postgresql +from civiccore.migrations.guards import ( + idempotent_create_index, + idempotent_create_table, +) + revision: str = "001" down_revision: Union[str, None] = None branch_labels: Union[str, Sequence[str], None] = None @@ -23,7 +28,7 @@ def upgrade() -> None: # Users table (fastapi-users compatible) # The Enum with create_type=True on the first table creates the user_role type - op.create_table( + idempotent_create_table( "users", sa.Column("id", fastapi_users_db_sqlalchemy.generics.GUID(), nullable=False), sa.Column("email", sa.String(length=320), nullable=False), @@ -37,10 +42,10 @@ def upgrade() -> None: sa.Column("last_login", sa.DateTime(timezone=True), nullable=True), sa.PrimaryKeyConstraint("id"), ) - op.create_index("ix_users_email", "users", ["email"], unique=True) + idempotent_create_index("ix_users_email", "users", ["email"], unique=True) # Service accounts table - op.create_table( + idempotent_create_table( "service_accounts", sa.Column("id", sa.UUID(), nullable=False), sa.Column("name", sa.String(255), nullable=False), @@ -54,7 +59,7 @@ def upgrade() -> None: ) # Audit log table (append-only, hash-chained) - op.create_table( + idempotent_create_table( "audit_log", sa.Column("id", sa.Integer(), autoincrement=True, nullable=False), sa.Column("prev_hash", sa.String(64), nullable=False, server_default="0" * 64), @@ -68,12 +73,12 @@ def upgrade() -> None: sa.Column("ai_generated", sa.Boolean(), nullable=False, server_default=sa.text("false")), sa.PrimaryKeyConstraint("id"), ) - op.create_index("ix_audit_log_entry_hash", "audit_log", ["entry_hash"]) - op.create_index("ix_audit_log_timestamp", "audit_log", ["timestamp"]) - op.create_index("ix_audit_log_action", "audit_log", ["action"]) - op.create_index("ix_audit_log_resource_type", "audit_log", ["resource_type"]) - op.create_index("ix_audit_log_user_id", "audit_log", ["user_id"]) - op.create_index("ix_audit_log_user_timestamp", "audit_log", ["user_id", "timestamp"]) + idempotent_create_index("ix_audit_log_entry_hash", "audit_log", ["entry_hash"]) + idempotent_create_index("ix_audit_log_timestamp", "audit_log", ["timestamp"]) + idempotent_create_index("ix_audit_log_action", "audit_log", ["action"]) + idempotent_create_index("ix_audit_log_resource_type", "audit_log", ["resource_type"]) + idempotent_create_index("ix_audit_log_user_id", "audit_log", ["user_id"]) + idempotent_create_index("ix_audit_log_user_timestamp", "audit_log", ["user_id", "timestamp"]) def downgrade() -> None: diff --git a/backend/alembic/versions/002_documents.py b/backend/alembic/versions/002_documents.py index 04b5aa3..14cfd3d 100644 --- a/backend/alembic/versions/002_documents.py +++ b/backend/alembic/versions/002_documents.py @@ -10,13 +10,18 @@ from sqlalchemy.dialects import postgresql from pgvector.sqlalchemy import Vector +from civiccore.migrations.guards import ( + idempotent_create_index, + idempotent_create_table, +) + revision: str = "002" down_revision: Union[str, None] = "001" branch_labels: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None def upgrade() -> None: - op.create_table("data_sources", + idempotent_create_table("data_sources", sa.Column("id", sa.UUID(), nullable=False), sa.Column("name", sa.String(255), nullable=False), sa.Column("source_type", sa.Enum("upload", "directory", name="source_type", create_type=True), nullable=False), @@ -29,7 +34,7 @@ def upgrade() -> None: sa.PrimaryKeyConstraint("id"), sa.UniqueConstraint("name"), ) - op.create_table("documents", + idempotent_create_table("documents", sa.Column("id", sa.UUID(), nullable=False), sa.Column("source_id", sa.UUID(), sa.ForeignKey("data_sources.id"), nullable=False), sa.Column("source_path", sa.Text(), nullable=False), @@ -44,10 +49,10 @@ def upgrade() -> None: sa.Column("metadata", postgresql.JSONB(), nullable=True), sa.PrimaryKeyConstraint("id"), ) - op.create_index("ix_documents_source_id", "documents", ["source_id"]) - op.create_index("ix_documents_file_hash", "documents", ["file_hash"]) - op.create_index("ix_documents_source_hash", "documents", ["source_id", "file_hash"]) - op.create_table("document_chunks", + idempotent_create_index("ix_documents_source_id", "documents", ["source_id"]) + idempotent_create_index("ix_documents_file_hash", "documents", ["file_hash"]) + idempotent_create_index("ix_documents_source_hash", "documents", ["source_id", "file_hash"]) + idempotent_create_table("document_chunks", sa.Column("id", sa.UUID(), nullable=False), sa.Column("document_id", sa.UUID(), sa.ForeignKey("documents.id", ondelete="CASCADE"), nullable=False), sa.Column("chunk_index", sa.Integer(), nullable=False), @@ -57,8 +62,8 @@ def upgrade() -> None: sa.Column("page_number", sa.Integer(), nullable=True), sa.PrimaryKeyConstraint("id"), ) - op.create_index("ix_document_chunks_document_id", "document_chunks", ["document_id"]) - op.create_index("ix_chunks_doc_index", "document_chunks", ["document_id", "chunk_index"]) + idempotent_create_index("ix_document_chunks_document_id", "document_chunks", ["document_id"]) + idempotent_create_index("ix_chunks_doc_index", "document_chunks", ["document_id", "chunk_index"]) def downgrade() -> None: op.drop_table("document_chunks") diff --git a/backend/alembic/versions/003_model_registry.py b/backend/alembic/versions/003_model_registry.py index dbdfcd4..2609124 100644 --- a/backend/alembic/versions/003_model_registry.py +++ b/backend/alembic/versions/003_model_registry.py @@ -8,13 +8,15 @@ from alembic import op import sqlalchemy as sa +from civiccore.migrations.guards import idempotent_create_table + revision: str = "003" down_revision: Union[str, None] = "002" branch_labels: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None def upgrade() -> None: - op.create_table("model_registry", + idempotent_create_table("model_registry", sa.Column("id", sa.Integer(), autoincrement=True, nullable=False), sa.Column("model_name", sa.String(255), nullable=False), sa.Column("model_version", sa.String(100), nullable=True), diff --git a/backend/alembic/versions/004_search.py b/backend/alembic/versions/004_search.py index d7a3363..50ddfe1 100644 --- a/backend/alembic/versions/004_search.py +++ b/backend/alembic/versions/004_search.py @@ -49,17 +49,21 @@ def upgrade() -> None: op.create_index("ix_search_results_query_id", "search_results", ["query_id"]) op.create_index("ix_search_results_chunk_id", "search_results", ["chunk_id"]) - # Add tsvector column to document_chunks for full-text search + # Add tsvector column to document_chunks for full-text search. + # IF NOT EXISTS keeps this idempotent: civiccore's baseline migration + # (per ADR-0003) creates document_chunks with this column already present; + # records' own history still applies this op for v1.2.x deployments that + # predate the civiccore extraction. Both paths converge on the same schema. op.execute(""" ALTER TABLE document_chunks - ADD COLUMN content_tsvector tsvector + ADD COLUMN IF NOT EXISTS content_tsvector tsvector GENERATED ALWAYS AS (to_tsvector('english', content_text)) STORED """) - op.execute("CREATE INDEX ix_chunks_tsvector ON document_chunks USING GIN (content_tsvector)") + op.execute("CREATE INDEX IF NOT EXISTS ix_chunks_tsvector ON document_chunks USING GIN (content_tsvector)") # HNSW index on embedding column for fast semantic search op.execute(""" - CREATE INDEX ix_chunks_embedding_hnsw ON document_chunks + CREATE INDEX IF NOT EXISTS ix_chunks_embedding_hnsw ON document_chunks USING hnsw (embedding vector_cosine_ops) WITH (m = 16, ef_construction = 64) """) diff --git a/backend/alembic/versions/006_exemptions.py b/backend/alembic/versions/006_exemptions.py index dfb449e..5a1e9d6 100644 --- a/backend/alembic/versions/006_exemptions.py +++ b/backend/alembic/versions/006_exemptions.py @@ -9,6 +9,11 @@ import sqlalchemy as sa from sqlalchemy.dialects import postgresql +from civiccore.migrations.guards import ( + idempotent_create_index, + idempotent_create_table, +) + revision: str = "006" down_revision: Union[str, None] = "005" branch_labels: Union[str, Sequence[str], None] = None @@ -16,7 +21,7 @@ def upgrade() -> None: - op.create_table("exemption_rules", + idempotent_create_table("exemption_rules", sa.Column("id", sa.UUID(), nullable=False), sa.Column("state_code", sa.String(2), nullable=False), sa.Column("category", sa.String(100), nullable=False), @@ -28,8 +33,8 @@ def upgrade() -> None: sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now()), sa.PrimaryKeyConstraint("id"), ) - op.create_index("ix_exemption_rules_state", "exemption_rules", ["state_code"]) - op.create_index("ix_exemption_rules_category", "exemption_rules", ["category"]) + idempotent_create_index("ix_exemption_rules_state", "exemption_rules", ["state_code"]) + idempotent_create_index("ix_exemption_rules_category", "exemption_rules", ["category"]) op.create_table("exemption_flags", sa.Column("id", sa.UUID(), nullable=False), diff --git a/backend/alembic/versions/011_fix_schema_drift.py b/backend/alembic/versions/011_fix_schema_drift.py index 2e99387..e4aec9e 100644 --- a/backend/alembic/versions/011_fix_schema_drift.py +++ b/backend/alembic/versions/011_fix_schema_drift.py @@ -51,6 +51,8 @@ from alembic import op import sqlalchemy as sa +from civiccore.migrations.guards import idempotent_add_column + # revision identifiers revision: str = '011_fix_drift' @@ -76,7 +78,8 @@ def upgrade() -> None: # server_default ensures any pre-existing rows backfill cleanly. The # model declares default=1 (Python-side) so new ORM inserts will also # pass an explicit value. - op.add_column( + # SHARED table (exemption_rules is CivicCore-owned) — guarded. + idempotent_add_column( "exemption_rules", sa.Column("version", sa.Integer(), nullable=False, server_default="1"), ) diff --git a/backend/alembic/versions/012_add_liaison_public_roles.py b/backend/alembic/versions/012_add_liaison_public_roles.py index bd36205..8e9fa03 100644 --- a/backend/alembic/versions/012_add_liaison_public_roles.py +++ b/backend/alembic/versions/012_add_liaison_public_roles.py @@ -32,6 +32,12 @@ def upgrade() -> None: # PostgreSQL 12+ allows ALTER TYPE ... ADD VALUE IF NOT EXISTS inside a transaction. # The project targets PostgreSQL 17, so the prior COMMIT workaround (which breaks # asyncpg's protocol-level transaction management) is not needed. + # + # The user_role enum is a SHARED (CivicCore-owned) type — civiccore baseline + # already declares the full role set including 'liaison' and 'public', so on + # any DB that ran the baseline these statements no-op via ADD VALUE IF NOT + # EXISTS. Native Postgres idempotency is sufficient here; no `has_table` + # gate is needed because we are mutating a TYPE, not a table. op.execute(sa.text("ALTER TYPE user_role ADD VALUE IF NOT EXISTS 'liaison'")) op.execute(sa.text("ALTER TYPE user_role ADD VALUE IF NOT EXISTS 'public'")) diff --git a/backend/alembic/versions/013_add_connector_types.py b/backend/alembic/versions/013_add_connector_types.py index 8630801..d6d05d2 100644 --- a/backend/alembic/versions/013_add_connector_types.py +++ b/backend/alembic/versions/013_add_connector_types.py @@ -8,6 +8,8 @@ import sqlalchemy as sa from alembic import op +from civiccore.migrations.guards import idempotent_add_column + revision: str = '013_connector_types' down_revision: Union[str, None] = '012_liaison_public_roles' branch_labels: Union[str, Sequence[str], None] = None @@ -15,13 +17,16 @@ def upgrade() -> None: - # Add new SourceType enum values - # PostgreSQL requires ALTER TYPE ... ADD VALUE; cannot be done inside a transaction. + # Add new SourceType enum values. + # source_type is a SHARED (CivicCore-owned) enum, but ADD VALUE IF NOT EXISTS + # is natively idempotent — civiccore baseline can pre-declare these and the + # statements still run cleanly here. op.execute("ALTER TYPE source_type ADD VALUE IF NOT EXISTS 'rest_api'") op.execute("ALTER TYPE source_type ADD VALUE IF NOT EXISTS 'odbc'") - # Add last_sync_cursor column (last_sync_at already exists) - op.add_column( + # Add last_sync_cursor column on SHARED data_sources table — guarded. + # (last_sync_at already exists from 787207afc66a.) + idempotent_add_column( "data_sources", sa.Column("last_sync_cursor", sa.String(), nullable=True), ) diff --git a/backend/alembic/versions/014_p6a_idempotency.py b/backend/alembic/versions/014_p6a_idempotency.py index b8d92b6..cbeee4e 100644 --- a/backend/alembic/versions/014_p6a_idempotency.py +++ b/backend/alembic/versions/014_p6a_idempotency.py @@ -13,6 +13,12 @@ import sqlalchemy as sa from alembic import op +from civiccore.migrations.guards import ( + idempotent_add_column, + idempotent_create_check_constraint, + idempotent_create_index, +) + revision: str = '014_p6a_idempotency' down_revision: Union[str, None] = '013_connector_types' branch_labels: Union[str, Sequence[str], None] = None @@ -20,14 +26,15 @@ def upgrade() -> None: - # 1. Add connector_type column to documents (denormalized from data_sources.source_type) - op.add_column( + # 1. Add connector_type column to SHARED documents table — guarded. + # (denormalized from data_sources.source_type) + idempotent_add_column( "documents", sa.Column("connector_type", sa.String(20), nullable=True), ) - # 2. Add updated_at column to documents - op.add_column( + # 2. Add updated_at column to SHARED documents table — guarded. + idempotent_add_column( "documents", sa.Column("updated_at", sa.DateTime(timezone=True), nullable=True), ) @@ -55,7 +62,7 @@ def upgrade() -> None: """) # 5. Add source_path max length constraint (2048 chars) - op.create_check_constraint( + idempotent_create_check_constraint( "chk_source_path_length", "documents", "source_path IS NULL OR length(source_path) <= 2048", @@ -63,7 +70,7 @@ def upgrade() -> None: # 6. Partial UNIQUE index for binary connectors: dedup by (source_id, file_hash) # Excludes structured connectors (rest_api, odbc). - op.create_index( + idempotent_create_index( "uq_documents_binary_hash", "documents", ["source_id", "file_hash"], @@ -72,7 +79,7 @@ def upgrade() -> None: ) # 7. Partial UNIQUE index for structured connectors: dedup by (source_id, source_path) - op.create_index( + idempotent_create_index( "uq_documents_structured_path", "documents", ["source_id", "source_path"], diff --git a/backend/alembic/versions/015_p6b_scheduler.py b/backend/alembic/versions/015_p6b_scheduler.py index 2eca92e..f4ae67c 100644 --- a/backend/alembic/versions/015_p6b_scheduler.py +++ b/backend/alembic/versions/015_p6b_scheduler.py @@ -12,7 +12,12 @@ import sqlalchemy as sa from alembic import op -from sqlalchemy import text +from sqlalchemy import inspect, text + +from civiccore.migrations.guards import ( + has_table, + idempotent_add_column, +) revision: str = '015_p6b_scheduler' down_revision: Union[str, None] = '014_p6a_idempotency' @@ -39,17 +44,27 @@ def upgrade() -> None: conn = op.get_bind() - op.add_column( + # SHARED data_sources column — guarded. + idempotent_add_column( "data_sources", sa.Column("schedule_enabled", sa.Boolean(), nullable=False, server_default="true"), ) - op.create_check_constraint( - "chk_sync_schedule_nonempty", - "data_sources", - "sync_schedule IS NULL OR length(trim(sync_schedule)) > 0", + # check_constraint targets shared data_sources. Postgres lacks + # `ADD CONSTRAINT IF NOT EXISTS` for CHECK, so use a DO block that + # checks pg_constraint first. Idempotent on fresh installs (baseline + # doesn't add this) and on re-runs. + op.execute( + "DO $$ BEGIN " + "IF NOT EXISTS (" + " SELECT 1 FROM pg_constraint WHERE conname = 'chk_sync_schedule_nonempty'" + ") THEN " + " ALTER TABLE data_sources ADD CONSTRAINT chk_sync_schedule_nonempty " + " CHECK (sync_schedule IS NULL OR length(trim(sync_schedule)) > 0); " + "END IF; END $$;" ) + # Records-only transient workspace table — leave unguarded per parent spec. op.create_table( "_migration_015_report", sa.Column("id", sa.Integer(), primary_key=True, autoincrement=True), @@ -62,9 +77,23 @@ def upgrade() -> None: sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now()), ) - rows = conn.execute( - text("SELECT id, name, schedule_minutes FROM data_sources WHERE schedule_minutes IS NOT NULL") - ).fetchall() + # Data migration: convert legacy schedule_minutes to cron sync_schedule. + # On a fresh install (civiccore baseline created data_sources WITHOUT the + # legacy schedule_minutes column, since baseline reflects records HEAD + # state where 015 already dropped it), the column does not exist and the + # SELECT would raise. Gate the entire conversion on column presence. + insp = inspect(conn) + has_legacy_col = ( + has_table("data_sources") + and "schedule_minutes" in {c["name"] for c in insp.get_columns("data_sources")} + ) + rows = ( + conn.execute( + text("SELECT id, name, schedule_minutes FROM data_sources WHERE schedule_minutes IS NOT NULL") + ).fetchall() + if has_legacy_col + else [] + ) for row in rows: source_id, name, minutes = str(row[0]), row[1], row[2] @@ -109,8 +138,11 @@ def upgrade() -> None: f"Admin action required." ) - op.drop_column("data_sources", "schedule_minutes") + # SHARED data_sources column drop — guarded with IF EXISTS so a fresh + # install (where baseline never created this legacy column) succeeds. + op.execute("ALTER TABLE data_sources DROP COLUMN IF EXISTS schedule_minutes") + # SHARED data_sources columns — each guarded. for col_def in [ sa.Column("consecutive_failure_count", sa.Integer(), nullable=False, server_default="0"), sa.Column("last_error_message", sa.String(500), nullable=True), @@ -121,7 +153,7 @@ def upgrade() -> None: sa.Column("retry_batch_size", sa.Integer(), nullable=True), sa.Column("retry_time_limit_seconds", sa.Integer(), nullable=True), ]: - op.add_column("data_sources", col_def) + idempotent_add_column("data_sources", col_def) def downgrade() -> None: diff --git a/backend/alembic/versions/016_p7_sync_failures.py b/backend/alembic/versions/016_p7_sync_failures.py index c01ae8c..c266984 100644 --- a/backend/alembic/versions/016_p7_sync_failures.py +++ b/backend/alembic/versions/016_p7_sync_failures.py @@ -10,6 +10,8 @@ import sqlalchemy as sa from alembic import op +from civiccore.migrations.guards import idempotent_create_table + revision: str = '016_p7_sync_failures' down_revision: Union[str, None] = '015_p6b_scheduler' branch_labels: Union[str, Sequence[str], None] = None @@ -17,8 +19,8 @@ def upgrade() -> None: - # 1. sync_failures table - op.create_table( + # 1. sync_failures table (SHARED — guarded) + idempotent_create_table( "sync_failures", sa.Column("id", sa.dialects.postgresql.UUID(as_uuid=True), primary_key=True, server_default=sa.text("gen_random_uuid()")), @@ -38,11 +40,19 @@ def upgrade() -> None: sa.Column("dismissed_by", sa.dialects.postgresql.UUID(as_uuid=True), sa.ForeignKey("users.id"), nullable=True), ) - op.create_index("ix_sync_failures_source_status", "sync_failures", ["source_id", "status"]) - op.create_index("ix_sync_failures_created", "sync_failures", ["first_failed_at"]) + # Indexes on SHARED sync_failures — baseline may have already created + # these. Use IF NOT EXISTS for native idempotency. + op.execute( + "CREATE INDEX IF NOT EXISTS ix_sync_failures_source_status " + "ON sync_failures (source_id, status)" + ) + op.execute( + "CREATE INDEX IF NOT EXISTS ix_sync_failures_created " + "ON sync_failures (first_failed_at)" + ) - # 2. sync_run_log table - op.create_table( + # 2. sync_run_log table (SHARED — guarded) + idempotent_create_table( "sync_run_log", sa.Column("id", sa.dialects.postgresql.UUID(as_uuid=True), primary_key=True, server_default=sa.text("gen_random_uuid()")), @@ -57,7 +67,11 @@ def upgrade() -> None: sa.Column("records_failed", sa.Integer(), server_default="0"), sa.Column("error_summary", sa.Text(), nullable=True), ) - op.create_index("ix_sync_run_log_source", "sync_run_log", ["source_id", "started_at"]) + # Index on SHARED sync_run_log — IF NOT EXISTS for baseline coexistence. + op.execute( + "CREATE INDEX IF NOT EXISTS ix_sync_run_log_source " + "ON sync_run_log (source_id, started_at)" + ) # NOTE: The eight DataSource tracking columns (consecutive_failure_count, sync_paused, etc.) # are added by migration 015 (P6b) as nullable stubs. They MUST NOT be re-added here. diff --git a/backend/alembic/versions/017_rename_connector_enum_values.py b/backend/alembic/versions/017_rename_connector_enum_values.py index 21ff763..b48868c 100644 --- a/backend/alembic/versions/017_rename_connector_enum_values.py +++ b/backend/alembic/versions/017_rename_connector_enum_values.py @@ -20,8 +20,28 @@ def upgrade() -> None: - op.execute("ALTER TYPE source_type RENAME VALUE 'upload' TO 'manual_drop'") - op.execute("ALTER TYPE source_type RENAME VALUE 'directory' TO 'file_system'") + # source_type is a SHARED (CivicCore-owned) enum. On a fresh install the + # civiccore baseline declares it with the canonical post-rename values + # ('manual_drop', 'file_system') already, so 'upload'/'directory' do not + # exist and the bare RENAME would raise. Gate each rename on the source + # value's presence in pg_enum so the migration is a no-op when the + # values already match the target shape. + op.execute( + "DO $$ BEGIN " + "IF EXISTS (SELECT 1 FROM pg_enum e " + "JOIN pg_type t ON e.enumtypid = t.oid " + "WHERE t.typname = 'source_type' AND e.enumlabel = 'upload') THEN " + " ALTER TYPE source_type RENAME VALUE 'upload' TO 'manual_drop'; " + "END IF; END $$;" + ) + op.execute( + "DO $$ BEGIN " + "IF EXISTS (SELECT 1 FROM pg_enum e " + "JOIN pg_type t ON e.enumtypid = t.oid " + "WHERE t.typname = 'source_type' AND e.enumlabel = 'directory') THEN " + " ALTER TYPE source_type RENAME VALUE 'directory' TO 'file_system'; " + "END IF; END $$;" + ) def downgrade() -> None: diff --git a/backend/alembic/versions/018_city_profile_state_nullable.py b/backend/alembic/versions/018_city_profile_state_nullable.py index bb0500a..81fc8bd 100644 --- a/backend/alembic/versions/018_city_profile_state_nullable.py +++ b/backend/alembic/versions/018_city_profile_state_nullable.py @@ -19,6 +19,8 @@ import sqlalchemy as sa from alembic import op +from civiccore.migrations.guards import idempotent_alter_column + revision: str = '018_city_profile_state_nullable' down_revision: Union[str, None] = '017_rename_connector_enum_values' branch_labels: Union[str, Sequence[str], None] = None @@ -26,7 +28,10 @@ def upgrade() -> None: - op.alter_column( + # SHARED city_profile column — guarded. On fresh install, baseline + # already creates city_profile.state with nullable=True (HEAD shape), + # so this is a no-op via the helper's introspection check. + idempotent_alter_column( 'city_profile', 'state', existing_type=sa.String(length=2), diff --git a/backend/alembic/versions/019_encrypt_connection_config.py b/backend/alembic/versions/019_encrypt_connection_config.py index 2f42bcd..ed6f5d0 100644 --- a/backend/alembic/versions/019_encrypt_connection_config.py +++ b/backend/alembic/versions/019_encrypt_connection_config.py @@ -43,6 +43,7 @@ from alembic import op from app.security.at_rest import decrypt_json, encrypt_json, is_encrypted +from civiccore.migrations.guards import has_table revision: str = '019_encrypt_connection_config' down_revision: Union[str, None] = '018_city_profile_state_nullable' @@ -80,7 +81,21 @@ def _write_row(conn, row_id, new_value) -> None: def upgrade() -> None: - """Encrypt every plaintext row. Skip already-encrypted rows.""" + """Encrypt every plaintext row. Skip already-encrypted rows. + + SHARED data_sources table — gated on table presence so the data + migration is a clean no-op if civiccore baseline (or any future + migration order) has not yet created it. The per-row ``is_encrypted`` + check below is the authoritative post-migration-state guard: re-runs + against an already-encrypted column skip every row and report + encrypted=0. + """ + if not has_table("data_sources"): + print( + "[019_encrypt_connection_config] upgrade skipped: " + "data_sources table not present" + ) + return conn = op.get_bind() encrypted = 0 skipped = 0 @@ -108,7 +123,16 @@ def upgrade() -> None: def downgrade() -> None: - """Decrypt every envelope row back to plaintext. Skip already-plaintext.""" + """Decrypt every envelope row back to plaintext. Skip already-plaintext. + + Mirrors ``upgrade``: gated on shared table presence. + """ + if not has_table("data_sources"): + print( + "[019_encrypt_connection_config] downgrade skipped: " + "data_sources table not present" + ) + return conn = op.get_bind() decrypted = 0 skipped = 0 diff --git a/backend/alembic/versions/787207afc66a_phase2_extensions_12_new_tables_and_.py b/backend/alembic/versions/787207afc66a_phase2_extensions_12_new_tables_and_.py index 08f2234..df0c5c8 100644 --- a/backend/alembic/versions/787207afc66a_phase2_extensions_12_new_tables_and_.py +++ b/backend/alembic/versions/787207afc66a_phase2_extensions_12_new_tables_and_.py @@ -11,6 +11,12 @@ from sqlalchemy.dialects import postgresql import fastapi_users_db_sqlalchemy.generics +from civiccore.migrations.guards import ( + idempotent_add_column, + idempotent_create_foreign_key, + idempotent_create_table, +) + # revision identifiers revision: str = '787207afc66a' down_revision: Union[str, None] = '006' @@ -21,7 +27,7 @@ def upgrade() -> None: # === 12 New Tables === - op.create_table('connector_templates', + idempotent_create_table('connector_templates', sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), sa.Column('vendor_name', sa.String(length=200), nullable=False), sa.Column('protocol', sa.String(length=50), nullable=False), @@ -36,7 +42,7 @@ def upgrade() -> None: sa.PrimaryKeyConstraint('id') ) - op.create_table('departments', + idempotent_create_table('departments', sa.Column('id', sa.Uuid(), nullable=False), sa.Column('name', sa.String(length=200), nullable=False), sa.Column('code', sa.String(length=20), nullable=False), @@ -46,7 +52,7 @@ def upgrade() -> None: sa.UniqueConstraint('code') ) - op.create_table('system_catalog', + idempotent_create_table('system_catalog', sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), sa.Column('domain', sa.String(length=100), nullable=False), sa.Column('function', sa.String(length=200), nullable=False), @@ -63,7 +69,7 @@ def upgrade() -> None: sa.PrimaryKeyConstraint('id') ) - op.create_table('city_profile', + idempotent_create_table('city_profile', sa.Column('id', sa.Uuid(), nullable=False), sa.Column('city_name', sa.String(length=200), nullable=False), sa.Column('state', sa.String(length=2), nullable=False), @@ -95,7 +101,7 @@ def upgrade() -> None: sa.PrimaryKeyConstraint('id') ) - op.create_table('notification_templates', + idempotent_create_table('notification_templates', sa.Column('id', sa.Uuid(), nullable=False), sa.Column('event_type', sa.String(length=50), nullable=False), sa.Column('channel', sa.String(length=20), nullable=False), @@ -109,7 +115,7 @@ def upgrade() -> None: sa.UniqueConstraint('event_type') ) - op.create_table('prompt_templates', + idempotent_create_table('prompt_templates', sa.Column('id', sa.Uuid(), nullable=False), sa.Column('name', sa.String(length=200), nullable=False), sa.Column('purpose', sa.String(length=50), nullable=False), @@ -207,21 +213,21 @@ def upgrade() -> None: # === Add new columns to existing tables === - # data_sources - op.add_column('data_sources', sa.Column('discovered_source_id', sa.Uuid(), nullable=True)) - op.add_column('data_sources', sa.Column('connector_template_id', sa.Integer(), nullable=True)) - op.add_column('data_sources', sa.Column('sync_schedule', sa.String(50), nullable=True)) - op.add_column('data_sources', sa.Column('last_sync_at', sa.DateTime(timezone=True), nullable=True)) - op.add_column('data_sources', sa.Column('last_sync_status', sa.String(20), nullable=True)) - op.add_column('data_sources', sa.Column('health_status', sa.String(20), nullable=True)) - op.add_column('data_sources', sa.Column('schema_hash', sa.String(64), nullable=True)) - - # documents - op.add_column('documents', sa.Column('display_name', sa.String(500), nullable=True)) - op.add_column('documents', sa.Column('department_id', sa.Uuid(), nullable=True)) - op.add_column('documents', sa.Column('redaction_status', sa.String(20), server_default='none', nullable=False)) - op.add_column('documents', sa.Column('derivative_path', sa.String(1000), nullable=True)) - op.add_column('documents', sa.Column('original_locked', sa.Boolean(), server_default='false', nullable=False)) + # data_sources (shared — guarded) + idempotent_add_column('data_sources', sa.Column('discovered_source_id', sa.Uuid(), nullable=True)) + idempotent_add_column('data_sources', sa.Column('connector_template_id', sa.Integer(), nullable=True)) + idempotent_add_column('data_sources', sa.Column('sync_schedule', sa.String(50), nullable=True)) + idempotent_add_column('data_sources', sa.Column('last_sync_at', sa.DateTime(timezone=True), nullable=True)) + idempotent_add_column('data_sources', sa.Column('last_sync_status', sa.String(20), nullable=True)) + idempotent_add_column('data_sources', sa.Column('health_status', sa.String(20), nullable=True)) + idempotent_add_column('data_sources', sa.Column('schema_hash', sa.String(64), nullable=True)) + + # documents (shared — guarded) + idempotent_add_column('documents', sa.Column('display_name', sa.String(500), nullable=True)) + idempotent_add_column('documents', sa.Column('department_id', sa.Uuid(), nullable=True)) + idempotent_add_column('documents', sa.Column('redaction_status', sa.String(20), server_default='none', nullable=False)) + idempotent_add_column('documents', sa.Column('derivative_path', sa.String(1000), nullable=True)) + idempotent_add_column('documents', sa.Column('original_locked', sa.Boolean(), server_default='false', nullable=False)) # records_requests op.add_column('records_requests', sa.Column('requester_phone', sa.String(50), nullable=True)) @@ -244,14 +250,14 @@ def upgrade() -> None: op.add_column('exemption_flags', sa.Column('detection_method', sa.String(50), nullable=True)) op.add_column('exemption_flags', sa.Column('auto_detected', sa.Boolean(), server_default='false', nullable=False)) - # model_registry - op.add_column('model_registry', sa.Column('context_window_size', sa.Integer(), nullable=True)) - op.add_column('model_registry', sa.Column('supports_ner', sa.Boolean(), server_default='false', nullable=False)) - op.add_column('model_registry', sa.Column('supports_vision', sa.Boolean(), server_default='false', nullable=False)) + # model_registry (shared — guarded) + idempotent_add_column('model_registry', sa.Column('context_window_size', sa.Integer(), nullable=True)) + idempotent_add_column('model_registry', sa.Column('supports_ner', sa.Boolean(), server_default='false', nullable=False)) + idempotent_add_column('model_registry', sa.Column('supports_vision', sa.Boolean(), server_default='false', nullable=False)) - # users — add department_id - op.add_column('users', sa.Column('department_id', sa.Uuid(), nullable=True)) - op.create_foreign_key('fk_users_department', 'users', 'departments', ['department_id'], ['id']) + # users — add department_id (shared — guarded) + idempotent_add_column('users', sa.Column('department_id', sa.Uuid(), nullable=True)) + idempotent_create_foreign_key('fk_users_department', 'users', 'departments', ['department_id'], ['id']) def downgrade() -> None: diff --git a/backend/pyproject.toml b/backend/pyproject.toml index b3627e5..f392d10 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -17,6 +17,7 @@ dependencies = [ "python-jose[cryptography]>=3.5.0", "passlib[bcrypt]>=1.7.4", "celery>=5.6.0", + "civiccore @ git+https://github.com/CivicSuite/civiccore.git@e7c5570", "croniter>=2.0.0", "redis>=5.0.0,<8.0.0", "httpx>=0.28.0", @@ -46,6 +47,7 @@ asyncio_default_fixture_loop_scope = "session" testpaths = ["tests"] markers = [ "portal_mode(mode): T5D — set PORTAL_MODE for this test ('public' or 'private'). See tests/test_portal_mode.py::_portal_mode_override.", + "integration: Phase 1 — integration test that requires an external runtime (Docker Compose, Alembic, civiccore migration runner). See tests/test_civiccore_migration_gates.py.", ] [tool.setuptools.packages.find] diff --git a/backend/tests/test_civiccore_migration_gates.py b/backend/tests/test_civiccore_migration_gates.py new file mode 100644 index 0000000..4b373e0 --- /dev/null +++ b/backend/tests/test_civiccore_migration_gates.py @@ -0,0 +1,438 @@ +"""ADR-0003 §5 — Three migration gates for the civiccore Alembic baseline. + +These tests gate Phase 1 PR merge. They verify the three deployment scenarios +described in ADR-0003 §4: + +* **Gate 1 (fresh-install)** — empty Postgres → records env.py with civiccore + wiring → all 16 shared tables + 15 records-only tables present, both + ``alembic_version`` heads stamped at the expected revisions. +* **Gate 2 (upgrade-from-v1.2)** — Postgres seeded with records HEAD 019 but + no civiccore version table → ``alembic upgrade head`` → records head + unchanged, civiccore baseline stamped, schema unchanged. +* **Gate 3 (reapplication idempotent)** — fully migrated DB → ``alembic + upgrade head`` again → no "running upgrade" lines, no errors, both heads + unchanged. + +Fixture strategy +---------------- +Each test gets its own unique-named ephemeral Postgres database created on the +project's running ``postgres`` container. We invoke records' Alembic via +``subprocess.run([sys.executable, "-m", "alembic", "upgrade", "head"], ...)`` +with ``DATABASE_URL`` overridden to point at that ephemeral DB. This mirrors +the pattern already used by ``conftest.setup_db`` and avoids +docker-in-docker complexity. + +For **Gate 2** we seed the v1.2.x state by stamping ``alembic_version`` to +``019_encrypt_connection_config`` on a freshly-created (but otherwise empty) +DB **without** the civiccore wiring active. This is semantically equivalent +to "ran records v1.2.x migrations to head" for the purpose of the gate +assertion (the gate proves: civiccore baseline runs against a DB whose +records head is already 019, no-ops the table creates, and stamps its own +version table). We intentionally do *not* run the records chain a second +time before the test action; doing so would require either a separate +records-only env.py or a pre-civiccore tag of the records source — both +materially more complex than a stamp, with no additional coverage. If the +guard pass in records 001–019 is incorrect, Gate 1 catches it. + +These tests are marked ``@pytest.mark.integration`` per ADR-0003 §5 and the +records CLAUDE.md "Integration Tests" section. They require the running +``postgres`` container plus ``civiccore`` installed in the ``api`` image +(after Subagent B's ``pyproject.toml`` change). +""" + +from __future__ import annotations + +import os +import subprocess +import sys +import uuid +from collections.abc import Iterator +from pathlib import Path +from typing import Final + +import pytest +import sqlalchemy as sa +from sqlalchemy import create_engine +from sqlalchemy.engine import Engine + +# --------------------------------------------------------------------------- +# Constants — hard-coded so assertions are unambiguous and any drift between +# this test file and the live schema is loud and obvious. See ADR-0003 +# "Context — Shared vs records-owned schema" tables. +# --------------------------------------------------------------------------- + +SHARED_TABLES: Final[frozenset[str]] = frozenset({ + # 001 + "users", "service_accounts", "audit_log", + # 002 + "data_sources", "documents", "document_chunks", + # 003 + "model_registry", + # 006 (only this one of the three from 006) + "exemption_rules", + # 787207afc66a (6 of 12) + "connector_templates", "departments", "system_catalog", + "city_profile", "notification_templates", "prompt_templates", + # 016 + "sync_run_log", "sync_failures", +}) # 16 total — see ADR-0003 §Context shared list + +RECORDS_TABLES: Final[frozenset[str]] = frozenset({ + # 004 + "search_sessions", "search_queries", "search_results", + # 005 + "records_requests", "request_documents", "document_cache", + # 006 (records-only portion) + "exemption_flags", "disclosure_templates", + # 009 + "fee_waivers", + # 787207afc66a (6 of 12) + "fee_schedules", "fee_line_items", "notification_log", + "request_messages", "request_timeline", "response_letters", +}) # 15 total — see ADR-0003 §Context records list + +CIVICCORE_BASELINE_REV: Final[str] = "civiccore_0001_baseline_v1" +RECORDS_HEAD_REV: Final[str] = "019_encrypt_connection_config" + +BACKEND_DIR: Final[Path] = Path(__file__).resolve().parent.parent + + +# --------------------------------------------------------------------------- +# Helpers — ephemeral test-DB creation on the existing postgres container. +# Mirrors conftest.setup_db's pattern (DROP DATABASE WITH (FORCE), CREATE +# EXTENSION vector, subprocess alembic). Uses sync psycopg2 to avoid event-loop +# entanglement with pytest-asyncio. +# --------------------------------------------------------------------------- + + +def _admin_sync_url() -> str: + """Return a sync psycopg2 URL pointed at the cluster admin DB ('postgres').""" + from app.config import settings + base = settings.database_url.rsplit("/", 1)[0] + sync_base = base.replace("postgresql+asyncpg", "postgresql+psycopg2") + return f"{sync_base}/postgres" + + +def _ephemeral_db_url(db_name: str) -> str: + """Return the asyncpg-style URL records' Alembic env.py will consume.""" + from app.config import settings + base = settings.database_url.rsplit("/", 1)[0] + return f"{base}/{db_name}" + + +def _ephemeral_db_sync_url(db_name: str) -> str: + """Return the psycopg2-style URL for direct schema introspection.""" + return _ephemeral_db_url(db_name).replace("postgresql+asyncpg", "postgresql+psycopg2") + + +def _create_test_db(name: str) -> None: + """CREATE DATABASE on the cluster + install pgvector.""" + admin = create_engine(_admin_sync_url(), echo=False) + try: + with admin.connect().execution_options(isolation_level="AUTOCOMMIT") as conn: + conn.execute(sa.text(f'DROP DATABASE IF EXISTS "{name}" WITH (FORCE)')) + conn.execute(sa.text(f'CREATE DATABASE "{name}"')) + finally: + admin.dispose() + + db = create_engine(_ephemeral_db_sync_url(name), echo=False) + try: + with db.connect().execution_options(isolation_level="AUTOCOMMIT") as conn: + conn.execute(sa.text("CREATE EXTENSION IF NOT EXISTS vector")) + finally: + db.dispose() + + +def _drop_test_db(name: str) -> None: + admin = create_engine(_admin_sync_url(), echo=False) + try: + with admin.connect().execution_options(isolation_level="AUTOCOMMIT") as conn: + conn.execute(sa.text(f'DROP DATABASE IF EXISTS "{name}" WITH (FORCE)')) + finally: + admin.dispose() + + +def _run_alembic_upgrade_head(database_url: str) -> subprocess.CompletedProcess[str]: + """Invoke records' Alembic against ``database_url`` via the programmatic API. + + Implementation note: alembic 1.18 changed its CLI config-file detection so + that the default ``Config()`` no longer auto-discovers ``alembic.ini`` from + cwd (``config_file_name`` returns ``None``), and even ``alembic -c + alembic.ini upgrade head`` fails with "No 'script_location' key found in + configuration." Records does not use the alembic CLI in production — its + application startup runs ``command.upgrade(Config(), "head")`` + directly — so this helper mirrors that pattern. ADR-0003 §5 cares about the + behavior of records' env.py wiring, not the CLI surface. + + The CompletedProcess return shape is preserved so callers can keep their + ``returncode``/``stdout``/``stderr`` assertions unchanged. + """ + import contextlib + import io + + from alembic import command + from alembic.config import Config + + cfg = Config(str(BACKEND_DIR / "alembic.ini")) + + stdout_buf = io.StringIO() + stderr_buf = io.StringIO() + old_db_url = os.environ.get("DATABASE_URL") + os.environ["DATABASE_URL"] = database_url + + # Records' env.py captures ``settings`` at module-load via + # ``from app.config import settings``, so reassigning ``app.config.settings`` + # would not update the env.py module's local reference. Instead mutate the + # original Settings instance in place — every holder sees the new value. + # Pydantic v1 BaseSettings permits attribute assignment; Pydantic v2's default + # config is also non-frozen. + # + # Test-harness state only — production sets DATABASE_URL before the Alembic + # process starts, so the singleton binds to the intended DB on first import + # and never needs mutation. + import app.config as _app_config + + saved_database_url = _app_config.settings.database_url + _app_config.settings.database_url = database_url + + try: + with contextlib.redirect_stdout(stdout_buf), contextlib.redirect_stderr(stderr_buf): + try: + command.upgrade(cfg, "head") + rc = 0 + except SystemExit as exc: + rc = int(exc.code) if isinstance(exc.code, int) else 1 + except Exception as exc: # noqa: BLE001 — preserve details for the assertion message + stderr_buf.write(f"\nException: {type(exc).__name__}: {exc}") + rc = 1 + finally: + _app_config.settings.database_url = saved_database_url + if old_db_url is None: + os.environ.pop("DATABASE_URL", None) + else: + os.environ["DATABASE_URL"] = old_db_url + + return subprocess.CompletedProcess( + args=["python", "-m", "alembic", "upgrade", "head"], + returncode=rc, + stdout=stdout_buf.getvalue(), + stderr=stderr_buf.getvalue(), + ) + + +def _table_names(engine: Engine) -> set[str]: + inspector = sa.inspect(engine) + return set(inspector.get_table_names(schema="public")) + + +def _alembic_version(engine: Engine, table: str) -> str | None: + """Return the single ``version_num`` from an alembic version table, or None.""" + with engine.connect() as conn: + if not sa.inspect(conn).has_table(table): + return None + row = conn.execute(sa.text(f"SELECT version_num FROM {table}")).fetchone() + return row[0] if row else None + + +def _column_set(engine: Engine, table: str) -> set[str]: + inspector = sa.inspect(engine) + if table not in inspector.get_table_names(schema="public"): + return set() + return {c["name"] for c in inspector.get_columns(table, schema="public")} + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def fresh_db() -> Iterator[str]: + """Empty Postgres database. Yields the asyncpg-style DATABASE_URL.""" + name = f"gate1_{uuid.uuid4().hex[:12]}" + _create_test_db(name) + try: + yield _ephemeral_db_url(name) + finally: + _drop_test_db(name) + + +@pytest.fixture +def v1_2_seeded_db() -> Iterator[str]: + """Database stamped at records HEAD 019 with NO civiccore version table. + + Implementation choice (documented in module docstring): we *stamp* + ``alembic_version`` rather than running the v1.2.x records chain. This + is sufficient for the Gate 2 assertion (civiccore baseline must no-op + its table creates and stamp its own version table when records head is + already 019). A pre-existing schema dump would be more thorough but + materially more complex; Gate 1 already proves the create path. + """ + name = f"gate2_{uuid.uuid4().hex[:12]}" + _create_test_db(name) + try: + sync = create_engine(_ephemeral_db_sync_url(name), echo=False) + try: + with sync.connect() as conn: + conn.execute(sa.text( + "CREATE TABLE alembic_version (" + "version_num VARCHAR(32) NOT NULL, " + "CONSTRAINT alembic_version_pkc PRIMARY KEY (version_num))" + )) + conn.execute( + sa.text("INSERT INTO alembic_version (version_num) VALUES (:v)"), + {"v": RECORDS_HEAD_REV}, + ) + conn.commit() + finally: + sync.dispose() + yield _ephemeral_db_url(name) + finally: + _drop_test_db(name) + + +# --------------------------------------------------------------------------- +# Gate tests +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +def test_gate1_fresh_install(fresh_db: str) -> None: + """ADR-0003 §5 Gate 1 — fresh install creates all shared + records tables. + + Empty DB → ``alembic upgrade head`` (records env.py invokes civiccore + runner first) → all 31 expected tables present, both heads stamped. + """ + result = _run_alembic_upgrade_head(fresh_db) + assert result.returncode == 0, ( + f"alembic upgrade head failed (rc={result.returncode}):\n" + f"--- stdout ---\n{result.stdout}\n" + f"--- stderr ---\n{result.stderr}" + ) + + sync = create_engine(_ephemeral_db_sync_url(fresh_db.rsplit("/", 1)[1]), echo=False) + try: + tables = _table_names(sync) + + missing_shared = SHARED_TABLES - tables + assert not missing_shared, ( + f"Gate 1: missing shared tables {sorted(missing_shared)}; " + f"present={sorted(tables)}" + ) + + missing_records = RECORDS_TABLES - tables + assert not missing_records, ( + f"Gate 1: missing records-only tables {sorted(missing_records)}; " + f"present={sorted(tables)}" + ) + + records_head = _alembic_version(sync, "alembic_version") + assert records_head == RECORDS_HEAD_REV, ( + f"Gate 1: alembic_version expected {RECORDS_HEAD_REV!r}, got {records_head!r}" + ) + + civiccore_head = _alembic_version(sync, "alembic_version_civiccore") + assert civiccore_head == CIVICCORE_BASELINE_REV, ( + f"Gate 1: alembic_version_civiccore expected {CIVICCORE_BASELINE_REV!r}, " + f"got {civiccore_head!r}" + ) + finally: + sync.dispose() + + +@pytest.mark.integration +def test_gate2_upgrade_from_v1_2(v1_2_seeded_db: str) -> None: + """ADR-0003 §5 Gate 2 — v1.2.x → v1.3.0 upgrade is no-op for shared tables. + + DB stamped at records HEAD 019, no civiccore version table → ``alembic + upgrade head`` → records head unchanged, civiccore baseline stamped, no + schema change for shared tables that would have existed in a real v1.2.x + deployment (here we just assert no errors and correct heads — a real + v1.2.x dump would also let us assert column-set equality, but per the + fixture-strategy note we accept that trade-off). + """ + sync = create_engine(_ephemeral_db_sync_url(v1_2_seeded_db.rsplit("/", 1)[1]), echo=False) + try: + # Pre-action invariants — confirm the fixture set up what we expect. + before_records_head = _alembic_version(sync, "alembic_version") + assert before_records_head == RECORDS_HEAD_REV + before_civiccore_head = _alembic_version(sync, "alembic_version_civiccore") + assert before_civiccore_head is None, ( + "Gate 2 fixture should not have stamped civiccore — it represents " + "a pre-Phase-1 v1.2.x DB where the civiccore version table does not yet exist." + ) + finally: + sync.dispose() + + result = _run_alembic_upgrade_head(v1_2_seeded_db) + assert result.returncode == 0, ( + f"alembic upgrade head failed (rc={result.returncode}):\n" + f"--- stdout ---\n{result.stdout}\n" + f"--- stderr ---\n{result.stderr}" + ) + + sync = create_engine(_ephemeral_db_sync_url(v1_2_seeded_db.rsplit("/", 1)[1]), echo=False) + try: + records_head = _alembic_version(sync, "alembic_version") + assert records_head == RECORDS_HEAD_REV, ( + f"Gate 2: records head MUST NOT have advanced. " + f"Expected {RECORDS_HEAD_REV!r}, got {records_head!r}." + ) + + civiccore_head = _alembic_version(sync, "alembic_version_civiccore") + assert civiccore_head == CIVICCORE_BASELINE_REV, ( + f"Gate 2: civiccore baseline MUST be stamped after upgrade. " + f"Expected {CIVICCORE_BASELINE_REV!r}, got {civiccore_head!r}." + ) + finally: + sync.dispose() + + +@pytest.mark.integration +def test_gate3_reapplication_idempotent(fresh_db: str) -> None: + """ADR-0003 §5 Gate 3 — second ``alembic upgrade head`` is a complete no-op. + + Run ``alembic upgrade head`` once to reach the end-state of Gate 1, then + run it again. The second run must: + * Exit zero with no errors. + * Emit no "Running upgrade" lines (proves no migration body executed). + * Leave both ``alembic_version`` heads at their expected values. + """ + first = _run_alembic_upgrade_head(fresh_db) + assert first.returncode == 0, ( + f"Gate 3 setup (first upgrade) failed:\n" + f"--- stdout ---\n{first.stdout}\n" + f"--- stderr ---\n{first.stderr}" + ) + + second = _run_alembic_upgrade_head(fresh_db) + assert second.returncode == 0, ( + f"Gate 3: second upgrade must succeed, got rc={second.returncode}:\n" + f"--- stdout ---\n{second.stdout}\n" + f"--- stderr ---\n{second.stderr}" + ) + + combined = (second.stdout or "") + "\n" + (second.stderr or "") + # Alembic emits "Running upgrade -> " for each applied revision. + # Idempotent re-run must not print any such line. + assert "Running upgrade" not in combined, ( + "Gate 3: second upgrade must be a no-op — no 'Running upgrade' lines.\n" + f"--- stdout ---\n{second.stdout}\n" + f"--- stderr ---\n{second.stderr}" + ) + + sync = create_engine(_ephemeral_db_sync_url(fresh_db.rsplit("/", 1)[1]), echo=False) + try: + records_head = _alembic_version(sync, "alembic_version") + assert records_head == RECORDS_HEAD_REV, ( + f"Gate 3: records head changed across re-run. " + f"Expected {RECORDS_HEAD_REV!r}, got {records_head!r}." + ) + + civiccore_head = _alembic_version(sync, "alembic_version_civiccore") + assert civiccore_head == CIVICCORE_BASELINE_REV, ( + f"Gate 3: civiccore head changed across re-run. " + f"Expected {CIVICCORE_BASELINE_REV!r}, got {civiccore_head!r}." + ) + finally: + sync.dispose()