From 6d84701a5f3c045eb672d3c211765b195b23fc1a Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Tue, 30 Dec 2025 16:44:31 -0800 Subject: [PATCH] fix: migration 022 fails with SQL syntax error on v0.30.3 upgrade db.Exec was being passed %s format specifiers directly in the SQL string, but db.Exec does not do printf-style formatting - it uses the variadic args as SQL parameter bindings. This caused the literal %s to be sent to SQLite, which failed with near percent: syntax error. Fixed by using fmt.Sprintf to interpolate the column expressions into the SQL string before passing to db.Exec. Added regression test that creates an old-schema database with edge columns (replies_to, relates_to, duplicate_of, superseded_by) and verifies migration 022 completes successfully. Fixes #809 Generated with Claude Code Co-Authored-By: Claude Opus 4.5 --- .../migrations/022_drop_edge_columns.go | 5 +- .../migrations/022_drop_edge_columns_test.go | 226 ++++++++++++++++++ 2 files changed, 230 insertions(+), 1 deletion(-) create mode 100644 internal/storage/sqlite/migrations/022_drop_edge_columns_test.go diff --git a/internal/storage/sqlite/migrations/022_drop_edge_columns.go b/internal/storage/sqlite/migrations/022_drop_edge_columns.go index b2cee13e..1fd7d89d 100644 --- a/internal/storage/sqlite/migrations/022_drop_edge_columns.go +++ b/internal/storage/sqlite/migrations/022_drop_edge_columns.go @@ -183,7 +183,9 @@ func MigrateDropEdgeColumns(db *sql.DB) error { } // Copy data from old table to new table (excluding deprecated columns) - _, err = db.Exec(` + // NOTE: We use fmt.Sprintf here (not db.Exec parameters) because we're interpolating + // column names/expressions, not values. db.Exec parameters only work for VALUES. + copySQL := fmt.Sprintf(` INSERT INTO issues_new ( id, content_hash, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, @@ -203,6 +205,7 @@ func MigrateDropEdgeColumns(db *sql.DB) error { COALESCE(close_reason, '') FROM issues `, pinnedExpr, isTemplateExpr, awaitTypeExpr, awaitIDExpr, timeoutNsExpr, waitersExpr) + _, err = db.Exec(copySQL) if err != nil { return fmt.Errorf("failed to copy issues data: %w", err) } diff --git a/internal/storage/sqlite/migrations/022_drop_edge_columns_test.go b/internal/storage/sqlite/migrations/022_drop_edge_columns_test.go new file mode 100644 index 00000000..f02efa91 --- /dev/null +++ b/internal/storage/sqlite/migrations/022_drop_edge_columns_test.go @@ -0,0 +1,226 @@ +package migrations + +import ( + "database/sql" + "testing" + + _ "github.com/ncruces/go-sqlite3/driver" + _ "github.com/ncruces/go-sqlite3/embed" +) + +// TestMigrateDropEdgeColumns_FromOldSchema verifies that migration 022 works +// when upgrading from a database that still has the deprecated edge columns. +// This is a regression test for GitHub issue #809 where the migration failed +// with "SQL logic error: near '%': syntax error" because db.Exec was being +// called with %s format specifiers instead of using fmt.Sprintf first. +func TestMigrateDropEdgeColumns_FromOldSchema(t *testing.T) { + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("failed to open database: %v", err) + } + defer db.Close() + + // Create old schema WITH the deprecated edge columns (simulating v0.30.3) + // This is a simplified version of the old schema with just the columns + // needed to trigger the migration path. + _, err = db.Exec(` + CREATE TABLE issues ( + id TEXT PRIMARY KEY, + content_hash TEXT, + title TEXT NOT NULL CHECK(length(title) <= 500), + description TEXT NOT NULL DEFAULT '', + design TEXT NOT NULL DEFAULT '', + acceptance_criteria TEXT NOT NULL DEFAULT '', + notes TEXT NOT NULL DEFAULT '', + status TEXT NOT NULL DEFAULT 'open', + priority INTEGER NOT NULL DEFAULT 2 CHECK(priority >= 0 AND priority <= 4), + issue_type TEXT NOT NULL DEFAULT 'task', + assignee TEXT, + estimated_minutes INTEGER, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + closed_at DATETIME, + external_ref TEXT, + source_repo TEXT DEFAULT '', + compaction_level INTEGER DEFAULT 0, + compacted_at DATETIME, + compacted_at_commit TEXT, + original_size INTEGER, + deleted_at DATETIME, + deleted_by TEXT DEFAULT '', + delete_reason TEXT DEFAULT '', + original_type TEXT DEFAULT '', + sender TEXT DEFAULT '', + ephemeral INTEGER DEFAULT 0, + close_reason TEXT DEFAULT '', + -- These are the deprecated edge columns that trigger migration 022 + replies_to TEXT, + relates_to TEXT, + duplicate_of TEXT, + superseded_by TEXT, + CHECK ((status = 'closed') = (closed_at IS NOT NULL)) + ) + `) + if err != nil { + t.Fatalf("failed to create old issues table: %v", err) + } + + // Create the dependencies table (required by migration 022) + _, err = db.Exec(` + CREATE TABLE IF NOT EXISTS dependencies ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + issue_id TEXT NOT NULL, + depends_on_id TEXT NOT NULL, + type TEXT NOT NULL DEFAULT 'blocks', + metadata TEXT DEFAULT '{}', + thread_id TEXT DEFAULT '', + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + UNIQUE(issue_id, depends_on_id, type) + ) + `) + if err != nil { + t.Fatalf("failed to create dependencies table: %v", err) + } + + // Insert test data to ensure the migration copies correctly + _, err = db.Exec(` + INSERT INTO issues (id, title, status, replies_to, relates_to) + VALUES ('test-001', 'Test Issue', 'open', 'old-ref-1', 'old-ref-2') + `) + if err != nil { + t.Fatalf("failed to insert test issue: %v", err) + } + + // Run the migration - this should NOT fail with SQL syntax error + err = MigrateDropEdgeColumns(db) + if err != nil { + t.Fatalf("MigrateDropEdgeColumns failed: %v", err) + } + + // Verify the issue still exists and data was preserved + var id, title, status string + err = db.QueryRow(`SELECT id, title, status FROM issues WHERE id = 'test-001'`).Scan(&id, &title, &status) + if err != nil { + t.Fatalf("failed to query migrated issue: %v", err) + } + if title != "Test Issue" { + t.Errorf("title mismatch: got %q, want %q", title, "Test Issue") + } + if status != "open" { + t.Errorf("status mismatch: got %q, want %q", status, "open") + } + + // Verify the edge columns no longer exist + var colCount int + err = db.QueryRow(` + SELECT COUNT(*) + FROM pragma_table_info('issues') + WHERE name IN ('replies_to', 'relates_to', 'duplicate_of', 'superseded_by') + `).Scan(&colCount) + if err != nil { + t.Fatalf("failed to check columns: %v", err) + } + if colCount != 0 { + t.Errorf("expected edge columns to be removed, but %d still exist", colCount) + } +} + +// TestMigrateDropEdgeColumns_WithPinnedAndTemplate verifies that the migration +// correctly preserves pinned and is_template columns when they exist. +func TestMigrateDropEdgeColumns_WithPinnedAndTemplate(t *testing.T) { + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("failed to open database: %v", err) + } + defer db.Close() + + // Create schema with edge columns AND pinned/is_template columns + _, err = db.Exec(` + CREATE TABLE issues ( + id TEXT PRIMARY KEY, + content_hash TEXT, + title TEXT NOT NULL CHECK(length(title) <= 500), + description TEXT NOT NULL DEFAULT '', + design TEXT NOT NULL DEFAULT '', + acceptance_criteria TEXT NOT NULL DEFAULT '', + notes TEXT NOT NULL DEFAULT '', + status TEXT NOT NULL DEFAULT 'open', + priority INTEGER NOT NULL DEFAULT 2, + issue_type TEXT NOT NULL DEFAULT 'task', + assignee TEXT, + estimated_minutes INTEGER, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + closed_at DATETIME, + external_ref TEXT, + source_repo TEXT DEFAULT '', + compaction_level INTEGER DEFAULT 0, + compacted_at DATETIME, + compacted_at_commit TEXT, + original_size INTEGER, + deleted_at DATETIME, + deleted_by TEXT DEFAULT '', + delete_reason TEXT DEFAULT '', + original_type TEXT DEFAULT '', + sender TEXT DEFAULT '', + ephemeral INTEGER DEFAULT 0, + close_reason TEXT DEFAULT '', + pinned INTEGER DEFAULT 0, + is_template INTEGER DEFAULT 0, + -- Deprecated edge columns + replies_to TEXT, + relates_to TEXT, + duplicate_of TEXT, + superseded_by TEXT, + CHECK ((status = 'closed') = (closed_at IS NOT NULL)) + ) + `) + if err != nil { + t.Fatalf("failed to create issues table: %v", err) + } + + // Create dependencies table + _, err = db.Exec(` + CREATE TABLE IF NOT EXISTS dependencies ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + issue_id TEXT NOT NULL, + depends_on_id TEXT NOT NULL, + type TEXT NOT NULL DEFAULT 'blocks', + metadata TEXT DEFAULT '{}', + thread_id TEXT DEFAULT '', + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + UNIQUE(issue_id, depends_on_id, type) + ) + `) + if err != nil { + t.Fatalf("failed to create dependencies table: %v", err) + } + + // Insert test data with pinned and is_template set + _, err = db.Exec(` + INSERT INTO issues (id, title, status, pinned, is_template) + VALUES ('test-001', 'Pinned Template', 'open', 1, 1) + `) + if err != nil { + t.Fatalf("failed to insert test issue: %v", err) + } + + // Run migration + err = MigrateDropEdgeColumns(db) + if err != nil { + t.Fatalf("MigrateDropEdgeColumns failed: %v", err) + } + + // Verify pinned and is_template were preserved + var pinned, isTemplate int + err = db.QueryRow(`SELECT pinned, is_template FROM issues WHERE id = 'test-001'`).Scan(&pinned, &isTemplate) + if err != nil { + t.Fatalf("failed to query migrated issue: %v", err) + } + if pinned != 1 { + t.Errorf("pinned not preserved: got %d, want 1", pinned) + } + if isTemplate != 1 { + t.Errorf("is_template not preserved: got %d, want 1", isTemplate) + } +}