From 89be2a9d7f9190aee0ee557fa5940855e6d3ffb4 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Fri, 26 Dec 2025 20:57:57 -0800 Subject: [PATCH] fix: add idempotency check to migration 028 (tombstone_closed_at) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migration 028 lacked an idempotency check, causing it to fail on databases where the migration had already been applied. The migration would attempt to copy data from issues to issues_new, but both tables had the same CHECK constraint, causing the insert to fail. Added check for "status = 'tombstone'" in the table schema to detect if the migration has already been applied and skip if so. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../sqlite/migrations/028_tombstone_closed_at.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/storage/sqlite/migrations/028_tombstone_closed_at.go b/internal/storage/sqlite/migrations/028_tombstone_closed_at.go index 7b2eff4e..32d7b66c 100644 --- a/internal/storage/sqlite/migrations/028_tombstone_closed_at.go +++ b/internal/storage/sqlite/migrations/028_tombstone_closed_at.go @@ -3,6 +3,7 @@ package migrations import ( "database/sql" "fmt" + "strings" ) // MigrateTombstoneClosedAt updates the closed_at constraint to allow tombstones @@ -22,8 +23,20 @@ func MigrateTombstoneClosedAt(db *sql.DB) error { // SQLite doesn't support ALTER TABLE to modify CHECK constraints // We must recreate the table with the new constraint + // Idempotency check: see if the new CHECK constraint already exists + // The new constraint contains "status = 'tombstone'" which the old one didn't + var tableSql string + err := db.QueryRow(`SELECT sql FROM sqlite_master WHERE type='table' AND name='issues'`).Scan(&tableSql) + if err != nil { + return fmt.Errorf("failed to get issues table schema: %w", err) + } + // If the schema already has the tombstone clause, migration is already applied + if strings.Contains(tableSql, "status = 'tombstone'") || strings.Contains(tableSql, `status = "tombstone"`) { + return nil + } + // Step 0: Drop views that depend on the issues table - _, err := db.Exec(`DROP VIEW IF EXISTS ready_issues`) + _, err = db.Exec(`DROP VIEW IF EXISTS ready_issues`) if err != nil { return fmt.Errorf("failed to drop ready_issues view: %w", err) }