From 3d8486438264aa541b1ecd35688dc66b23328331 Mon Sep 17 00:00:00 2001
From: yukkop <hectic.yukkop@gmail.com>
Date: Tue, 23 Dec 2025 19:31:44 +0000
Subject: [PATCH] fix(`package`): `migrator`: sqlite transactions

---
 package/migrator/migrator.sh                  |  43 +---
 .../postgresql/migration-failure-rollback.sh  | 214 ++++++++++++++++
 .../test/sqlite/migration-failure-rollback.sh | 239 ++++++++++++++++++
 3 files changed, 459 insertions(+), 37 deletions(-)
 create mode 100644 test/package/migrator/test/postgresql/migration-failure-rollback.sh
 create mode 100644 test/package/migrator/test/sqlite/migration-failure-rollback.sh

diff --git a/package/migrator/migrator.sh b/package/migrator/migrator.sh
index 45d3071..a624550 100644
--- a/package/migrator/migrator.sh
+++ b/package/migrator/migrator.sh
@@ -100,7 +100,7 @@ db_query() {
       local db_path
       db_path=$(get_sqlite_path)
       # Use -noheader -list for clean output (one value per line, no formatting)
-      sqlite3 -noheader -list "$db_path" "$sql" | awk NF
+      sqlite3 -bail -noheader -list "$db_path" "$sql" | awk NF
       ;;
   esac
 }
@@ -126,7 +126,7 @@ SQL
     sqlite)
       local db_path
       db_path=$(get_sqlite_path)
-      sqlite3 -batch "$db_path" <<SQL
+      sqlite3 -bail -batch "$db_path" <<SQL
 BEGIN;
 .read $file_path
 COMMIT;
@@ -808,8 +808,8 @@ SQL
         sqlite)
           local db_path
           db_path=$(get_sqlite_path)
-          if ! sqlite3 -batch "$db_path" <<SQL
-BEGIN;
+          if ! sqlite3 -bail -batch "$db_path" <<SQL
+BEGIN TRANSACTION;
 .read $mig_path
 INSERT INTO hectic_migration (name, hash) VALUES ('$escaped_name', '$mig_hash');
 COMMIT;
@@ -864,8 +864,8 @@ SQL
         sqlite)
           local db_path
           db_path=$(get_sqlite_path)
-          if ! sqlite3 -batch "$db_path" <<SQL
-BEGIN;
+          if ! sqlite3 -bail -batch "$db_path" <<SQL
+BEGIN TRANSACTION;
 .read $mig_path
 DELETE FROM hectic_migration WHERE name = '$escaped_name';
 COMMIT;
@@ -888,37 +888,6 @@ SQL
   fi
 }
 
-migrate_inner() {
-  # depricated, rewrite
-  printf '%s\n' "$fs_migrations" | while IFS= read -r fs_migration; do
-    # skip already applied migrations
-    printf '%s' "$db_migrations" | grep -qxF "$fs_migration" && continue
-
-    psql_args="$(form_psql_args)"
-
-    direction=1
-    mig_direction=$([ "$direction" -gt 0 ] && printf 'up.sql' || printf 'down.sql')
-
-    escaped_name=$(printf '%s' "$fs_migration" | sed "s/'/''/g")
-    mig_path=$(printf '%s/%s/%s' "$MIGRATION_DIR" "$fs_migration" "$mig_direction")
-    escaped_path=$(printf '%s' "$mig_path" | sed "s/'/''/g")
-
-    log trace "mig name: $escaped_name; mig path: $escaped_path"
-
-    # shellcheck disable=SC2086
-    if ! psql $psql_args <<SQL
-BEGIN;
-\i '$escaped_path';
-INSERT INTO hectic.migration (name, hash) VALUES ('$escaped_name', '$(sha256sum "$mig_path")');
-COMMIT;
-SQL
-    then
-      log error "migration failed: ${WHITE}$fs_migration${NC}"
-      exit 4
-    fi
-  done
-}
-
 form_psql_args() {
   local psql_args="-v ON_ERROR_STOP=1"
   for var in ${VARIABLE_LIST:-}; do
diff --git a/test/package/migrator/test/postgresql/migration-failure-rollback.sh b/test/package/migrator/test/postgresql/migration-failure-rollback.sh
new file mode 100644
index 0000000..7d21020
--- /dev/null
+++ b/test/package/migrator/test/postgresql/migration-failure-rollback.sh
@@ -0,0 +1,214 @@
+#!/bin/dash
+
+HECTIC_NAMESPACE=test-migration-failure-rollback
+
+log notice "test case: ${WHITE}migration failure causes transaction rollback"
+
+# Create initial schema
+psql "$DATABASE_URL" -c 'CREATE TABLE products (id INTEGER PRIMARY KEY, name TEXT)'
+
+# Initialize migrator
+if ! migrator --db-url "$DATABASE_URL" init; then
+  log error "test failed: ${WHITE}init failed"
+  exit 1
+fi
+
+log info "setup complete"
+
+### CASE 1: Successful migration followed by failed migration
+log notice "test case: ${WHITE}failed migration doesn't create migration record"
+
+# Create migrations directory
+mkdir -p migration
+
+# Create first SUCCESSFUL migration
+mig1="20250101000001-add-price"
+mkdir -p "migration/${mig1}"
+cat > "migration/${mig1}/up.sql" <<SQL
+-- This should succeed
+ALTER TABLE products ADD COLUMN price DECIMAL(10,2);
+SQL
+echo "ALTER TABLE products DROP COLUMN price;" > "migration/${mig1}/down.sql"
+
+# Create second FAILING migration (syntax error)
+mig2="20250101000002-broken-migration"
+mkdir -p "migration/${mig2}"
+cat > "migration/${mig2}/up.sql" <<SQL
+-- This SQL is intentionally broken to cause failure
+ALTER TABLE products ADD COLUMN description TEXT;
+THISISNOTVALIDSQL;  -- <-- This will cause error
+ALTER TABLE products ADD COLUMN category TEXT;
+SQL
+echo "-- rollback" > "migration/${mig2}/down.sql"
+
+# Apply first migration (should succeed)
+if ! migrator --db-url "$DATABASE_URL" migrate up; then
+  log error "test failed: ${WHITE}first migration should succeed"
+  exit 1
+fi
+
+# Verify first migration was recorded
+count=$(psql -Atc "SELECT COUNT(*) FROM hectic.migration" "$DATABASE_URL")
+if [ "$count" != "1" ]; then
+  log error "test failed: ${WHITE}expected 1 migration, got $count"
+  exit 1
+fi
+
+log info "first migration successful and recorded"
+
+# Try to apply second migration (should fail)
+set +e
+migrator --db-url "$DATABASE_URL" migrate up 2>&1
+exit_code=$?
+set -e
+
+if [ "$exit_code" = "0" ]; then
+  log error "test failed: ${WHITE}broken migration should have failed"
+  exit 1
+fi
+
+log info "second migration failed as expected (exit code: $exit_code)"
+
+# CRITICAL CHECK: Verify the failed migration was NOT recorded
+count_after=$(psql -Atc "SELECT COUNT(*) FROM hectic.migration" "$DATABASE_URL")
+if [ "$count_after" != "1" ]; then
+  log error "test failed: ${WHITE}CRITICAL! Failed migration was recorded. Expected 1, got $count_after"
+  log error "This means the transaction was not rolled back properly!"
+  exit 1
+fi
+
+log info "✓ Failed migration was NOT recorded (transaction rolled back)"
+
+# Verify the description column was NOT created (transaction rollback)
+if psql -Atc "SELECT description FROM products LIMIT 0" "$DATABASE_URL" >/dev/null 2>&1; then
+  log error "test failed: ${WHITE}CRITICAL! 'description' column exists after failed migration"
+  log error "This means partial changes were committed!"
+  exit 1
+fi
+
+log info "✓ No partial changes committed"
+
+# Verify price column from first migration still exists
+if ! psql -Atc "SELECT price FROM products LIMIT 0" "$DATABASE_URL" >/dev/null 2>&1; then
+  log error "test failed: ${WHITE}first migration's changes were lost"
+  exit 1
+fi
+
+log info "✓ First migration's changes preserved"
+
+### CASE 2: Failed migration in the middle of a transaction
+log notice "test case: ${WHITE}multi-statement migration fails atomically"
+
+# Create migration with multiple statements, one fails in the middle
+mig3="20250101000003-multi-statement-fail"
+mkdir -p "migration/${mig3}"
+cat > "migration/${mig3}/up.sql" <<SQL
+-- First statement succeeds
+ALTER TABLE products ADD COLUMN stock INTEGER DEFAULT 0;
+
+-- Second statement succeeds
+INSERT INTO products (name, price, stock) VALUES ('Test Product', 10.00, 5);
+
+-- Third statement FAILS
+ALTER TABLE nonexistent_table ADD COLUMN foo TEXT;
+
+-- Fourth statement would succeed if we got here
+ALTER TABLE products ADD COLUMN discount DECIMAL(5,2);
+SQL
+echo "-- rollback" > "migration/${mig3}/down.sql"
+
+# Try to apply migration (should fail)
+set +e
+migrator --db-url "$DATABASE_URL" migrate up 2>&1
+exit_code=$?
+set -e
+
+if [ "$exit_code" = "0" ]; then
+  log error "test failed: ${WHITE}multi-statement broken migration should have failed"
+  exit 1
+fi
+
+log info "multi-statement migration failed as expected"
+
+# Verify migration was NOT recorded
+count_after_multi=$(psql -Atc "SELECT COUNT(*) FROM hectic.migration" "$DATABASE_URL")
+if [ "$count_after_multi" != "1" ]; then
+  log error "test failed: ${WHITE}failed multi-statement migration was recorded"
+  exit 1
+fi
+
+log info "✓ Failed migration was NOT recorded"
+
+# Verify NO partial changes were committed
+if psql -Atc "SELECT stock FROM products LIMIT 0" "$DATABASE_URL" >/dev/null 2>&1; then
+  log error "test failed: ${WHITE}CRITICAL! 'stock' column exists (partial commit in failed migration)"
+  exit 1
+fi
+
+log info "✓ No partial changes from multi-statement migration"
+
+# Verify no data was inserted
+row_count=$(psql -Atc "SELECT COUNT(*) FROM products" "$DATABASE_URL")
+if [ "$row_count" != "0" ]; then
+  log error "test failed: ${WHITE}data was inserted despite migration failure"
+  exit 1
+fi
+
+log info "✓ No data inserted from failed migration"
+
+### CASE 3: Migration with constraint violation
+log notice "test case: ${WHITE}constraint violation rolls back transaction"
+
+mig4="20250101000004-constraint-violation"
+mkdir -p "migration/${mig4}"
+cat > "migration/${mig4}/up.sql" <<SQL
+-- Add column
+ALTER TABLE products ADD COLUMN sku TEXT UNIQUE;
+
+-- Try to insert duplicate data (will violate UNIQUE constraint)
+INSERT INTO products (name, price, sku) VALUES ('Product A', 10.00, 'SKU001');
+INSERT INTO products (name, price, sku) VALUES ('Product B', 20.00, 'SKU001');  -- DUPLICATE!
+SQL
+echo "-- rollback" > "migration/${mig4}/down.sql"
+
+# Try to apply (should fail due to constraint violation)
+set +e
+migrator --db-url "$DATABASE_URL" migrate up 2>&1
+exit_code=$?
+set -e
+
+if [ "$exit_code" = "0" ]; then
+  log error "test failed: ${WHITE}constraint violation migration should have failed"
+  exit 1
+fi
+
+log info "constraint violation migration failed as expected"
+
+# Verify migration was NOT recorded
+final_count=$(psql -Atc "SELECT COUNT(*) FROM hectic.migration" "$DATABASE_URL")
+if [ "$final_count" != "1" ]; then
+  log error "test failed: ${WHITE}constraint violation migration was recorded"
+  exit 1
+fi
+
+log info "✓ Constraint violation migration was NOT recorded"
+
+# Verify sku column was NOT added (full rollback)
+if psql -Atc "SELECT sku FROM products LIMIT 0" "$DATABASE_URL" >/dev/null 2>&1; then
+  log error "test failed: ${WHITE}sku column exists after constraint violation"
+  exit 1
+fi
+
+log info "✓ All changes rolled back after constraint violation"
+
+# Verify no data was committed
+final_row_count=$(psql -Atc "SELECT COUNT(*) FROM products" "$DATABASE_URL")
+if [ "$final_row_count" != "0" ]; then
+  log error "test failed: ${WHITE}data exists after constraint violation rollback"
+  exit 1
+fi
+
+log info "✓ No data committed after constraint violation"
+
+log notice "test passed: all migration failures properly roll back transactions"
+
diff --git a/test/package/migrator/test/sqlite/migration-failure-rollback.sh b/test/package/migrator/test/sqlite/migration-failure-rollback.sh
new file mode 100644
index 0000000..117735c
--- /dev/null
+++ b/test/package/migrator/test/sqlite/migration-failure-rollback.sh
@@ -0,0 +1,239 @@
+#!/bin/dash
+
+HECTIC_NAMESPACE=test-sqlite-migration-failure-rollback
+
+log notice "test case: ${WHITE}SQLite migration failure causes transaction rollback"
+
+# Create SQLite database
+SQLITE_DB="$PWD/test.db"
+export DB_URL="sqlite://$SQLITE_DB"
+
+log info "using SQLite database: $SQLITE_DB"
+
+# Create initial schema
+sqlite3 "$SQLITE_DB" "CREATE TABLE items (id INTEGER PRIMARY KEY, name TEXT)"
+
+# Initialize migrator
+if ! migrator --db-url "$DB_URL" init; then
+  log error "test failed: ${WHITE}init failed"
+  exit 1
+fi
+
+log info "setup complete"
+
+### CASE 1: Failed migration doesn't create migration record
+log notice "test case: ${WHITE}failed SQLite migration doesn't create record"
+
+# Create migrations directory
+mkdir -p migration
+
+# Create first SUCCESSFUL migration
+mig1="20250101000001-add-quantity"
+mkdir -p "migration/${mig1}"
+cat > "migration/${mig1}/up.sql" <<SQL
+-- This should succeed
+ALTER TABLE items ADD COLUMN quantity INTEGER DEFAULT 0;
+SQL
+echo "-- Note: SQLite ALTER TABLE DROP COLUMN not in old versions" > "migration/${mig1}/down.sql"
+
+# Create second FAILING migration (syntax error)
+mig2="20250101000002-broken-migration"
+mkdir -p "migration/${mig2}"
+cat > "migration/${mig2}/up.sql" <<SQL
+-- This SQL is intentionally broken
+ALTER TABLE items ADD COLUMN status TEXT;
+THIS IS NOT VALID SQL;  -- <-- This will cause error
+ALTER TABLE items ADD COLUMN tag TEXT;
+SQL
+echo "-- rollback" > "migration/${mig2}/down.sql"
+
+# Apply first migration (should succeed)
+if ! migrator --db-url "$DB_URL" migrate up; then
+  log error "test failed: ${WHITE}first migration should succeed"
+  exit 1
+fi
+
+# Verify first migration was recorded
+count=$(sqlite3 "$SQLITE_DB" "SELECT COUNT(*) FROM hectic_migration")
+if [ "$count" != "1" ]; then
+  log error "test failed: ${WHITE}expected 1 migration, got $count"
+  exit 1
+fi
+
+log info "first migration successful and recorded"
+
+# Try to apply second migration (should fail)
+set +e
+migrator --db-url "$DB_URL" migrate up 2>&1
+exit_code=$?
+set -e
+
+if [ "$exit_code" = "0" ]; then
+  log error "test failed: ${WHITE}broken migration should have failed"
+  exit 1
+fi
+
+log info "second migration failed as expected (exit code: $exit_code)"
+
+# CRITICAL CHECK: Verify the failed migration was NOT recorded
+count_after=$(sqlite3 "$SQLITE_DB" "SELECT COUNT(*) FROM hectic_migration")
+if [ "$count_after" != "1" ]; then
+  log error "test failed: ${WHITE}CRITICAL! Failed migration was recorded. Expected 1, got $count_after"
+  log error "This means the transaction was not rolled back properly!"
+  exit 1
+fi
+
+log info "✓ Failed migration was NOT recorded (transaction rolled back)"
+
+# Verify the status column was NOT created (transaction rollback)
+set +e
+sqlite3 "$SQLITE_DB" "SELECT status FROM items LIMIT 0" >/dev/null 2>&1
+status_exists=$?
+set -e
+
+if [ "$status_exists" = "0" ]; then
+  log error "test failed: ${WHITE}CRITICAL! 'status' column exists after failed migration"
+  log error "This means partial changes were committed!"
+  exit 1
+fi
+
+log info "✓ No partial changes committed"
+
+# Verify quantity column from first migration still exists
+if ! sqlite3 "$SQLITE_DB" "SELECT quantity FROM items LIMIT 0" >/dev/null 2>&1; then
+  log error "test failed: ${WHITE}first migration's changes were lost"
+  exit 1
+fi
+
+log info "✓ First migration's changes preserved"
+
+### CASE 2: Multi-statement migration fails atomically
+log notice "test case: ${WHITE}multi-statement SQLite migration fails atomically"
+
+mig3="20250101000003-multi-statement-fail"
+mkdir -p "migration/${mig3}"
+cat > "migration/${mig3}/up.sql" <<SQL
+-- First statement succeeds
+ALTER TABLE items ADD COLUMN location TEXT;
+
+-- Second statement succeeds  
+CREATE TABLE temp_table (id INTEGER);
+
+-- Third statement FAILS
+ALTER TABLE nonexistent_table ADD COLUMN foo TEXT;
+
+-- Fourth statement would succeed if we got here
+ALTER TABLE items ADD COLUMN notes TEXT;
+SQL
+echo "-- rollback" > "migration/${mig3}/down.sql"
+
+# Try to apply migration (should fail)
+set +e
+migrator --db-url "$DB_URL" migrate up 2>&1
+exit_code=$?
+set -e
+
+if [ "$exit_code" = "0" ]; then
+  log error "test failed: ${WHITE}multi-statement broken migration should have failed"
+  exit 1
+fi
+
+log info "multi-statement migration failed as expected"
+
+# Verify migration was NOT recorded
+count_after_multi=$(sqlite3 "$SQLITE_DB" "SELECT COUNT(*) FROM hectic_migration")
+if [ "$count_after_multi" != "1" ]; then
+  log error "test failed: ${WHITE}failed multi-statement migration was recorded"
+  exit 1
+fi
+
+log info "✓ Failed migration was NOT recorded"
+
+# Verify NO partial changes were committed
+set +e
+sqlite3 "$SQLITE_DB" "SELECT location FROM items LIMIT 0" >/dev/null 2>&1
+location_exists=$?
+set -e
+
+if [ "$location_exists" = "0" ]; then
+  log error "test failed: ${WHITE}CRITICAL! 'location' column exists (partial commit in failed migration)"
+  exit 1
+fi
+
+log info "✓ No partial changes from multi-statement migration"
+
+# Verify temp_table was NOT created
+set +e
+sqlite3 "$SQLITE_DB" "SELECT * FROM temp_table LIMIT 0" >/dev/null 2>&1
+temp_exists=$?
+set -e
+
+if [ "$temp_exists" = "0" ]; then
+  log error "test failed: ${WHITE}temp_table exists (partial commit in failed migration)"
+  exit 1
+fi
+
+log info "✓ No tables created from failed migration"
+
+### CASE 3: Constraint violation rolls back transaction
+log notice "test case: ${WHITE}SQLite constraint violation rolls back transaction"
+
+mig4="20250101000004-constraint-violation"
+mkdir -p "migration/${mig4}"
+cat > "migration/${mig4}/up.sql" <<SQL
+-- Add column with UNIQUE constraint
+ALTER TABLE items ADD COLUMN code TEXT;
+CREATE UNIQUE INDEX items_code_unique ON items(code);
+
+-- Try to insert duplicate data (will violate UNIQUE constraint)
+INSERT INTO items (name, quantity, code) VALUES ('Item A', 10, 'CODE001');
+INSERT INTO items (name, quantity, code) VALUES ('Item B', 20, 'CODE001');  -- DUPLICATE!
+SQL
+echo "-- rollback" > "migration/${mig4}/down.sql"
+
+# Try to apply (should fail due to constraint violation)
+set +e
+migrator --db-url "$DB_URL" migrate up 2>&1
+exit_code=$?
+set -e
+
+if [ "$exit_code" = "0" ]; then
+  log error "test failed: ${WHITE}constraint violation migration should have failed"
+  exit 1
+fi
+
+log info "constraint violation migration failed as expected"
+
+# Verify migration was NOT recorded
+final_count=$(sqlite3 "$SQLITE_DB" "SELECT COUNT(*) FROM hectic_migration")
+if [ "$final_count" != "1" ]; then
+  log error "test failed: ${WHITE}constraint violation migration was recorded"
+  exit 1
+fi
+
+log info "✓ Constraint violation migration was NOT recorded"
+
+# Verify code column was NOT added (full rollback)
+set +e
+sqlite3 "$SQLITE_DB" "SELECT code FROM items LIMIT 0" >/dev/null 2>&1
+code_exists=$?
+set -e
+
+if [ "$code_exists" = "0" ]; then
+  log error "test failed: ${WHITE}code column exists after constraint violation"
+  exit 1
+fi
+
+log info "✓ All changes rolled back after constraint violation"
+
+# Verify no data was committed
+final_row_count=$(sqlite3 "$SQLITE_DB" "SELECT COUNT(*) FROM items")
+if [ "$final_row_count" != "0" ]; then
+  log error "test failed: ${WHITE}data exists after constraint violation rollback (got $final_row_count rows)"
+  exit 1
+fi
+
+log info "✓ No data committed after constraint violation"
+
+log notice "test passed: all SQLite migration failures properly roll back transactions"
+