Add schema compatibility probe to prevent silent migration failures (bd-ckvw)
- Implement comprehensive schema probe in sqlite.New() that verifies all expected tables and columns after migrations - Add retry logic: if probe fails, retry migrations once - Return clear fatal error with missing schema elements if probe still fails - Enhance daemon version gating: refuse RPC if client has newer minor version - Improve checkVersionMismatch messaging: verify schema before claiming upgrade - Add schema compatibility check to bd doctor command - Add comprehensive tests for schema probing This prevents the silent migration failure bug where: 1. Migrations fail silently 2. Database queries fail with 'no such column' errors 3. Import logic misinterprets as 'not found' and tries INSERT 4. Results in cryptic UNIQUE constraint errors Fixes #262 Amp-Thread-ID: https://ampcode.com/threads/T-0d7ae2c0-9f12-4f9b-85d1-1291488af150 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
121
internal/storage/sqlite/schema_probe.go
Normal file
121
internal/storage/sqlite/schema_probe.go
Normal file
@@ -0,0 +1,121 @@
|
||||
// Package sqlite - schema compatibility probing
|
||||
package sqlite
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"fmt"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// ErrSchemaIncompatible is returned when the database schema is incompatible with the current version
|
||||
var ErrSchemaIncompatible = fmt.Errorf("database schema is incompatible")
|
||||
|
||||
// expectedSchema defines all expected tables and their required columns
|
||||
// This is used to verify migrations completed successfully
|
||||
var expectedSchema = map[string][]string{
|
||||
"issues": {
|
||||
"id", "title", "description", "design", "acceptance_criteria", "notes",
|
||||
"status", "priority", "issue_type", "assignee", "estimated_minutes",
|
||||
"created_at", "updated_at", "closed_at", "content_hash", "external_ref",
|
||||
"compaction_level", "compacted_at", "compacted_at_commit", "original_size",
|
||||
},
|
||||
"dependencies": {"issue_id", "depends_on_id", "type", "created_at", "created_by"},
|
||||
"labels": {"issue_id", "label"},
|
||||
"comments": {"id", "issue_id", "author", "text", "created_at"},
|
||||
"events": {"id", "issue_id", "event_type", "actor", "old_value", "new_value", "comment", "created_at"},
|
||||
"config": {"key", "value"},
|
||||
"metadata": {"key", "value"},
|
||||
"dirty_issues": {"issue_id", "marked_at"},
|
||||
"export_hashes": {"issue_id", "content_hash", "exported_at"},
|
||||
"child_counters": {"parent_id", "last_child"},
|
||||
"issue_snapshots": {"id", "issue_id", "snapshot_time", "compaction_level", "original_size", "compressed_size", "original_content", "archived_events"},
|
||||
"compaction_snapshots": {"id", "issue_id", "compaction_level", "snapshot_json", "created_at"},
|
||||
"repo_mtimes": {"repo_path", "jsonl_path", "mtime_ns", "last_checked"},
|
||||
}
|
||||
|
||||
// SchemaProbeResult contains the results of a schema compatibility check
|
||||
type SchemaProbeResult struct {
|
||||
Compatible bool
|
||||
MissingTables []string
|
||||
MissingColumns map[string][]string // table -> missing columns
|
||||
ErrorMessage string
|
||||
}
|
||||
|
||||
// probeSchema verifies all expected tables and columns exist
|
||||
// Returns SchemaProbeResult with details about any missing schema elements
|
||||
func probeSchema(db *sql.DB) SchemaProbeResult {
|
||||
result := SchemaProbeResult{
|
||||
Compatible: true,
|
||||
MissingTables: []string{},
|
||||
MissingColumns: make(map[string][]string),
|
||||
}
|
||||
|
||||
for table, expectedCols := range expectedSchema {
|
||||
// Try to query the table with all expected columns
|
||||
query := fmt.Sprintf("SELECT %s FROM %s LIMIT 0", strings.Join(expectedCols, ", "), table)
|
||||
_, err := db.Exec(query)
|
||||
|
||||
if err != nil {
|
||||
errMsg := err.Error()
|
||||
|
||||
// Check if table doesn't exist
|
||||
if strings.Contains(errMsg, "no such table") {
|
||||
result.Compatible = false
|
||||
result.MissingTables = append(result.MissingTables, table)
|
||||
continue
|
||||
}
|
||||
|
||||
// Check if column doesn't exist
|
||||
if strings.Contains(errMsg, "no such column") {
|
||||
result.Compatible = false
|
||||
// Try to find which columns are missing
|
||||
missingCols := findMissingColumns(db, table, expectedCols)
|
||||
if len(missingCols) > 0 {
|
||||
result.MissingColumns[table] = missingCols
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Build error message if incompatible
|
||||
if !result.Compatible {
|
||||
var parts []string
|
||||
if len(result.MissingTables) > 0 {
|
||||
parts = append(parts, fmt.Sprintf("missing tables: %s", strings.Join(result.MissingTables, ", ")))
|
||||
}
|
||||
if len(result.MissingColumns) > 0 {
|
||||
for table, cols := range result.MissingColumns {
|
||||
parts = append(parts, fmt.Sprintf("missing columns in %s: %s", table, strings.Join(cols, ", ")))
|
||||
}
|
||||
}
|
||||
result.ErrorMessage = strings.Join(parts, "; ")
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
// findMissingColumns determines which columns are missing from a table
|
||||
func findMissingColumns(db *sql.DB, table string, expectedCols []string) []string {
|
||||
missing := []string{}
|
||||
|
||||
for _, col := range expectedCols {
|
||||
query := fmt.Sprintf("SELECT %s FROM %s LIMIT 0", col, table)
|
||||
_, err := db.Exec(query)
|
||||
if err != nil && strings.Contains(err.Error(), "no such column") {
|
||||
missing = append(missing, col)
|
||||
}
|
||||
}
|
||||
|
||||
return missing
|
||||
}
|
||||
|
||||
// verifySchemaCompatibility runs schema probe and returns detailed error on failure
|
||||
func verifySchemaCompatibility(db *sql.DB) error {
|
||||
result := probeSchema(db)
|
||||
|
||||
if !result.Compatible {
|
||||
return fmt.Errorf("%w: %s", ErrSchemaIncompatible, result.ErrorMessage)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
207
internal/storage/sqlite/schema_probe_test.go
Normal file
207
internal/storage/sqlite/schema_probe_test.go
Normal file
@@ -0,0 +1,207 @@
|
||||
package sqlite
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"testing"
|
||||
|
||||
_ "github.com/ncruces/go-sqlite3/driver"
|
||||
_ "github.com/ncruces/go-sqlite3/embed"
|
||||
)
|
||||
|
||||
func TestProbeSchema_AllTablesPresent(t *testing.T) {
|
||||
// Create in-memory database with full schema
|
||||
db, err := sql.Open("sqlite3", ":memory:")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to open database: %v", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
// Initialize schema and run migrations
|
||||
if _, err := db.Exec(schema); err != nil {
|
||||
t.Fatalf("failed to initialize schema: %v", err)
|
||||
}
|
||||
if err := RunMigrations(db); err != nil {
|
||||
t.Fatalf("failed to run migrations: %v", err)
|
||||
}
|
||||
|
||||
// Run schema probe
|
||||
result := probeSchema(db)
|
||||
|
||||
// Should be compatible
|
||||
if !result.Compatible {
|
||||
t.Errorf("expected schema to be compatible, got: %s", result.ErrorMessage)
|
||||
}
|
||||
if len(result.MissingTables) > 0 {
|
||||
t.Errorf("unexpected missing tables: %v", result.MissingTables)
|
||||
}
|
||||
if len(result.MissingColumns) > 0 {
|
||||
t.Errorf("unexpected missing columns: %v", result.MissingColumns)
|
||||
}
|
||||
}
|
||||
|
||||
func TestProbeSchema_MissingTable(t *testing.T) {
|
||||
// Create in-memory database without child_counters table
|
||||
db, err := sql.Open("sqlite3", ":memory:")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to open database: %v", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
// Create minimal schema (just issues table)
|
||||
_, err = db.Exec(`
|
||||
CREATE TABLE issues (
|
||||
id TEXT PRIMARY KEY,
|
||||
title TEXT NOT NULL,
|
||||
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,
|
||||
content_hash TEXT,
|
||||
external_ref TEXT,
|
||||
compaction_level INTEGER DEFAULT 0,
|
||||
compacted_at DATETIME,
|
||||
compacted_at_commit TEXT,
|
||||
original_size INTEGER
|
||||
)
|
||||
`)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create issues table: %v", err)
|
||||
}
|
||||
|
||||
// Run schema probe
|
||||
result := probeSchema(db)
|
||||
|
||||
// Should not be compatible
|
||||
if result.Compatible {
|
||||
t.Error("expected schema to be incompatible (missing tables)")
|
||||
}
|
||||
if len(result.MissingTables) == 0 {
|
||||
t.Error("expected missing tables to be reported")
|
||||
}
|
||||
}
|
||||
|
||||
func TestProbeSchema_MissingColumn(t *testing.T) {
|
||||
// Create in-memory database with issues table missing content_hash
|
||||
db, err := sql.Open("sqlite3", ":memory:")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to open database: %v", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
// Create issues table WITHOUT content_hash column
|
||||
_, err = db.Exec(`
|
||||
CREATE TABLE issues (
|
||||
id TEXT PRIMARY KEY,
|
||||
title TEXT NOT NULL,
|
||||
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,
|
||||
compaction_level INTEGER DEFAULT 0,
|
||||
compacted_at DATETIME,
|
||||
compacted_at_commit TEXT,
|
||||
original_size INTEGER
|
||||
);
|
||||
CREATE TABLE dependencies (
|
||||
issue_id TEXT NOT NULL,
|
||||
depends_on_id TEXT NOT NULL,
|
||||
type TEXT NOT NULL DEFAULT 'blocks',
|
||||
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||||
created_by TEXT NOT NULL,
|
||||
PRIMARY KEY (issue_id, depends_on_id)
|
||||
);
|
||||
CREATE TABLE labels (issue_id TEXT NOT NULL, label TEXT NOT NULL, PRIMARY KEY (issue_id, label));
|
||||
CREATE TABLE comments (id INTEGER PRIMARY KEY AUTOINCREMENT, issue_id TEXT NOT NULL, author TEXT NOT NULL, text TEXT NOT NULL, created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP);
|
||||
CREATE TABLE events (id INTEGER PRIMARY KEY AUTOINCREMENT, issue_id TEXT NOT NULL, event_type TEXT NOT NULL, actor TEXT NOT NULL, old_value TEXT, new_value TEXT, comment TEXT, created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP);
|
||||
CREATE TABLE config (key TEXT PRIMARY KEY, value TEXT NOT NULL);
|
||||
CREATE TABLE metadata (key TEXT PRIMARY KEY, value TEXT NOT NULL);
|
||||
CREATE TABLE dirty_issues (issue_id TEXT PRIMARY KEY, marked_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP);
|
||||
CREATE TABLE export_hashes (issue_id TEXT PRIMARY KEY, content_hash TEXT NOT NULL, exported_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP);
|
||||
CREATE TABLE child_counters (parent_id TEXT PRIMARY KEY, last_child INTEGER NOT NULL DEFAULT 0);
|
||||
CREATE TABLE issue_snapshots (id INTEGER PRIMARY KEY AUTOINCREMENT, issue_id TEXT NOT NULL, snapshot_time DATETIME NOT NULL, compaction_level INTEGER NOT NULL, original_size INTEGER NOT NULL, compressed_size INTEGER NOT NULL, original_content TEXT NOT NULL, archived_events TEXT);
|
||||
CREATE TABLE compaction_snapshots (id INTEGER PRIMARY KEY AUTOINCREMENT, issue_id TEXT NOT NULL, compaction_level INTEGER NOT NULL, snapshot_json BLOB NOT NULL, created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP);
|
||||
CREATE TABLE repo_mtimes (repo_path TEXT PRIMARY KEY, jsonl_path TEXT NOT NULL, mtime_ns INTEGER NOT NULL, last_checked DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP);
|
||||
`)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create tables: %v", err)
|
||||
}
|
||||
|
||||
// Run schema probe
|
||||
result := probeSchema(db)
|
||||
|
||||
// Should not be compatible
|
||||
if result.Compatible {
|
||||
t.Error("expected schema to be incompatible (missing content_hash column)")
|
||||
}
|
||||
if len(result.MissingColumns) == 0 {
|
||||
t.Error("expected missing columns to be reported")
|
||||
}
|
||||
if _, ok := result.MissingColumns["issues"]; !ok {
|
||||
t.Error("expected missing columns in issues table")
|
||||
}
|
||||
}
|
||||
|
||||
func TestVerifySchemaCompatibility(t *testing.T) {
|
||||
// Create in-memory database with full schema
|
||||
db, err := sql.Open("sqlite3", ":memory:")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to open database: %v", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
// Initialize schema and run migrations
|
||||
if _, err := db.Exec(schema); err != nil {
|
||||
t.Fatalf("failed to initialize schema: %v", err)
|
||||
}
|
||||
if err := RunMigrations(db); err != nil {
|
||||
t.Fatalf("failed to run migrations: %v", err)
|
||||
}
|
||||
|
||||
// Verify schema compatibility
|
||||
err = verifySchemaCompatibility(db)
|
||||
if err != nil {
|
||||
t.Errorf("expected schema to be compatible, got error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestVerifySchemaCompatibility_Incompatible(t *testing.T) {
|
||||
// Create in-memory database with minimal schema
|
||||
db, err := sql.Open("sqlite3", ":memory:")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to open database: %v", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
// Create minimal schema
|
||||
_, err = db.Exec(`CREATE TABLE issues (id TEXT PRIMARY KEY, title TEXT NOT NULL)`)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create issues table: %v", err)
|
||||
}
|
||||
|
||||
// Verify schema compatibility
|
||||
err = verifySchemaCompatibility(db)
|
||||
if err == nil {
|
||||
t.Error("expected schema incompatibility error, got nil")
|
||||
}
|
||||
if err != nil && err != ErrSchemaIncompatible {
|
||||
// Check that error wraps ErrSchemaIncompatible
|
||||
t.Logf("got error: %v", err)
|
||||
}
|
||||
}
|
||||
@@ -78,6 +78,21 @@ func New(path string) (*SQLiteStorage, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Verify schema compatibility after migrations (bd-ckvw)
|
||||
// First attempt
|
||||
if err := verifySchemaCompatibility(db); err != nil {
|
||||
// Schema probe failed - retry migrations once
|
||||
if retryErr := RunMigrations(db); retryErr != nil {
|
||||
return nil, fmt.Errorf("migration retry failed after schema probe failure: %w (original: %v)", retryErr, err)
|
||||
}
|
||||
|
||||
// Probe again after retry
|
||||
if err := verifySchemaCompatibility(db); err != nil {
|
||||
// Still failing - return fatal error with clear message
|
||||
return nil, fmt.Errorf("schema probe failed after migration retry: %w. Database may be corrupted or from incompatible version. Run 'bd doctor' to diagnose", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Convert to absolute path for consistency (but keep :memory: as-is)
|
||||
absPath := path
|
||||
if path != ":memory:" {
|
||||
|
||||
Reference in New Issue
Block a user