From a11209d1236a13fc676312ff77366527d0bb4b54 Mon Sep 17 00:00:00 2001 From: gti Date: Wed, 23 Sep 2026 00:30:51 -0300 Subject: [PATCH] fix: make the migration chain runnable from an empty database 0001 called Base.metadata.create_all(), building the schema from the current models, so on a fresh PostgreSQL 0002 collided on datasets.raw_sha256 and `alembic upgrade head` could never run from zero. 0001 is now an explicit, frozen baseline: the seven base tables exactly as the real history created them, before 0002/0006 extended them. Databases already past 0001 never re-run it. Proof: a fresh `upgrade head` produces a schema identical (pg_dump --schema-only) to the long-lived dev database. 0019 repairs a latent bug found by that comparison: 0006 used server_default="now()" as a string, which PostgreSQL froze at the instant the column was created, so every Agent and Job got the same created_at. It also adds three indexes the models declare but no migration created. central/tests/test_migration_baseline.py guards (via AST) against migrations importing the models or using string timestamp defaults. Co-Authored-By: Claude Opus 5.5 --- central/alembic/versions/0001_initial.py | 94 ++++++++++++++++++- .../versions/0019_fix_timestamp_defaults.py | 43 +++++++++ central/tests/test_migration_baseline.py | 64 +++++++++++++ docs/PILOT_DEPLOYMENT.md | 12 ++- 4 files changed, 207 insertions(+), 6 deletions(-) create mode 100644 central/alembic/versions/0019_fix_timestamp_defaults.py create mode 100644 central/tests/test_migration_baseline.py diff --git a/central/alembic/versions/0001_initial.py b/central/alembic/versions/0001_initial.py index 33a06ce..eb2263d 100644 --- a/central/alembic/versions/0001_initial.py +++ b/central/alembic/versions/0001_initial.py @@ -1,11 +1,99 @@ """initial hub persistence Revision ID: 0001 + +Frozen, explicit baseline. This revision used to run +``Base.metadata.create_all()``, which builds the schema from the *current* +models: on an empty database it created tables/columns that 0002..0018 then +tried to create again (first collision: ``datasets.raw_sha256`` in 0002), so +``alembic upgrade head`` could never run from zero. + +This body reproduces exactly what 0001 created on the databases that went +through the real history: the seven base tables, *before* 0002 added +``datasets.raw_sha256`` and 0006 added the agent status/timestamp columns. +Constraint and index names match the ones those databases already have +(PostgreSQL defaults for PK/FK/UNIQUE, ``ix__`` for indexes). + +Databases already at 0001 or later never re-run this revision, so changing +its body only affects new installations. It must never import the ORM +models again: a migration describes one point in history, not the present. """ from alembic import op -from central.app.models import Base +import sqlalchemy as sa + revision = "0001" down_revision = None branch_labels = depends_on = None -def upgrade(): Base.metadata.create_all(op.get_bind()) -def downgrade(): Base.metadata.drop_all(op.get_bind()) + + +def upgrade(): + op.create_table( + "tenants", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("name", sa.String(128), nullable=False, unique=True), + ) + op.create_table( + "datasets", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("kind", sa.String(32), nullable=False), + sa.Column("provider", sa.String(128), nullable=False), + sa.Column("sha256", sa.String(64), nullable=False), + sa.Column("version_label", sa.String(256), nullable=False), + sa.Column("act", sa.Text(), nullable=False), + sa.Column("source_url", sa.Text(), nullable=False), + sa.Column("downloaded_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + sa.Column("status", sa.String(32), nullable=False), + sa.Column("record_count", sa.Integer(), nullable=False), + sa.UniqueConstraint("provider", "sha256", name="uq_dataset_provider_hash"), + ) + op.create_index("ix_datasets_kind", "datasets", ["kind"]) + op.create_index("ix_datasets_status", "datasets", ["status"]) + op.create_table( + "ncm_entries", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("dataset_id", sa.Integer(), sa.ForeignKey("datasets.id"), nullable=False), + sa.Column("code", sa.String(16), nullable=False), + sa.Column("description", sa.Text(), nullable=False), + sa.Column("valid_from", sa.String(32), nullable=False), + sa.Column("valid_to", sa.String(32), nullable=False), + sa.UniqueConstraint("dataset_id", "code", "valid_from", "valid_to", name="uq_ncm_versioned_entry"), + ) + op.create_index("ix_ncm_entries_dataset_id", "ncm_entries", ["dataset_id"]) + op.create_index("ix_ncm_entries_code", "ncm_entries", ["code"]) + op.create_table( + "agents", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("tenant_id", sa.Integer(), sa.ForeignKey("tenants.id"), nullable=False), + sa.Column("name", sa.String(128), nullable=False), + sa.Column("token_hash", sa.String(64), nullable=False, unique=True), + ) + op.create_index("ix_agents_tenant_id", "agents", ["tenant_id"]) + op.create_table( + "classification_cache", + sa.Column("fingerprint", sa.String(64), primary_key=True), + sa.Column("tenant_id", sa.Integer(), sa.ForeignKey("tenants.id"), nullable=False), + sa.Column("result", sa.JSON(), nullable=False), + ) + op.create_index("ix_classification_cache_tenant_id", "classification_cache", ["tenant_id"]) + op.create_table( + "classification_requests", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("tenant_id", sa.Integer(), sa.ForeignKey("tenants.id"), nullable=False), + sa.Column("agent_id", sa.Integer(), sa.ForeignKey("agents.id"), nullable=False), + sa.Column("fingerprint", sa.String(64), nullable=False, unique=True), + sa.Column("payload", sa.JSON(), nullable=False), + sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + ) + op.create_index("ix_classification_requests_tenant_id", "classification_requests", ["tenant_id"]) + op.create_index("ix_classification_requests_agent_id", "classification_requests", ["agent_id"]) + op.create_table( + "classification_results", + sa.Column("id", sa.Integer(), primary_key=True), + sa.Column("request_id", sa.Integer(), sa.ForeignKey("classification_requests.id"), nullable=False, unique=True), + sa.Column("result", sa.JSON(), nullable=False), + ) + + +def downgrade(): + for table in ("classification_results", "classification_requests", "classification_cache", "agents", "ncm_entries", "datasets", "tenants"): + op.drop_table(table) diff --git a/central/alembic/versions/0019_fix_timestamp_defaults.py b/central/alembic/versions/0019_fix_timestamp_defaults.py new file mode 100644 index 0000000..be72187 --- /dev/null +++ b/central/alembic/versions/0019_fix_timestamp_defaults.py @@ -0,0 +1,43 @@ +"""Fix frozen timestamp defaults from 0006 and add model indexes never migrated. + +0006 declared ``server_default="now()"`` as a plain string. SQLAlchemy renders +that as the literal ``'now()'``, which PostgreSQL casts to a timestamp once, +when the column is created — every later row got that same fixed instant +instead of the insert time (agents.created_at/updated_at, jobs.created_at). +The models rely on the server default, so Jobs and Agents registered after +0006 all carried the migration's timestamp. Existing rows are left as they +are: their real creation time is not recoverable. + +The models also declare ``index=True`` on three columns that no migration +ever indexed. + +Revision ID: 0019_fix_timestamp_defaults +Revises: 0018_blocker_installation +""" +from alembic import op +import sqlalchemy as sa + +revision = "0019_fix_timestamp_defaults" +down_revision = "0018_blocker_installation" +branch_labels = None +depends_on = None + +_TIMESTAMP_COLUMNS = (("agents", "created_at"), ("agents", "updated_at"), ("jobs", "created_at")) +_MISSING_INDEXES = ( + ("ix_kb_observations_tenant_id", "kb_observations", "tenant_id"), + ("ix_kb_observations_company_key", "kb_observations", "company_key"), + ("ix_tenant_product_context_company_key", "tenant_product_context", "company_key"), +) + + +def upgrade(): + for table, column in _TIMESTAMP_COLUMNS: + op.alter_column(table, column, server_default=sa.func.now(), existing_type=sa.DateTime(timezone=True), existing_nullable=False) + for name, table, column in _MISSING_INDEXES: + op.create_index(name, table, [column]) + + +def downgrade(): + # The frozen-timestamp defaults are a bug and are deliberately not restored. + for name, table, _column in reversed(_MISSING_INDEXES): + op.drop_index(name, table_name=table) diff --git a/central/tests/test_migration_baseline.py b/central/tests/test_migration_baseline.py new file mode 100644 index 0000000..f881732 --- /dev/null +++ b/central/tests/test_migration_baseline.py @@ -0,0 +1,64 @@ +"""Guards for the migration chain being runnable from an empty database. + +0001 used to build the schema from the *current* ORM models, so every later +migration collided with it on a fresh database. These checks run offline (no +PostgreSQL) and inspect code, not comments, so they fail if that pattern +comes back. +""" +import ast +import re +from pathlib import Path + +from alembic.config import Config +from alembic.script import ScriptDirectory + +ROOT = Path(__file__).parents[2] +VERSIONS = ROOT / "central" / "alembic" / "versions" + + +def _migrations(): + return {path.name: ast.parse(path.read_text(encoding="utf-8")) for path in sorted(VERSIONS.glob("*.py"))} + + +def _script_directory(): + config = Config(str(ROOT / "central" / "alembic.ini")) + config.set_main_option("script_location", str(ROOT / "central" / "alembic")) + return ScriptDirectory.from_config(config) + + +def test_no_migration_builds_schema_from_current_models(): + offenders = [] + for name, tree in _migrations().items(): + for node in ast.walk(tree): + if isinstance(node, ast.Attribute) and node.attr in ("create_all", "drop_all"): + offenders.append((name, node.attr)) + if isinstance(node, ast.ImportFrom) and (node.module or "").startswith("central.app"): + offenders.append((name, node.module)) + assert offenders == [] + + +def test_chain_is_linear_with_a_single_head(): + script = _script_directory() + assert len(script.get_heads()) == 1 + revisions = list(script.walk_revisions()) + assert revisions[-1].revision == "0001" + assert revisions[-1].down_revision is None + + +def test_timestamp_defaults_are_sql_functions_not_string_literals(): + # A plain string such as "now()" is rendered as a quoted literal that + # PostgreSQL freezes at column creation time. 0006 did that and 0019 + # repairs it; no other migration may repeat it. + offenders = [] + for name, tree in _migrations().items(): + if name.startswith("0006_"): + continue + for node in ast.walk(tree): + if ( + isinstance(node, ast.keyword) + and node.arg == "server_default" + and isinstance(node.value, ast.Constant) + and re.fullmatch(r"\s*(now\s*\(\s*\)|now|current_timestamp|localtimestamp)\s*", str(node.value.value), re.IGNORECASE) + ): + offenders.append(name) + assert offenders == [] diff --git a/docs/PILOT_DEPLOYMENT.md b/docs/PILOT_DEPLOYMENT.md index 26693a8..5b707d9 100644 --- a/docs/PILOT_DEPLOYMENT.md +++ b/docs/PILOT_DEPLOYMENT.md @@ -121,9 +121,15 @@ Agents with `central/scripts/register_agent.py` (see below). `docker exec pg_dump -U gti gti_fiscal > backup.sql` (or `pg_dumpall` for the whole cluster) on whatever schedule your operations policy requires. -- Migrations are additive-only so far (`0001`..`0006`); always run - `alembic upgrade head` (the container does this automatically) rather than - hand-editing the schema. +- Always run `alembic upgrade head` (the container does this automatically) + rather than hand-editing the schema. An **empty** database is initialized + by that same command — never with `alembic stamp` or `create_all`. `0001` is + an explicit, frozen baseline (it used to call `Base.metadata.create_all()`, + which made `0002` collide on `datasets.raw_sha256` on any fresh database); + `central/tests/test_migration_baseline.py` keeps that from coming back. +- `0019` repairs `agents.created_at/updated_at` and `jobs.created_at`, whose + defaults `0006` had frozen at the instant it ran (rows created before `0019` + keep that frozen value — their real creation time is not recoverable). ### HTTPS / reverse proxy