From 5d9b2c03fb6bdd28e4c233a35005c709cdd3760c Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sun, 13 Sep 2026 21:41:28 +0200 Subject: fix(hub): the migration chain runs, and a test says so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `add_email_verification` wrote PostgreSQL's `(now() at time zone 'utc')` as a literal server default, where every other migration in the chain uses `sa.func.now()` and lets the dialect render it. On SQLite that is `sqlite3.OperationalError: near "at": syntax error` — so `meshbay-hub migrate` could not reach head on the database the suite and the documented local-hub workflow both use. Which is how it survived: the only test that ran alembic at all stopped at `c3d4e5f6a7b8`, the revision immediately before it. The two newest migrations — email verification, and the mail quota committed two days ago — had been run by exactly one thing, a production deploy, and the newest by nothing at all. `test_migrations_reach_head.py` upgrades to head and compares what that built against `Base.metadata`, both directions: a column in the models and in no migration never reaches production, and one in the migrations and in no model is a rename abandoned halfway. Both tests fail on the unfixed migration with the error above; the drift half was checked by adding a model column on purpose and watching it be named. The schemas agree today. It still does not check PostgreSQL-only behaviour — a default, an index type or a constraint one dialect accepts and the other refuses. Running the chain somewhere beats running it nowhere, and is not the same as running it where it ships. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UMxEQadpzPkYLFf5CYKhpW --- CLAUDE.md | 14 +++- .../d4e5f6a7b8c9_add_email_verification.py | 8 +- .../tests/test_migrations_reach_head.py | 86 ++++++++++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 packages/meshbay-hub/tests/test_migrations_reach_head.py diff --git a/CLAUDE.md b/CLAUDE.md index 10792aa..528ee0e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -639,7 +639,19 @@ do. Read them before writing anything that touches the same mechanism. reaches the deployed hub. Symptom: one endpoint answering 500 with an HTML body while everything else works, and a `psycopg` `UndefinedColumn` in the journal. `deploy-hub.sh` runs `alembic upgrade head` before restarting the service; a schema - change that skips a migration file will still pass every test you have + change that skips a migration file used to pass every test there was. + **`test_migrations_reach_head.py` is the one that does not**: it upgrades to head + on SQLite and compares what that built against `Base.metadata`, in both + directions. It could not have existed before 2026-09-13, because + `add_email_verification` wrote PostgreSQL's `(now() at time zone 'utc')` as a + literal server default — a syntax error on SQLite — so the chain could not reach + head on the only database the suite has, and the single test that ran alembic + stopped at the revision before it. The two newest migrations had therefore been + run by exactly one thing: a production deploy. **A server default goes in + `sa.func.now()`**, which the dialect running it renders; a dialect's own SQL + written out by hand is a migration that only one database can apply. What this + still does not check is PostgreSQL-only behaviour — running the chain somewhere + beats running it nowhere and is not the same as running it where it ships - **Some paths only exist in a browser, and only one browser has them.** The download-to-disk story is three different mechanisms — File System Access diff --git a/packages/meshbay-hub/src/meshbay_hub/db/migrations/versions/d4e5f6a7b8c9_add_email_verification.py b/packages/meshbay-hub/src/meshbay_hub/db/migrations/versions/d4e5f6a7b8c9_add_email_verification.py index 20ba29d..6741eb8 100644 --- a/packages/meshbay-hub/src/meshbay_hub/db/migrations/versions/d4e5f6a7b8c9_add_email_verification.py +++ b/packages/meshbay-hub/src/meshbay_hub/db/migrations/versions/d4e5f6a7b8c9_add_email_verification.py @@ -34,8 +34,14 @@ def upgrade() -> None: sa.Column('purpose', sa.String(16), nullable=False), sa.Column('user_id', sa.String(36), sa.ForeignKey('users.id'), nullable=True), sa.Column('group_id', sa.String(36), sa.ForeignKey('groups.id'), nullable=True), + # `sa.func.now()`, as every other migration in this chain uses: the + # dialect running it renders it. Written out as PostgreSQL's + # `(now() at time zone 'utc')`, this was a syntax error on SQLite — so + # the chain could not reach head on the database the suite and the + # local-hub workflow both use, and the two newest migrations had been + # run by exactly one thing: a production deploy. sa.Column('created_at', sa.DateTime(timezone=True), - server_default=sa.text("(now() at time zone 'utc')")), + server_default=sa.func.now()), sa.Column('expires_at', sa.DateTime(timezone=True), nullable=False), sa.Column('verified_at', sa.DateTime(timezone=True), nullable=True), sa.Column('attempts', sa.Integer, server_default='0'), diff --git a/packages/meshbay-hub/tests/test_migrations_reach_head.py b/packages/meshbay-hub/tests/test_migrations_reach_head.py new file mode 100644 index 0000000..3631438 --- /dev/null +++ b/packages/meshbay-hub/tests/test_migrations_reach_head.py @@ -0,0 +1,86 @@ +""" +The migration chain runs, and it builds the schema the models expect. + +`create_all()` is not a migration: it creates missing *tables* and never a +missing *column*, so the schema the suite runs on is not the schema a deployed +hub has. The symptom is one endpoint answering 500 with an HTML body while +everything else works, and an `UndefinedColumn` in the journal. + +Two things went unnoticed because nothing ran the chain to the end. The only +test that ran alembic at all stopped at `c3d4e5f6a7b8`, the revision before +`add_email_verification` — and that one wrote PostgreSQL's +`(now() at time zone 'utc')` as a literal server default, which is a syntax +error on SQLite. So the chain could not reach head on the database the suite +and the local-hub workflow both use, and the two newest migrations had been run +by exactly one thing: a production deploy. + +Hence the two tests here, and they are cheap enough to stay cheap: **upgrade to +head**, and **compare what that built against `Base.metadata`**. A column added +to a model and to no migration fails the second one; a migration that cannot run +fails the first. + +What this does *not* check is PostgreSQL-only behaviour — a default, an index +type or a constraint SQLite accepts and PostgreSQL does not, or the reverse. +Running the chain somewhere is enormously better than running it nowhere, and it +is not the same thing as running it where it ships. +""" + +from pathlib import Path + +import pytest +from alembic import command +from alembic.config import Config +from sqlalchemy import create_engine, inspect + +from meshbay_hub.db.models import Base + +HUB = Path(__file__).resolve().parents[1] + + +@pytest.fixture +def migrated(tmp_path, monkeypatch): + """A database built by the migrations alone, at head.""" + db = tmp_path / "hub.db" + monkeypatch.setenv("MESHBAY_DATABASE_URL", f"sqlite+aiosqlite:///{db}") + command.upgrade(Config(str(HUB / "alembic.ini")), "head") + return db + + +def test_the_chain_reaches_head(migrated): + """It ran at all — which is the half that was false.""" + insp = inspect(create_engine(f"sqlite:///{migrated}")) + assert "alembic_version" in insp.get_table_names() + # A table from the newest revision, so "head" means head and not "as far as + # the last revision anybody happened to run". + assert "mail_quota" in insp.get_table_names() + + +def test_the_migrated_schema_is_the_one_the_models_expect(migrated): + """Every table and column, both directions. + + Both directions on purpose: a column in the models and in no migration + never reaches production, and a column in the migrations and in no model is + a write nothing performs and a rename somebody abandoned halfway. + """ + insp = inspect(create_engine(f"sqlite:///{migrated}")) + migrated_tables = { + name: {c["name"] for c in insp.get_columns(name)} + for name in insp.get_table_names() if name != "alembic_version" + } + + problems: list[str] = [] + for name, table in Base.metadata.tables.items(): + if name not in migrated_tables: + problems.append(f"{name}: in the models, no migration creates it") + continue + expected = {c.name for c in table.columns} + for missing in sorted(expected - migrated_tables[name]): + problems.append(f"{name}.{missing}: in the models, in no migration") + for extra in sorted(migrated_tables[name] - expected): + problems.append(f"{name}.{extra}: in the migrations, in no model") + for name in sorted(set(migrated_tables) - set(Base.metadata.tables)): + problems.append(f"{name}: a migration creates it, no model uses it") + + assert not problems, ( + "the deployed schema and the tested one have drifted:\n " + + "\n ".join(problems)) -- cgit v1.2.3