johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: dedupe error classification, fix --force+--source=db conflict

Browse Source 
Post-merge cleanup of PR #805:

1. Extract duplicate error classification logic into classifyDatabaseError()
   helper function (was duplicated in two places in database.go)

2. Fix semantic conflict between --force and --source=db flags:
   - --force implies "database is broken, rebuild from JSONL"
   - --source=db implies "use database as source of truth"
   - These are contradictory; now errors with clear message
   - --force with --source=auto or --source=jsonl works as expected

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Executed-By: beads/crew/dave
Rig: beads
Role: crew
This commit is contained in:
Steve Yegge
2025-12-30 10:37:53 -08:00
parent c86bffc045
commit eefeb1a5bc
2 changed files with 79 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

155
cmd/bd/doctor/database.go
View File

@@ -249,65 +249,19 @@ func CheckDatabaseIntegrity(path string) DoctorCheck {
// Open database in read-only mode for integrity check
db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true))
if err != nil {
// Classify the error type for better recovery guidance
errMsg := err.Error()
var errorType string
var recoverySteps string
// Check if JSONL recovery is possible
jsonlCount, _, jsonlErr := CountJSONLIssues(filepath.Join(beadsDir, "issues.jsonl"))
if jsonlErr != nil {
jsonlCount, _, jsonlErr = CountJSONLIssues(filepath.Join(beadsDir, "beads.jsonl"))
}
jsonlAvailable := jsonlErr == nil && jsonlCount > 0
// Classify error type
if strings.Contains(errMsg, "database is locked") {
errorType = "Database is locked"
recoverySteps = "1. Check for running bd processes: ps aux | grep bd\n" +
"2. Kill any stale processes\n" +
"3. Remove stale locks: rm .beads/beads.db-shm .beads/beads.db-wal .beads/daemon.lock\n" +
"4. Retry: bd doctor --fix"
} else if strings.Contains(errMsg, "not a database") || strings.Contains(errMsg, "file is not a database") {
errorType = "File is not a valid SQLite database"
if jsonlErr == nil && jsonlCount > 0 {
recoverySteps = fmt.Sprintf("Database file is corrupted beyond repair.\n\n"+
"Recovery steps:\n"+
"1. Backup corrupt database: mv .beads/beads.db .beads/beads.db.broken\n"+
"2. Rebuild from JSONL (%d issues): bd doctor --fix --force --source=jsonl\n"+
"3. Verify: bd stats", jsonlCount)
} else {
recoverySteps = "Database file is corrupted and no JSONL backup found.\n" +
"Manual recovery required:\n" +
"1. Restore from git: git checkout HEAD -- .beads/issues.jsonl\n" +
"2. Rebuild database: bd doctor --fix --force"
}
} else if strings.Contains(errMsg, "migration") || strings.Contains(errMsg, "validation failed") {
errorType = "Database migration or validation failed"
if jsonlErr == nil && jsonlCount > 0 {
recoverySteps = fmt.Sprintf("Database has validation errors (possibly orphaned dependencies).\n\n"+
"Recovery steps:\n"+
"1. Backup database: mv .beads/beads.db .beads/beads.db.broken\n"+
"2. Rebuild from JSONL (%d issues): bd doctor --fix --force --source=jsonl\n"+
"3. Verify: bd stats\n\n"+
"Alternative: bd doctor --fix --force (attempts to repair in-place)", jsonlCount)
} else {
recoverySteps = "Database validation failed and no JSONL backup available.\n" +
"Try: bd doctor --fix --force"
}
} else {
errorType = "Failed to open database"
if jsonlErr == nil && jsonlCount > 0 {
recoverySteps = fmt.Sprintf("Run 'bd doctor --fix --source=jsonl' to rebuild from JSONL (%d issues)", jsonlCount)
} else {
recoverySteps = "Run 'bd doctor --fix --force' to attempt recovery"
}
}
errorType, recoverySteps := classifyDatabaseError(err.Error(), jsonlCount, jsonlAvailable)
return DoctorCheck{
Name: "Database Integrity",
Status: StatusError,
Message: errorType,
Detail: fmt.Sprintf("%s\n\nError: %s", recoverySteps, errMsg),
Detail: fmt.Sprintf("%s\n\nError: %s", recoverySteps, err.Error()),
Fix: "See recovery steps above",
}
}
@@ -317,65 +271,23 @@ func CheckDatabaseIntegrity(path string) DoctorCheck {
// This checks the entire database for corruption
rows, err := db.Query("PRAGMA integrity_check")
if err != nil {
// Classify the error type for better recovery guidance
errMsg := err.Error()
var errorType string
var recoverySteps string
// Check if JSONL recovery is possible
jsonlCount, _, jsonlErr := CountJSONLIssues(filepath.Join(beadsDir, "issues.jsonl"))
if jsonlErr != nil {
jsonlCount, _, jsonlErr = CountJSONLIssues(filepath.Join(beadsDir, "beads.jsonl"))
}
jsonlAvailable := jsonlErr == nil && jsonlCount > 0
// Classify error type (same logic as opening errors)
if strings.Contains(errMsg, "database is locked") {
errorType = "Database is locked"
recoverySteps = "1. Check for running bd processes: ps aux | grep bd\n" +
"2. Kill any stale processes\n" +
"3. Remove stale locks: rm .beads/beads.db-shm .beads/beads.db-wal .beads/daemon.lock\n" +
"4. Retry: bd doctor --fix"
} else if strings.Contains(errMsg, "not a database") || strings.Contains(errMsg, "file is not a database") {
errorType = "File is not a valid SQLite database"
if jsonlErr == nil && jsonlCount > 0 {
recoverySteps = fmt.Sprintf("Database file is corrupted beyond repair.\n\n"+
"Recovery steps:\n"+
"1. Backup corrupt database: mv .beads/beads.db .beads/beads.db.broken\n"+
"2. Rebuild from JSONL (%d issues): bd doctor --fix --force --source=jsonl\n"+
"3. Verify: bd stats", jsonlCount)
} else {
recoverySteps = "Database file is corrupted and no JSONL backup found.\n" +
"Manual recovery required:\n" +
"1. Restore from git: git checkout HEAD -- .beads/issues.jsonl\n" +
"2. Rebuild database: bd doctor --fix --force"
}
} else if strings.Contains(errMsg, "migration") || strings.Contains(errMsg, "validation failed") {
errorType = "Database migration or validation failed"
if jsonlErr == nil && jsonlCount > 0 {
recoverySteps = fmt.Sprintf("Database has validation errors (possibly orphaned dependencies).\n\n"+
"Recovery steps:\n"+
"1. Backup database: mv .beads/beads.db .beads/beads.db.broken\n"+
"2. Rebuild from JSONL (%d issues): bd doctor --fix --force --source=jsonl\n"+
"3. Verify: bd stats\n\n"+
"Alternative: bd doctor --fix --force (attempts to repair in-place)", jsonlCount)
} else {
recoverySteps = "Database validation failed and no JSONL backup available.\n" +
"Try: bd doctor --fix --force"
}
} else {
errorType, recoverySteps := classifyDatabaseError(err.Error(), jsonlCount, jsonlAvailable)
// Override default error type for this specific case
if errorType == "Failed to open database" {
errorType = "Failed to run integrity check"
if jsonlErr == nil && jsonlCount > 0 {
recoverySteps = fmt.Sprintf("Run 'bd doctor --fix --force --source=jsonl' to rebuild from JSONL (%d issues)", jsonlCount)
} else {
recoverySteps = "Run 'bd doctor --fix --force' to attempt recovery"
}
}
return DoctorCheck{
Name: "Database Integrity",
Status: StatusError,
Message: errorType,
Detail: fmt.Sprintf("%s\n\nError: %s", recoverySteps, errMsg),
Detail: fmt.Sprintf("%s\n\nError: %s", recoverySteps, err.Error()),
Fix: "See recovery steps above",
}
}
@@ -649,6 +561,57 @@ func FixDBJSONLSync(path string) error {
// Helper functions
// classifyDatabaseError classifies a database error and returns appropriate recovery guidance.
// Returns the error type description and recovery steps based on error message and JSONL availability.
func classifyDatabaseError(errMsg string, jsonlCount int, jsonlAvailable bool) (errorType, recoverySteps string) {
switch {
case strings.Contains(errMsg, "database is locked"):
errorType = "Database is locked"
recoverySteps = "1. Check for running bd processes: ps aux | grep bd\n" +
"2. Kill any stale processes\n" +
"3. Remove stale locks: rm .beads/beads.db-shm .beads/beads.db-wal .beads/daemon.lock\n" +
"4. Retry: bd doctor --fix"
case strings.Contains(errMsg, "not a database") || strings.Contains(errMsg, "file is not a database"):
errorType = "File is not a valid SQLite database"
if jsonlAvailable {
recoverySteps = fmt.Sprintf("Database file is corrupted beyond repair.\n\n"+
"Recovery steps:\n"+
"1. Backup corrupt database: mv .beads/beads.db .beads/beads.db.broken\n"+
"2. Rebuild from JSONL (%d issues): bd doctor --fix --force --source=jsonl\n"+
"3. Verify: bd stats", jsonlCount)
} else {
recoverySteps = "Database file is corrupted and no JSONL backup found.\n" +
"Manual recovery required:\n" +
"1. Restore from git: git checkout HEAD -- .beads/issues.jsonl\n" +
"2. Rebuild database: bd doctor --fix --force"
}
case strings.Contains(errMsg, "migration") || strings.Contains(errMsg, "validation failed"):
errorType = "Database migration or validation failed"
if jsonlAvailable {
recoverySteps = fmt.Sprintf("Database has validation errors (possibly orphaned dependencies).\n\n"+
"Recovery steps:\n"+
"1. Backup database: mv .beads/beads.db .beads/beads.db.broken\n"+
"2. Rebuild from JSONL (%d issues): bd doctor --fix --force --source=jsonl\n"+
"3. Verify: bd stats\n\n"+
"Alternative: bd doctor --fix --force (attempts to repair in-place)", jsonlCount)
} else {
recoverySteps = "Database validation failed and no JSONL backup available.\n" +
"Try: bd doctor --fix --force"
}
default:
errorType = "Failed to open database"
if jsonlAvailable {
recoverySteps = fmt.Sprintf("Run 'bd doctor --fix --source=jsonl' to rebuild from JSONL (%d issues)", jsonlCount)
} else {
recoverySteps = "Run 'bd doctor --fix --force' to attempt recovery"
}
}
return
}
// getDatabaseVersionFromPath reads the database version from the given path
func getDatabaseVersionFromPath(dbPath string) string {
db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true))

30
cmd/bd/doctor/fix/recovery.go
View File

@@ -108,6 +108,11 @@ func DatabaseCorruptionRecoveryWithOptions(path string, force bool, source strin
jsonlPath := findJSONLPath(beadsDir)
jsonlExists := jsonlPath != ""
// Check for contradictory flags early
if force && source == "db" {
return fmt.Errorf("--force and --source=db are contradictory: --force implies the database is broken and recovery should use JSONL. Use --source=jsonl or --source=auto with --force")
}
// Determine source of truth based on --source flag and availability
var useJSONL bool
switch source {
@@ -117,17 +122,28 @@ func DatabaseCorruptionRecoveryWithOptions(path string, force bool, source strin
return fmt.Errorf("--source=jsonl specified but no JSONL file found")
}
useJSONL = true
fmt.Println(" Using JSONL as source of truth (--source=jsonl)")
if force {
fmt.Println(" Using JSONL as source of truth (--force --source=jsonl)")
} else {
fmt.Println(" Using JSONL as source of truth (--source=jsonl)")
}
case "db":
// Explicit database preference
// Explicit database preference (already checked for force+db contradiction above)
if !dbExists {
return fmt.Errorf("--source=db specified but no database found")
}
useJSONL = false
fmt.Println(" Using database as source of truth (--source=db)")
case "auto":
// Auto-detect: prefer JSONL if database is corrupted, otherwise prefer database
if !dbExists && jsonlExists {
// Auto-detect: prefer JSONL if database is corrupted or force is set
if force {
// Force mode implies database is broken - use JSONL
if !jsonlExists {
return fmt.Errorf("--force requires JSONL for recovery but no JSONL file found")
}
useJSONL = true
fmt.Println(" Using JSONL as source of truth (--force mode)")
} else if !dbExists && jsonlExists {
useJSONL = true
fmt.Println(" Using JSONL as source of truth (database missing)")
} else if dbExists && !jsonlExists {
@@ -144,12 +160,6 @@ func DatabaseCorruptionRecoveryWithOptions(path string, force bool, source strin
return fmt.Errorf("invalid --source value: %s (valid values: auto, jsonl, db)", source)
}
// If force mode is enabled, always use JSONL recovery (bypass database validation)
if force {
useJSONL = true
fmt.Println(" Force mode enabled: bypassing database validation, using JSONL recovery")
}
// If using database as source, just run migration (no recovery needed)
if !useJSONL {
fmt.Println(" Database is the source of truth - skipping recovery")