From 8fe48d09be5f04b677f5ea4c17bf36386f0fd518 Mon Sep 17 00:00:00 2001 From: Josh Nichols Date: Fri, 9 Jan 2026 16:40:52 -0500 Subject: [PATCH] fix(staleness): auto-import on stale DB for read-only commands (bd-9dao) (#982) When database is stale and a read-only command runs in --no-daemon mode, auto-import from JSONL instead of returning an error. This fixes `gt sling` failing with 'not a valid bead' after git pull. Root cause: `ensureDatabaseFresh` would return an error when DB was stale, but read-only commands like `bd show` should be able to auto-import and proceed rather than blocking the user. Changes: - Modify ensureDatabaseFresh to call autoImportIfNewer() when stale - Only error if --no-auto-import flag is explicitly set - Add comprehensive tests for all staleness scenarios Fixes: bd-9dao Co-authored-by: Claude --- cmd/bd/staleness.go | 10 +- cmd/bd/staleness_test.go | 373 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 382 insertions(+), 1 deletion(-) create mode 100644 cmd/bd/staleness_test.go diff --git a/cmd/bd/staleness.go b/cmd/bd/staleness.go index 480beaaa..67e9c94d 100644 --- a/cmd/bd/staleness.go +++ b/cmd/bd/staleness.go @@ -43,7 +43,15 @@ func ensureDatabaseFresh(ctx context.Context) error { return nil } - // Database is stale - refuse to operate + // Database is stale - auto-import to refresh (bd-9dao fix) + // For read-only commands running in --no-daemon mode, auto-import instead of + // returning an error. This allows commands like `bd show` to work after git pull. + if !noAutoImport { + autoImportIfNewer() + return nil + } + + // Auto-import is disabled, refuse to operate return fmt.Errorf( "Database out of sync with JSONL. Run 'bd sync --import-only' to fix.\n\n"+ "The JSONL file has been updated (e.g., after 'git pull') but the database\n"+ diff --git a/cmd/bd/staleness_test.go b/cmd/bd/staleness_test.go new file mode 100644 index 00000000..be670d57 --- /dev/null +++ b/cmd/bd/staleness_test.go @@ -0,0 +1,373 @@ +package main + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/steveyegge/beads/internal/storage/sqlite" + "github.com/steveyegge/beads/internal/types" +) + +// TestEnsureDatabaseFresh_AutoImportsOnStale verifies that when the database +// is stale (JSONL is newer), ensureDatabaseFresh triggers auto-import instead +// of returning an error. This is the fix for bd-9dao. +func TestEnsureDatabaseFresh_AutoImportsOnStale(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatalf("Failed to create beads dir: %v", err) + } + + testDBPath := filepath.Join(beadsDir, "bd.db") + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + + // Create database + testStore, err := sqlite.New(ctx, testDBPath) + if err != nil { + t.Fatalf("Failed to create store: %v", err) + } + defer testStore.Close() + + // Set prefix + if err := testStore.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Set an old last_import_time to make DB appear stale + oldTime := time.Now().Add(-1 * time.Hour) + if err := testStore.SetMetadata(ctx, "last_import_time", oldTime.Format(time.RFC3339Nano)); err != nil { + t.Fatalf("Failed to set metadata: %v", err) + } + + // Create JSONL with a new issue that should be auto-imported + jsonlIssue := &types.Issue{ + ID: "test-stale-auto-bd9dao", + Title: "Should Auto Import on Stale DB", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + f, err := os.Create(jsonlPath) + if err != nil { + t.Fatalf("Failed to create JSONL: %v", err) + } + encoder := json.NewEncoder(f) + if err := encoder.Encode(jsonlIssue); err != nil { + t.Fatalf("Failed to encode issue: %v", err) + } + f.Close() + + // Save and set global state + oldNoAutoImport := noAutoImport + oldAutoImportEnabled := autoImportEnabled + oldStore := store + oldDbPath := dbPath + oldRootCtx := rootCtx + oldStoreActive := storeActive + oldAllowStale := allowStale + + noAutoImport = false // Allow auto-import + autoImportEnabled = true // Enable auto-import + allowStale = false // Don't skip staleness check + store = testStore + dbPath = testDBPath + rootCtx = ctx + storeActive = true + + defer func() { + noAutoImport = oldNoAutoImport + autoImportEnabled = oldAutoImportEnabled + allowStale = oldAllowStale + store = oldStore + dbPath = oldDbPath + rootCtx = oldRootCtx + storeActive = oldStoreActive + }() + + // Call ensureDatabaseFresh - should auto-import and return nil + err = ensureDatabaseFresh(ctx) + if err != nil { + t.Errorf("ensureDatabaseFresh() returned error when it should have auto-imported: %v", err) + } + + // Verify issue was auto-imported + imported, err := testStore.GetIssue(ctx, "test-stale-auto-bd9dao") + if err != nil { + t.Fatalf("Failed to check for issue: %v", err) + } + if imported == nil { + t.Error("ensureDatabaseFresh() did not auto-import when DB was stale - bd-9dao fix failed") + } +} + +// TestEnsureDatabaseFresh_NoAutoImportFlag verifies that when noAutoImport is +// true, ensureDatabaseFresh returns an error instead of auto-importing. +func TestEnsureDatabaseFresh_NoAutoImportFlag(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatalf("Failed to create beads dir: %v", err) + } + + testDBPath := filepath.Join(beadsDir, "bd.db") + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + + // Create database + testStore, err := sqlite.New(ctx, testDBPath) + if err != nil { + t.Fatalf("Failed to create store: %v", err) + } + defer testStore.Close() + + // Set prefix + if err := testStore.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Set an old last_import_time to make DB appear stale + oldTime := time.Now().Add(-1 * time.Hour) + if err := testStore.SetMetadata(ctx, "last_import_time", oldTime.Format(time.RFC3339Nano)); err != nil { + t.Fatalf("Failed to set metadata: %v", err) + } + + // Create JSONL with a new issue + jsonlIssue := &types.Issue{ + ID: "test-noauto-bd9dao", + Title: "Should NOT Auto Import", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + f, err := os.Create(jsonlPath) + if err != nil { + t.Fatalf("Failed to create JSONL: %v", err) + } + encoder := json.NewEncoder(f) + if err := encoder.Encode(jsonlIssue); err != nil { + t.Fatalf("Failed to encode issue: %v", err) + } + f.Close() + + // Save and set global state + oldNoAutoImport := noAutoImport + oldAutoImportEnabled := autoImportEnabled + oldStore := store + oldDbPath := dbPath + oldRootCtx := rootCtx + oldStoreActive := storeActive + oldAllowStale := allowStale + + noAutoImport = true // Disable auto-import + autoImportEnabled = false // Disable auto-import + allowStale = false // Don't skip staleness check + store = testStore + dbPath = testDBPath + rootCtx = ctx + storeActive = true + + defer func() { + noAutoImport = oldNoAutoImport + autoImportEnabled = oldAutoImportEnabled + allowStale = oldAllowStale + store = oldStore + dbPath = oldDbPath + rootCtx = oldRootCtx + storeActive = oldStoreActive + }() + + // Call ensureDatabaseFresh - should return error since noAutoImport is set + err = ensureDatabaseFresh(ctx) + if err == nil { + t.Error("ensureDatabaseFresh() should have returned error when noAutoImport is true") + } + + // Verify issue was NOT imported + imported, err := testStore.GetIssue(ctx, "test-noauto-bd9dao") + if err != nil { + t.Fatalf("Failed to check for issue: %v", err) + } + if imported != nil { + t.Error("ensureDatabaseFresh() imported despite noAutoImport=true") + } +} + +// TestEnsureDatabaseFresh_AllowStaleFlag verifies that when allowStale is +// true, ensureDatabaseFresh skips the check entirely. +func TestEnsureDatabaseFresh_AllowStaleFlag(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatalf("Failed to create beads dir: %v", err) + } + + testDBPath := filepath.Join(beadsDir, "bd.db") + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + + // Create database + testStore, err := sqlite.New(ctx, testDBPath) + if err != nil { + t.Fatalf("Failed to create store: %v", err) + } + defer testStore.Close() + + // Set prefix + if err := testStore.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Set an old last_import_time to make DB appear stale + oldTime := time.Now().Add(-1 * time.Hour) + if err := testStore.SetMetadata(ctx, "last_import_time", oldTime.Format(time.RFC3339Nano)); err != nil { + t.Fatalf("Failed to set metadata: %v", err) + } + + // Create JSONL with a new issue + jsonlIssue := &types.Issue{ + ID: "test-allowstale-bd9dao", + Title: "Should Skip Check", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + f, err := os.Create(jsonlPath) + if err != nil { + t.Fatalf("Failed to create JSONL: %v", err) + } + encoder := json.NewEncoder(f) + if err := encoder.Encode(jsonlIssue); err != nil { + t.Fatalf("Failed to encode issue: %v", err) + } + f.Close() + + // Save and set global state + oldNoAutoImport := noAutoImport + oldAutoImportEnabled := autoImportEnabled + oldStore := store + oldDbPath := dbPath + oldRootCtx := rootCtx + oldStoreActive := storeActive + oldAllowStale := allowStale + + noAutoImport = true // Disable auto-import (shouldn't matter with allowStale) + autoImportEnabled = false + allowStale = true // Skip staleness check entirely + store = testStore + dbPath = testDBPath + rootCtx = ctx + storeActive = true + + defer func() { + noAutoImport = oldNoAutoImport + autoImportEnabled = oldAutoImportEnabled + allowStale = oldAllowStale + store = oldStore + dbPath = oldDbPath + rootCtx = oldRootCtx + storeActive = oldStoreActive + }() + + // Call ensureDatabaseFresh - should return nil (skip check) + err = ensureDatabaseFresh(ctx) + if err != nil { + t.Errorf("ensureDatabaseFresh() should have returned nil with allowStale=true: %v", err) + } + + // Verify issue was NOT imported (check was skipped entirely) + imported, err := testStore.GetIssue(ctx, "test-allowstale-bd9dao") + if err != nil { + t.Fatalf("Failed to check for issue: %v", err) + } + if imported != nil { + t.Error("ensureDatabaseFresh() imported even though allowStale=true (should skip check entirely)") + } +} + +// TestEnsureDatabaseFresh_FreshDB verifies that when the database is fresh, +// ensureDatabaseFresh returns nil without doing anything. +func TestEnsureDatabaseFresh_FreshDB(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatalf("Failed to create beads dir: %v", err) + } + + testDBPath := filepath.Join(beadsDir, "bd.db") + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + + // Create JSONL first + jsonlIssue := &types.Issue{ + ID: "test-fresh-bd9dao", + Title: "Fresh DB Test", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + } + f, err := os.Create(jsonlPath) + if err != nil { + t.Fatalf("Failed to create JSONL: %v", err) + } + encoder := json.NewEncoder(f) + if err := encoder.Encode(jsonlIssue); err != nil { + t.Fatalf("Failed to encode issue: %v", err) + } + f.Close() + + // Create database + testStore, err := sqlite.New(ctx, testDBPath) + if err != nil { + t.Fatalf("Failed to create store: %v", err) + } + defer testStore.Close() + + // Set prefix + if err := testStore.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("Failed to set prefix: %v", err) + } + + // Set a future last_import_time to make DB appear fresh + futureTime := time.Now().Add(1 * time.Hour) + if err := testStore.SetMetadata(ctx, "last_import_time", futureTime.Format(time.RFC3339Nano)); err != nil { + t.Fatalf("Failed to set metadata: %v", err) + } + + // Save and set global state + oldNoAutoImport := noAutoImport + oldAutoImportEnabled := autoImportEnabled + oldStore := store + oldDbPath := dbPath + oldRootCtx := rootCtx + oldStoreActive := storeActive + oldAllowStale := allowStale + + noAutoImport = false + autoImportEnabled = true + allowStale = false + store = testStore + dbPath = testDBPath + rootCtx = ctx + storeActive = true + + defer func() { + noAutoImport = oldNoAutoImport + autoImportEnabled = oldAutoImportEnabled + allowStale = oldAllowStale + store = oldStore + dbPath = oldDbPath + rootCtx = oldRootCtx + storeActive = oldStoreActive + }() + + // Call ensureDatabaseFresh - should return nil (DB is fresh) + err = ensureDatabaseFresh(ctx) + if err != nil { + t.Errorf("ensureDatabaseFresh() should have returned nil for fresh DB: %v", err) + } +}