.forgejo/workflows/ci.yml

@@ -184,7 +184,80 @@ jobs:
         run: uv pip install --python /opt/venv/bin/python -e ".[dev]"
 
       - name: Run tests
-        run: /opt/venv/bin/python -m pytest tests/ -q
+        # DB-free by design. Anything needing a real Postgres is marked `integration`
+        # and runs in the job below.
+        run: /opt/venv/bin/python -m pytest tests/ -q -m "not integration"
+
+  # Real-Postgres lane (family rule 6). Until this existed, `alembic upgrade head` ran
+  # for the first time when the operator's container started — 26 revisions, none of
+  # them ever executed by CI — and the schema the migrations build had never been
+  # checked against the models that read it.
+  #
+  # Runs for visibility and does NOT gate the build, matching the `test` lane and
+  # FabledScribe's equivalent job.
+  #
+  # Job key stays separator-free ("integration") with no `name:` — rule 80. act_runner
+  # derives the service-container name from the truncated job display name, and the
+  # discovery step below filters `docker ps` by it. Service hostnames are not routable
+  # on this runner (rule 79), so the step resolves the container's bridge IP.
+  integration:
+    if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v')
+    runs-on: python-ci
+    container:
+      image: git.fabledsword.com/bvandeusen/ci-python:3.14
+    services:
+      postgres:
+        # Same image the production compose runs, so the schema is proven against the
+        # Postgres it will actually meet.
+        image: postgres:16-alpine
+        env:
+          POSTGRES_USER: thoughtsync
+          POSTGRES_PASSWORD: ci_integration
+          POSTGRES_DB: thoughtsync_test
+        options: >-
+          --health-cmd "pg_isready -U thoughtsync"
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 10
+    steps:
+      - uses: actions/checkout@v6
+
+      - name: Create virtual environment
+        run: uv venv /opt/venv
+
+      # Same install as the unit lane — the two must agree on versions, or
+      # "unit green, integration red" stops being a signal about the code.
+      - name: Install package with dev deps
+        run: uv pip install --python /opt/venv/bin/python -e ".[dev]"
+
+      - name: Integration suite (resolve service IP, migrate, test)
+        run: |
+          set -eux
+          echo "=== container landscape (diagnostic for the name filter) ==="
+          docker ps -a --format '{{.ID}} {{.Image}} -> {{.Names}}'
+          PG=$(docker ps --filter "name=integration" --filter "ancestor=postgres:16-alpine" -q | head -n1)
+          test -n "$PG"
+          PG_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$PG")
+          test -n "$PG_IP"
+          export THOUGHTSYNC_DATABASE_URL="postgresql+asyncpg://thoughtsync:ci_integration@${PG_IP}:5432/thoughtsync_test"
+          # Wait for Postgres to accept connections. `run:` is busybox sh (rule 81) —
+          # no bash /dev/tcp — so use the Python that is always present here.
+          /opt/venv/bin/python - "$PG_IP" <<'PY'
+          import socket, sys, time
+          for _ in range(30):
+              try:
+                  socket.create_connection((sys.argv[1], 5432), timeout=2).close()
+                  break
+              except OSError:
+                  time.sleep(1)
+          else:
+              sys.exit("postgres did not become reachable")
+          PY
+          # Real migrations build the schema, never metadata.create_all (rule 82) —
+          # testing a schema no deployment has ever seen would prove nothing. This
+          # step IS the migration test: a broken revision fails the job here.
+          /opt/venv/bin/alembic upgrade head
+          /opt/venv/bin/python -m pytest tests/ -v -m integration
 
   build:
     name: Build & push image

ci-requirements.md

@@ -73,6 +73,43 @@ entirely on `ci-python:3.14`.
   `…/actions/artifacts/{id}/zip`. Note the workstation has no `unzip` — use
   `python3 -m zipfile -e`.
 
+## The integration lane
+
+Added 2026-08-23. Before it, `alembic upgrade head` ran for the first time when the
+operator's container started — 26 revisions, none of them ever executed by CI — and
+the schema the migrations build had never been checked against the models that read
+it. M13 dropped three columns and rebuilt a STORED GENERATED column with nothing
+watching.
+
+Copied from FabledScribe's `integration` job, which had already solved the awkward
+parts. Three of them are family rules for a reason:
+
+- **Job key `integration`, no `name:`** (rule 80). act_runner derives the service
+  container's name from the truncated job DISPLAY name, and the discovery step filters
+  `docker ps` by it. A spaced or underscored name breaks the filter.
+- **Service hostnames are not routable** on this runner (rule 79), so the step resolves
+  the Postgres container's bridge IP with `docker ps --filter` + `docker inspect` and
+  builds `THOUGHTSYNC_DATABASE_URL` from it. `postgres:5432` will not connect.
+- **`run:` is busybox sh** (rule 81) — no `/dev/tcp` — so the readiness wait is a small
+  Python heredoc. Its terminator must dedent to column 0 after YAML strips the block
+  indent; check with `yaml.safe_load` and print the `run` string if you edit it.
+
+`postgres:16-alpine`, matching the production compose, so the schema is proven against
+the Postgres it will actually meet. The schema comes from **real migrations, never
+`metadata.create_all`** (rule 82): testing a schema no deployment has ever seen proves
+nothing, and that `alembic upgrade head` step IS the migration test — a broken revision
+fails the job there, before it can fail a container start.
+
+Tests are marked `integration` (registered in `pyproject.toml`); the unit lane runs
+`-m "not integration"` and stays DB-free. Data resets with `TRUNCATE ... CASCADE`
+BEFORE each test rather than after, so a failure leaves its rows behind to look at.
+
+Like `test`, it runs for visibility and does **not** gate the build.
+
+There is no local way to run it — that would mean standing up Postgres on the
+workstation, which rule 12 reserves for an explicit request. This lane is verified in
+CI.
+
 ## Desktop (Tauri) lane — separate workflow
 
 The Tauri desktop client (`desktop/`) builds in its own workflow,

pyproject.toml

@@ -29,6 +29,12 @@ where = ["src"]
 [tool.pytest.ini_options]
 asyncio_mode = "auto"
 testpaths = ["tests"]
+# The unit lane runs `-m "not integration"`; the integration lane runs `-m integration`
+# against a real Postgres. Registered here so an unmarked typo fails loudly instead of
+# quietly landing a test in neither lane.
+markers = [
+    "integration: needs a live Postgres — CI's integration job, not the unit lane",
+]
 
 [tool.ruff]
 line-length = 120

tests/test_integration.py

@@ -0,0 +1,203 @@
+"""The real-Postgres lane (family rule 6).
+
+Everything else in this suite is deliberately DB-free, which means the schema the
+migrations build has never been checked against the models that read it. That gap is
+what this file closes, and it is not theoretical: M13 dropped three columns and
+rebuilt a generated column, and until now `alembic upgrade head` ran for the first
+time when the operator's container started.
+
+Marked `integration` and excluded from the unit lane by `-m "not integration"`, so a
+workstation without Postgres runs the rest of the suite unchanged.
+
+The schema comes from real migrations, never `metadata.create_all` (rule 82) — the
+point is to test what actually ships, and `create_all` would build a schema no
+deployment has ever seen.
+"""
+from __future__ import annotations
+
+import uuid
+
+import pytest
+import pytest_asyncio
+from sqlalchemy import select, text
+
+from thoughtsync.db import dispose_engine, session_scope
+from thoughtsync.models.note import Note
+from thoughtsync.models.note_item import NoteItem
+from thoughtsync.models.user import User
+from thoughtsync.notes.helpers import derive_display_title
+from thoughtsync.sync import _apply_note_items
+
+pytestmark = pytest.mark.integration
+
+# Every table the tests touch, child-first so FKs never block the truncate.
+# RESTART IDENTITY + CASCADE keeps this honest if a table gains children later.
+_TABLES = "notes, note_items, note_revisions, note_labels, labels, users"
+
+
+@pytest_asyncio.fixture
+async def db():
+    """A session against the migrated database, wiped before each test.
+
+    Wiped BEFORE rather than after so a failed test leaves its rows behind to look at.
+    """
+    async with session_scope() as session:
+        await session.execute(text(f"TRUNCATE {_TABLES} RESTART IDENTITY CASCADE"))
+        await session.commit()
+        yield session
+    await dispose_engine()
+
+
+@pytest_asyncio.fixture
+async def owner(db):
+    """A user to hang notes off — `notes.owner_id` is a real foreign key."""
+    user = User(email=f"{uuid.uuid4().hex}@example.test", display_name="Integration")
+    db.add(user)
+    await db.commit()
+    await db.refresh(user)
+    return user
+
+
+async def test_the_migrated_schema_matches_the_models(db, owner):
+    """The check that has never run: insert through the ORM, read it back.
+
+    A column the models expect and the migrations never created — or the reverse —
+    fails right here, instead of when a container starts.
+    """
+    note = Note(owner_id=owner.id, body="a thought", display_title="a thought")
+    db.add(note)
+    await db.commit()
+    await db.refresh(note)
+
+    found = await db.scalar(select(Note).where(Note.id == note.id))
+    assert found is not None
+    assert found.body == "a thought"
+    assert found.display_title == "a thought"
+
+
+async def test_the_dropped_columns_are_actually_gone(db):
+    """M13 dropped three. If a migration silently no-opped, this is where it shows."""
+    cols = set(
+        (
+            await db.execute(
+                text("SELECT column_name FROM information_schema.columns WHERE table_name = 'notes'")
+            )
+        )
+        .scalars()
+        .all()
+    )
+    assert "title" not in cols, "notes.title should have gone in 0026"
+    assert "kind" not in cols, "notes.kind should have gone in 0025"
+    assert "display_title" in cols and "body" in cols
+
+    rev_cols = set(
+        (
+            await db.execute(
+                text("SELECT column_name FROM information_schema.columns WHERE table_name = 'note_revisions'")
+            )
+        )
+        .scalars()
+        .all()
+    )
+    assert "title" not in rev_cols, "note_revisions.title should have gone in 0026"
+
+    tables = set(
+        (await db.execute(text("SELECT table_name FROM information_schema.tables WHERE table_schema = 'public'")))
+        .scalars()
+        .all()
+    )
+    assert "note_links" not in tables, "note_links should have gone in 0024"
+
+
+async def test_the_search_vector_was_rebuilt_over_the_name(db, owner):
+    """0026 had to drop and recreate a STORED GENERATED column.
+
+    Postgres refuses to drop a column another generated column depends on, so getting
+    this wrong doesn't produce a subtly wrong ranking — it produces a migration that
+    won't run at all. Worth proving the replacement actually indexes something.
+    """
+    note = Note(owner_id=owner.id, body="ferry tickets\nbook before friday", display_title="ferry tickets")
+    db.add(note)
+    await db.commit()
+
+    hit = await db.scalar(
+        text(
+            "SELECT count(*) FROM notes "
+            "WHERE search_vector @@ websearch_to_tsquery('english', :q)"
+        ).bindparams(q="ferry")
+    )
+    assert hit == 1
+
+    # The NAME is weight A and the body weight B, which is what makes a name match
+    # rank above a body-only one. Both must be in the vector at all.
+    body_only = await db.scalar(
+        text(
+            "SELECT count(*) FROM notes "
+            "WHERE search_vector @@ websearch_to_tsquery('english', :q)"
+        ).bindparams(q="friday")
+    )
+    assert body_only == 1
+
+
+async def test_a_note_keeps_both_its_body_and_its_items(db, owner):
+    """The shape M13 step 2 made normal: a note HAS a checklist, it isn't one."""
+    note = Note(owner_id=owner.id, body="weekend shop", display_title="weekend shop")
+    db.add(note)
+    await db.flush()
+    db.add_all(
+        [
+            NoteItem(note_id=note.id, text="milk", position=0),
+            NoteItem(note_id=note.id, text="eggs", position=1),
+        ]
+    )
+    await db.commit()
+
+    items = (
+        await db.scalars(select(NoteItem).where(NoteItem.note_id == note.id).order_by(NoteItem.position))
+    ).all()
+    assert [i.text for i in items] == ["milk", "eggs"]
+    assert (await db.scalar(select(Note.body).where(Note.id == note.id))) == "weekend shop"
+
+
+async def test_sync_no_longer_deletes_items_from_a_note_with_a_body(db, owner):
+    """The data-loss path step 2 removed, pinned against a real database.
+
+    `_apply_note_items` used to delete every item when the note wasn't `kind = "list"`.
+    Nothing can produce that state any more, but this is the regression that would
+    have silently eaten a checklist, and it deserves a test that would catch its
+    return.
+    """
+    note = Note(owner_id=owner.id, body="packing", display_title="packing")
+    db.add(note)
+    await db.flush()
+    db.add(NoteItem(note_id=note.id, text="socks", position=0))
+    await db.commit()
+
+    # A change that says nothing about items must LEAVE them alone — absent means
+    # "not telling us", not "empty".
+    await _apply_note_items(db, note, {"body": "packing"})
+    await db.commit()
+    assert (await db.scalar(select(NoteItem.text).where(NoteItem.note_id == note.id))) == "socks"
+
+    # An explicit list replaces them.
+    await _apply_note_items(db, note, {"items": [{"text": "charger", "checked": True}]})
+    await db.commit()
+    rows = (await db.scalars(select(NoteItem).where(NoteItem.note_id == note.id))).all()
+    assert [(r.text, r.checked) for r in rows] == [("charger", True)]
+
+
+async def test_a_note_with_only_items_still_has_a_name(db, owner):
+    """The hole that made removing the title unsafe until step 2 closed it."""
+    note = Note(owner_id=owner.id, body="", display_title="")
+    db.add(note)
+    await db.flush()
+    db.add(NoteItem(note_id=note.id, text="milk", position=0))
+    await db.commit()
+
+    first = await db.scalar(
+        select(NoteItem.text).where(NoteItem.note_id == note.id).order_by(NoteItem.position).limit(1)
+    )
+    note.display_title = derive_display_title(note.body, first)
+    await db.commit()
+
+    assert (await db.scalar(select(Note.display_title).where(Note.id == note.id))) == "milk"