From 934d623d3abe12193aa64f7917f978f08922ff68 Mon Sep 17 00:00:00 2001
From: Tyler <tyler@crispygoat.com>
Date: Fri, 19 Jun 2026 21:25:15 -0600
Subject: [PATCH] =?UTF-8?q?feat(backend):=20harden=20db=5Fmigrate=20?=
 =?UTF-8?q?=E2=80=94=20rollback=20test,=20non-sql=20filter=20test,=20doc?=
 =?UTF-8?q?=20convention?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 backend/src/cyclone/db_migrate.py |  4 +++
 backend/tests/test_db_migrate.py  | 50 ++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/backend/src/cyclone/db_migrate.py b/backend/src/cyclone/db_migrate.py
index c209f4b..3e30e61 100644
--- a/backend/src/cyclone/db_migrate.py
+++ b/backend/src/cyclone/db_migrate.py
@@ -9,6 +9,10 @@ version bump so the next startup retries.
 
 This is intentionally NOT Alembic. One operator, one machine, one
 schema-version counter.
+
+Convention: migrations are DDL-only. Do not use `--` comments except for the
+required `-- version: N` header. Do not use string literals containing `;`.
+This is enforced by the line-based comment stripper and naive `;` splitter.
 """
 
 from __future__ import annotations
diff --git a/backend/tests/test_db_migrate.py b/backend/tests/test_db_migrate.py
index 8a9f6a8..d99b995 100644
--- a/backend/tests/test_db_migrate.py
+++ b/backend/tests/test_db_migrate.py
@@ -6,6 +6,7 @@ from pathlib import Path
 
 import pytest
 import sqlalchemy as sa
+import sqlalchemy.exc
 
 from cyclone import db_migrate
 
@@ -64,4 +65,51 @@ def test_run_raises_on_missing_version_header(tmp_path: Path, monkeypatch: pytes
 
     engine = _fresh_engine(tmp_path)
     with pytest.raises(RuntimeError, match="missing '-- version"):
-        db_migrate.run(engine)
\ No newline at end of file
+        db_migrate.run(engine)
+
+
+def test_run_rolls_back_version_on_failed_statement(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """If a DDL statement fails, the version bump must also roll back."""
+    monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path)
+    (tmp_path / "0001_initial.sql").write_text(
+        "-- version: 1\nCREATE TABLE t (id INTEGER);\n"
+    )
+
+    engine = _fresh_engine(tmp_path)
+    db_migrate.run(engine)  # applies 0001, user_version=1
+    assert _user_version(engine) == 1
+
+    # Now drop in a second migration whose DDL conflicts with the first.
+    # The runner should apply 0001 and 0002 in one call; 0002 fails, the
+    # transaction rolls back, and user_version must stay at 1 (not bump to 2).
+    (tmp_path / "0002_bad.sql").write_text(
+        "-- version: 2\nCREATE TABLE t (id INTEGER);\n"  # table already exists!
+    )
+
+    with pytest.raises(sqlalchemy.exc.SQLAlchemyError):
+        db_migrate.run(engine)
+
+    assert _user_version(engine) == 1  # bumped atomically, rolled back on failure
+
+
+def test_run_ignores_non_sql_files(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """README.md and similar files in MIGRATIONS_DIR must not be executed."""
+    monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path)
+    (tmp_path / "0001_initial.sql").write_text(
+        "-- version: 1\nCREATE TABLE t (id INTEGER);\n"
+    )
+    (tmp_path / "README.md").write_text("Don't run me!\nCREATE TABLE should_not_exist (x INT);\n")
+
+    engine = _fresh_engine(tmp_path)
+    db_migrate.run(engine)
+
+    with engine.connect() as c:
+        rows = c.exec_driver_sql(
+            "SELECT name FROM sqlite_master WHERE type='table' AND name='should_not_exist'"
+        ).all()
+    assert len(rows) == 0
+    assert _user_version(engine) == 1
\ No newline at end of file