diff --git a/.beads/bd.jsonl b/.beads/bd.jsonl index ed8fb196..459e3dc4 100644 --- a/.beads/bd.jsonl +++ b/.beads/bd.jsonl @@ -72,7 +72,10 @@ {"id":"bd-163","title":"Add .beads/config.json for database path configuration","description":"Create config file to eliminate ambiguity about which database is active.\n\nConfig file format (.beads/config.json):\n{\n \"database\": \"beads.db\",\n \"version\": \"0.17.5\",\n \"jsonl_export\": \"beads.jsonl\" // Allow user to rename\n}\n\nImplementation:\n- bd init creates config.json\n- Daemon and clients read config first (single source of truth)\n- Fall back to beads.db if config missing (backward compat)\n- bd init --jsonl-name allows customizing export filename\n- Gitignore: do NOT ignore config.json (part of repo state)\n\nBenefits:\n- Explicit configuration over convention\n- Allows JSONL renaming for git history hygiene\n- Single source of truth for file paths","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-26T18:06:07.571909-07:00","updated_at":"2025-10-26T18:44:16.133085-07:00","closed_at":"2025-10-26T18:44:16.133085-07:00","dependencies":[{"issue_id":"bd-163","depends_on_id":"bd-159","type":"parent-child","created_at":"2025-10-26T18:06:07.572636-07:00","created_by":"daemon"}]} {"id":"bd-164","title":"Add migration tooling for database upgrades","description":"Create bd migrate command and auto-migration logic for version upgrades.\n\nImplementation:\n- bd migrate command (or bd init --migrate)\n- Auto-run on first command after daemon version upgrade\n- Detection logic:\n - Find all .db files in .beads/\n - Check schema version in each\n - Prompt to migrate/rename/delete\n- Migration operations:\n - Rename old database to beads.db\n - Update schema version metadata\n - Remove stale databases (with confirmation)\n- Could be part of daemon auto-start logic\n\nUser experience:\n$ bd ready\nDatabase schema mismatch detected.\n Found: vc.db (schema v0.16.0)\n Expected: beads.db (schema v0.17.5)\n \nRun 'bd migrate' to migrate automatically.\n\nBenefits:\n- Smooth upgrade path\n- Prevents confusion on version changes\n- Clean up stale databases\n\nDepends on:\n- Canonical naming (bd-160)\n- Schema versioning (bd-161)","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-26T18:06:07.571855-07:00","updated_at":"2025-10-26T19:04:02.023089-07:00","closed_at":"2025-10-26T19:04:02.023089-07:00","dependencies":[{"issue_id":"bd-164","depends_on_id":"bd-159","type":"parent-child","created_at":"2025-10-26T18:06:07.573546-07:00","created_by":"daemon"},{"issue_id":"bd-164","depends_on_id":"bd-162","type":"blocks","created_at":"2025-10-26T18:06:17.327717-07:00","created_by":"daemon"},{"issue_id":"bd-164","depends_on_id":"bd-160","type":"blocks","created_at":"2025-10-26T18:06:17.351768-07:00","created_by":"daemon"}]} {"id":"bd-165","title":"Enforce canonical database name (beads.db)","description":"Always use beads.db as the canonical database name. Never auto-detect from multiple .db files.\n\nImplementation:\n- bd init always creates/uses beads.db\n- bd init detects and migrates old databases (vc.db → beads.db, bd.db → beads.db)\n- Daemon refuses to start if multiple .db files exist in .beads/ (exit with ambiguity error)\n- Update database discovery logic to prefer beads.db, error on ambiguity\n\nBenefits:\n- Prevents accidental use of stale databases\n- Clear single source of truth\n- Migration path for existing users","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-26T18:06:18.33827-07:00","updated_at":"2025-10-26T18:10:34.132537-07:00","closed_at":"2025-10-26T18:10:34.132537-07:00","dependencies":[{"issue_id":"bd-165","depends_on_id":"bd-159","type":"parent-child","created_at":"2025-10-26T18:06:18.339465-07:00","created_by":"daemon"}]} -{"id":"bd-166","title":"bd import/sync created 173 duplicate issues with wrong prefix","description":"## What Happened\nDuring corruption recovery investigation (beads-173), discovered the database contained 338 issues instead of expected 165:\n- 165 issues with correct `bd-` prefix \n- 173 duplicate issues with `beads-` prefix (bd-1 → beads-1, etc.)\n- Database config was set to `beads` prefix instead of `bd`\n\n## Root Cause\nSome bd operation (likely import or sync) created duplicate issues with the wrong prefix. The database should have rejected or warned about prefix mismatch, but instead:\n1. Silently created duplicates with wrong prefix\n2. Changed database prefix config from `bd` to `beads`\n\n## Impact\n- **Data integrity violation**: Duplicate issues with different IDs\n- **Silent corruption**: No error or warning during creation \n- **Wrong prefix**: Database config changed without user consent\n- **Confusion**: Users see double the issues, dependencies broken\n\n## Recovery \nHad to manually fix the `issue_prefix` config key (not `prefix` as initially thought):\n```bash\nsqlite3 .beads/beads.db \"UPDATE config SET value = 'bd' WHERE key = 'issue_prefix';\"\nsqlite3 .beads/beads.db \"DELETE FROM issues WHERE id LIKE 'beads-%';\"\n```\n\n## What Should Happen\n1. **Reject prefix mismatch**: If importing issues with different prefix than configured, error or require `--rename-on-import`\n2. **Never auto-change prefix**: Database prefix should only change via explicit `bd rename-prefix` command \n3. **Validate on import**: Check imported issue IDs match configured prefix before creating\n4. **Warn on duplicate IDs**: Even with different prefixes, detect potential duplicates\n\n## Related\n- Discovered during beads-173 (database corruption investigation)\n- Similar to existing prefix validation in `bd sync` (bd-21)","notes":"## Root Cause Analysis\n\nFound the smoking gun! The bug is a combination of three factors:\n\n**1. Database filename-based prefix fallback (sqlite.go:577-589)**\n```go\nfunc derivePrefixFromPath(dbPath string) string {\n dbFileName := filepath.Base(dbPath)\n dbFileName = strings.TrimSuffix(dbFileName, \".db\")\n prefix := strings.TrimSuffix(dbFileName, \"-\")\n if prefix == \"\" {\n prefix = \"bd\"\n }\n return prefix\n}\n```\nWhen `issue_prefix` config is missing, CreateIssue uses database filename (beads.db → \"beads\" prefix).\n\n**2. Auto-import skips prefix validation (autoimport.go:175)**\n```go\nopts := ImportOptions{\n ResolveCollisions: true,\n SkipPrefixValidation: true, // Auto-import is lenient about prefixes\n}\n```\nThis allows importing bd- issues without error even when config says \"beads\".\n\n**3. Missing issue_prefix config**\nSomehow the database lost its `issue_prefix = 'bd'` config (should be set by `bd init --prefix bd`), triggering the fallback.\n\n**Timeline:**\n1. Database exists with bd- issues\n2. `issue_prefix` config gets lost (corruption? manual deletion? reinit?)\n3. Auto-import brings in bd- issues from git (SkipPrefixValidation allows this)\n4. New issue creation falls back to filename → uses \"beads\" prefix\n5. Result: 165 bd- issues + 173 beads- duplicates\n\n**Fixes needed:**\n1. Never derive prefix from filename - always require explicit config\n2. Auto-import should SET issue_prefix if missing (from first imported issue)\n3. Add validation that rejects operations if issue_prefix is missing\n4. Warn loudly if prefix fallback is triggered","status":"open","priority":0,"issue_type":"bug","created_at":"2025-10-26T21:38:39.096165-07:00","updated_at":"2025-10-26T21:42:08.17707-07:00"} +{"id":"bd-166","title":"bd import/sync created 173 duplicate issues with wrong prefix","description":"## What Happened\nDuring corruption recovery investigation (beads-173), discovered the database contained 338 issues instead of expected 165:\n- 165 issues with correct `bd-` prefix \n- 173 duplicate issues with `beads-` prefix (bd-1 → beads-1, etc.)\n- Database config was set to `beads` prefix instead of `bd`\n\n## Root Cause\nSome bd operation (likely import or sync) created duplicate issues with the wrong prefix. The database should have rejected or warned about prefix mismatch, but instead:\n1. Silently created duplicates with wrong prefix\n2. Changed database prefix config from `bd` to `beads`\n\n## Impact\n- **Data integrity violation**: Duplicate issues with different IDs\n- **Silent corruption**: No error or warning during creation \n- **Wrong prefix**: Database config changed without user consent\n- **Confusion**: Users see double the issues, dependencies broken\n\n## Recovery \nHad to manually fix the `issue_prefix` config key (not `prefix` as initially thought):\n```bash\nsqlite3 .beads/beads.db \"UPDATE config SET value = 'bd' WHERE key = 'issue_prefix';\"\nsqlite3 .beads/beads.db \"DELETE FROM issues WHERE id LIKE 'beads-%';\"\n```\n\n## What Should Happen\n1. **Reject prefix mismatch**: If importing issues with different prefix than configured, error or require `--rename-on-import`\n2. **Never auto-change prefix**: Database prefix should only change via explicit `bd rename-prefix` command \n3. **Validate on import**: Check imported issue IDs match configured prefix before creating\n4. **Warn on duplicate IDs**: Even with different prefixes, detect potential duplicates\n\n## Related\n- Discovered during beads-173 (database corruption investigation)\n- Similar to existing prefix validation in `bd sync` (bd-21)","notes":"## Root Cause Analysis\n\nFound the smoking gun! The bug is a combination of three factors:\n\n**1. Database filename-based prefix fallback (sqlite.go:577-589)**\n```go\nfunc derivePrefixFromPath(dbPath string) string {\n dbFileName := filepath.Base(dbPath)\n dbFileName = strings.TrimSuffix(dbFileName, \".db\")\n prefix := strings.TrimSuffix(dbFileName, \"-\")\n if prefix == \"\" {\n prefix = \"bd\"\n }\n return prefix\n}\n```\nWhen `issue_prefix` config is missing, CreateIssue uses database filename (beads.db → \"beads\" prefix).\n\n**2. Auto-import skips prefix validation (autoimport.go:175)**\n```go\nopts := ImportOptions{\n ResolveCollisions: true,\n SkipPrefixValidation: true, // Auto-import is lenient about prefixes\n}\n```\nThis allows importing bd- issues without error even when config says \"beads\".\n\n**3. Missing issue_prefix config**\nSomehow the database lost its `issue_prefix = 'bd'` config (should be set by `bd init --prefix bd`), triggering the fallback.\n\n**Timeline:**\n1. Database exists with bd- issues\n2. `issue_prefix` config gets lost (corruption? manual deletion? reinit?)\n3. Auto-import brings in bd- issues from git (SkipPrefixValidation allows this)\n4. New issue creation falls back to filename → uses \"beads\" prefix\n5. Result: 165 bd- issues + 173 beads- duplicates\n\n**Fixes implemented:**\n1. ✅ Removed derivePrefixFromPath - never derive prefix from filename\n2. ✅ CreateIssue now REJECTS operations if issue_prefix is missing (hard error)\n3. ✅ Auto-import now SETS issue_prefix from first imported issue if missing\n4. ✅ Updated all test helpers to set issue_prefix during database initialization\n\nThis prevents silent corruption - if issue_prefix is missing, operations fail loudly instead of creating duplicate issues.","status":"closed","priority":0,"issue_type":"bug","created_at":"2025-10-26T21:38:39.096165-07:00","updated_at":"2025-10-26T21:51:44.171474-07:00","closed_at":"2025-10-26T21:51:44.171474-07:00"} +{"id":"bd-167","title":"Complete test fixes for bd-166 (missing issue_prefix)","description":"Several test files still fail with \"database not initialized: issue_prefix config is missing\" error:\n\n- cmd/bd/comments_test.go\n- cmd/bd/export_test.go \n- cmd/bd/git_sync_test.go\n- cmd/bd/label_test.go\n- cmd/bd/list_test.go\n- cmd/bd/reopen_test.go\n- cmd/bd/direct_mode_test.go\n\nAll need to either:\n1. Use the new `newTestStore(t, dbPath)` helper, or\n2. Call `store.SetConfig(ctx, \"issue_prefix\", \"bd\")` after `sqlite.New()`\n\nPattern to fix:\n```go\n// Old:\nstore, err := sqlite.New(dbPath)\n\n// New:\nstore := newTestStore(t, dbPath)\n```","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-26T21:54:36.625952-07:00","updated_at":"2025-10-26T21:54:36.625952-07:00","dependencies":[{"issue_id":"bd-167","depends_on_id":"bd-166","type":"discovered-from","created_at":"2025-10-26T21:54:41.993277-07:00","created_by":"daemon"}]} +{"id":"bd-168","title":"Add bd doctor command to detect/fix missing issue_prefix","description":"Create a `bd doctor` command that validates database health and offers to fix common issues.\n\n**Phase 1: Detect missing issue_prefix**\n```bash\nbd doctor\n# Output:\n# ✗ issue_prefix not configured\n# Database has 165 issues with prefix \"bd\"\n# Run: bd doctor --fix-prefix bd\n```\n\n**Phase 2: Auto-fix**\n```bash\nbd doctor --fix-prefix bd\n# Sets issue_prefix config to \"bd\"\n# Or derive from existing issues if --fix-prefix not specified\n```\n\n**Phase 3: Other checks (future)**\n- Orphaned dependencies\n- Counter mismatches \n- Corrupted timestamps\n- Duplicate issues with different prefixes\n\nThis helps users recover from bd-166 corruption without manual SQL.","status":"open","priority":2,"issue_type":"feature","created_at":"2025-10-26T21:54:36.634246-07:00","updated_at":"2025-10-26T21:54:36.634246-07:00","dependencies":[{"issue_id":"bd-168","depends_on_id":"bd-166","type":"discovered-from","created_at":"2025-10-26T21:54:41.994194-07:00","created_by":"daemon"}]} +{"id":"bd-169","title":"Add test for CreateIssue with missing issue_prefix","description":"Add explicit test case that verifies CreateIssue fails correctly when issue_prefix config is missing.\n\n**Test:**\n```go\nfunc TestCreateIssue_MissingPrefix(t *testing.T) {\n store, cleanup := setupTestDB(t)\n defer cleanup()\n \n ctx := context.Background()\n \n // Clear the issue_prefix config\n err := store.SetConfig(ctx, \"issue_prefix\", \"\")\n require.NoError(t, err)\n \n // Attempt to create issue should fail\n issue := \u0026types.Issue{\n Title: \"Test issue\",\n Status: types.StatusOpen,\n Priority: 1,\n IssueType: types.TypeTask,\n }\n \n err = store.CreateIssue(ctx, issue, \"test\")\n require.Error(t, err)\n assert.Contains(t, err.Error(), \"database not initialized\")\n assert.Contains(t, err.Error(), \"issue_prefix config is missing\")\n}\n```\n\nThis ensures the fix for bd-166 doesn't regress.","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-26T21:54:36.63521-07:00","updated_at":"2025-10-26T21:54:36.63521-07:00","dependencies":[{"issue_id":"bd-169","depends_on_id":"bd-166","type":"discovered-from","created_at":"2025-10-26T21:54:41.995525-07:00","created_by":"daemon"}]} {"id":"bd-17","title":"Update EXTENDING.md with UnderlyingDB() usage and best practices","description":"EXTENDING.md currently shows how to use direct sql.Open() to access the database, but doesn't mention the new UnderlyingDB() method that's the recommended way for extensions.\n\n**Update needed:**\n1. Add section showing UnderlyingDB() usage:\n ```go\n store, err := beads.NewSQLiteStorage(dbPath)\n db := store.UnderlyingDB()\n // Create extension tables using db\n ```\n\n2. Document when to use UnderlyingDB() vs direct sql.Open():\n - Use UnderlyingDB() when you want to share the storage connection\n - Use sql.Open() when you need independent connection management\n\n3. Add safety warnings (cross-reference from UnderlyingDB() docs):\n - Don't close the DB\n - Don't modify pool settings\n - Keep transactions short\n\n4. Update the VC example to show UnderlyingDB() pattern\n\n5. Explain beads.Storage.UnderlyingDB() in the API section","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-22T17:07:56.820056-07:00","updated_at":"2025-10-25T23:15:33.478579-07:00","closed_at":"2025-10-22T19:41:19.895847-07:00","dependencies":[{"issue_id":"bd-17","depends_on_id":"bd-10","type":"discovered-from","created_at":"2025-10-24T13:17:40.32522-07:00","created_by":"renumber"}]} {"id":"bd-18","title":"Consider adding UnderlyingConn(ctx) for safer scoped DB access","description":"Currently UnderlyingDB() returns *sql.DB which is correct for most uses, but for extension migrations/DDL, a scoped connection might be safer.\n\n**Proposal:** Add optional UnderlyingConn(ctx) (*sql.Conn, error) method that:\n- Returns a scoped connection via s.db.Conn(ctx)\n- Encourages lifetime-bounded usage\n- Reduces temptation to tune global pool settings\n- Better for one-time DDL operations like CREATE TABLE\n\n**Implementation:**\n```go\n// UnderlyingConn returns a single connection from the pool for scoped use\n// Useful for migrations and DDL. Close the connection when done.\nfunc (s *SQLiteStorage) UnderlyingConn(ctx context.Context) (*sql.Conn, error) {\n return s.db.Conn(ctx)\n}\n```\n\n**Benefits:**\n- Safer for migrations (explicit scope)\n- Complements UnderlyingDB() for different use cases\n- Low implementation cost\n\n**Trade-off:** Adds another method to maintain, but Oracle considers this balanced compromise between safety and flexibility.\n\n**Decision:** This is optional - evaluate based on VC's actual usage patterns.","status":"closed","priority":3,"issue_type":"feature","created_at":"2025-10-22T17:07:56.832638-07:00","updated_at":"2025-10-25T23:15:33.479496-07:00","closed_at":"2025-10-22T22:02:18.479512-07:00","dependencies":[{"issue_id":"bd-18","depends_on_id":"bd-10","type":"related","created_at":"2025-10-24T13:17:40.325463-07:00","created_by":"renumber"}]} {"id":"bd-19","title":"MCP close tool method signature error - takes 1 positional argument but 2 were given","description":"The close approval routing fix in beads-mcp v0.11.0 works correctly and successfully routes update(status=\"closed\") calls to close() tool. However, the close() tool has a Python method signature bug that prevents execution.\n\nImpact: All MCP-based close operations are broken. Workaround: Use bd CLI directly.\n\nError: BdDaemonClient.close() takes 1 positional argument but 2 were given\n\nRoot cause: BdDaemonClient.close() only accepts self, but MCP tool passes issue_id and reason.\n\nAdditional issue: CLI close has FOREIGN KEY constraint error when recording reason parameter.\n\nSee GitHub issue #107 for full details.","status":"closed","priority":0,"issue_type":"bug","created_at":"2025-10-22T17:25:34.67056-07:00","updated_at":"2025-10-25T23:15:33.480292-07:00","closed_at":"2025-10-22T17:36:55.463445-07:00"} diff --git a/.beads/beads.db.corrupt-backup b/.beads/beads.db.corrupt-backup new file mode 100644 index 00000000..eb8eb9ef Binary files /dev/null and b/.beads/beads.db.corrupt-backup differ diff --git a/beads_integration_test.go b/beads_integration_test.go index 15084f39..cad821b1 100644 --- a/beads_integration_test.go +++ b/beads_integration_test.go @@ -167,6 +167,12 @@ func TestLibraryIntegration(t *testing.T) { } defer store.Close() + // CRITICAL (bd-166): Set issue_prefix to prevent "database not initialized" errors + ctx := context.Background() + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set issue_prefix: %v", err) + } + h := newIntegrationHelper(t, store) // Test 1: Create issue @@ -326,6 +332,11 @@ func TestBatchCreateIssues(t *testing.T) { ctx := context.Background() + // CRITICAL (bd-166): Set issue_prefix to prevent "database not initialized" errors + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set issue_prefix: %v", err) + } + // Create multiple issues issues := make([]*beads.Issue, 5) for i := 0; i < 5; i++ { @@ -399,6 +410,12 @@ func TestRoundTripIssue(t *testing.T) { } defer store.Close() + // CRITICAL (bd-166): Set issue_prefix to prevent "database not initialized" errors + ctx := context.Background() + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("Failed to set issue_prefix: %v", err) + } + h := newIntegrationHelper(t, store) original := h.createFullIssue("Full description", "Design notes", "Acceptance criteria", "Implementation notes", "developer") diff --git a/cmd/bd/autoimport.go b/cmd/bd/autoimport.go index 6fde28cf..ffe9e643 100644 --- a/cmd/bd/autoimport.go +++ b/cmd/bd/autoimport.go @@ -9,6 +9,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "github.com/steveyegge/beads/internal/storage" "github.com/steveyegge/beads/internal/types" @@ -167,7 +168,24 @@ func importFromGit(ctx context.Context, dbFilePath string, store storage.Storage return fmt.Errorf("failed to scan JSONL: %w", err) } + // CRITICAL (bd-166): Set issue_prefix from first imported issue if missing + // This prevents derivePrefixFromPath fallback which caused duplicate issues + if len(issues) > 0 { + configuredPrefix, err := store.GetConfig(ctx, "issue_prefix") + if err == nil && strings.TrimSpace(configuredPrefix) == "" { + // Database has no prefix configured - derive from first issue + firstPrefix := extractPrefix(issues[0].ID) + if firstPrefix != "" { + if err := store.SetConfig(ctx, "issue_prefix", firstPrefix); err != nil { + return fmt.Errorf("failed to set issue_prefix from imported issues: %w", err) + } + } + } + } + // Use existing import logic with auto-resolve collisions + // Note: SkipPrefixValidation allows mixed prefixes during auto-import + // (but now we set the prefix first, so CreateIssue won't use filename fallback) opts := ImportOptions{ ResolveCollisions: true, DryRun: false, diff --git a/cmd/bd/config_test.go b/cmd/bd/config_test.go index 815475d2..a3035c6c 100644 --- a/cmd/bd/config_test.go +++ b/cmd/bd/config_test.go @@ -162,6 +162,14 @@ func setupTestDB(t *testing.T) (*sqlite.SQLiteStorage, func()) { t.Fatalf("Failed to create test database: %v", err) } + // CRITICAL (bd-166): Set issue_prefix to prevent "database not initialized" errors + ctx := context.Background() + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + store.Close() + os.RemoveAll(tmpDir) + t.Fatalf("Failed to set issue_prefix: %v", err) + } + cleanup := func() { store.Close() os.RemoveAll(tmpDir) diff --git a/cmd/bd/daemon_test.go b/cmd/bd/daemon_test.go index 8b3e48c3..4395c539 100644 --- a/cmd/bd/daemon_test.go +++ b/cmd/bd/daemon_test.go @@ -161,10 +161,7 @@ func TestDaemonIntegration(t *testing.T) { } testDBPath := filepath.Join(dbDir, "test.db") - testStore, err := sqlite.New(testDBPath) - if err != nil { - t.Fatalf("Failed to create test database: %v", err) - } + testStore := newTestStore(t, testDBPath) oldStore := store oldDBPath := dbPath @@ -305,10 +302,7 @@ func TestDaemonRPCServerIntegration(t *testing.T) { } testDBPath := filepath.Join(dbDir, "test.db") - testStore, err := sqlite.New(testDBPath) - if err != nil { - t.Fatalf("Failed to create test database: %v", err) - } + testStore := newTestStore(t, testDBPath) defer testStore.Close() ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) @@ -342,10 +336,7 @@ func TestDaemonConcurrentOperations(t *testing.T) { } testDBPath := filepath.Join(dbDir, "test.db") - testStore, err := sqlite.New(testDBPath) - if err != nil { - t.Fatalf("Failed to create test database: %v", err) - } + testStore := newTestStore(t, testDBPath) defer testStore.Close() ctx := context.Background() @@ -410,10 +401,7 @@ func TestDaemonSocketCleanupOnShutdown(t *testing.T) { socketPath := filepath.Join(tmpDir, "test.sock") testDBPath := filepath.Join(tmpDir, "test.db") - testStore, err := sqlite.New(testDBPath) - if err != nil { - t.Fatalf("Failed to create test database: %v", err) - } + testStore := newTestStore(t, testDBPath) server := newMockDaemonServer(socketPath, testStore) @@ -543,10 +531,7 @@ func TestDaemonGracefulShutdown(t *testing.T) { socketPath := filepath.Join(tmpDir, "test.sock") testDBPath := filepath.Join(tmpDir, "test.db") - testStore, err := sqlite.New(testDBPath) - if err != nil { - t.Fatalf("Failed to create test database: %v", err) - } + testStore := newTestStore(t, testDBPath) server := newMockDaemonServer(socketPath, testStore) diff --git a/cmd/bd/test_helpers_test.go b/cmd/bd/test_helpers_test.go new file mode 100644 index 00000000..d9a0f1e6 --- /dev/null +++ b/cmd/bd/test_helpers_test.go @@ -0,0 +1,28 @@ +package main + +import ( + "context" + "testing" + + "github.com/steveyegge/beads/internal/storage/sqlite" +) + +// newTestStore creates a SQLite store with issue_prefix configured (bd-166) +// This prevents "database not initialized" errors in tests +func newTestStore(t *testing.T, dbPath string) *sqlite.SQLiteStorage { + t.Helper() + + store, err := sqlite.New(dbPath) + if err != nil { + t.Fatalf("Failed to create test database: %v", err) + } + + // CRITICAL (bd-166): Set issue_prefix to prevent "database not initialized" errors + ctx := context.Background() + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + store.Close() + t.Fatalf("Failed to set issue_prefix: %v", err) + } + + return store +} diff --git a/internal/compact/compactor_test.go b/internal/compact/compactor_test.go index 40ee629a..b3c1f502 100644 --- a/internal/compact/compactor_test.go +++ b/internal/compact/compactor_test.go @@ -20,6 +20,10 @@ func setupTestStorage(t *testing.T) *sqlite.SQLiteStorage { } ctx := context.Background() + // CRITICAL (bd-166): Set issue_prefix to prevent "database not initialized" errors + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("failed to set issue_prefix: %v", err) + } if err := store.SetConfig(ctx, "compact_tier1_days", "0"); err != nil { t.Fatalf("failed to set config: %v", err) } diff --git a/internal/rpc/rpc_test.go b/internal/rpc/rpc_test.go index d4865196..8a94a858 100644 --- a/internal/rpc/rpc_test.go +++ b/internal/rpc/rpc_test.go @@ -38,6 +38,14 @@ func setupTestServer(t *testing.T) (*Server, *Client, func()) { t.Fatalf("Failed to create store: %v", err) } + // CRITICAL (bd-166): Set issue_prefix to prevent "database not initialized" errors + ctx := context.Background() + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + store.Close() + os.RemoveAll(tmpDir) + t.Fatalf("Failed to set issue_prefix: %v", err) + } + server := NewServer(socketPath, store, tmpDir, dbPath) ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/storage/sqlite/sqlite.go b/internal/storage/sqlite/sqlite.go index f85ec509..eafa6089 100644 --- a/internal/storage/sqlite/sqlite.go +++ b/internal/storage/sqlite/sqlite.go @@ -572,21 +572,9 @@ func (s *SQLiteStorage) SyncAllCounters(ctx context.Context) error { return nil } -// derivePrefixFromPath derives the issue prefix from the database file path -// Database file is named like ".beads/wy-.db" -> prefix should be "wy" -func derivePrefixFromPath(dbPath string) string { - dbFileName := filepath.Base(dbPath) - // Strip ".db" extension - dbFileName = strings.TrimSuffix(dbFileName, ".db") - // Strip trailing hyphen (if any) - prefix := strings.TrimSuffix(dbFileName, "-") - - // Fallback if filename is weird - if prefix == "" { - prefix = "bd" - } - return prefix -} +// REMOVED (bd-166): derivePrefixFromPath was causing duplicate issues with wrong prefix +// The database should ALWAYS have issue_prefix config set explicitly (by 'bd init' or auto-import) +// Never derive prefix from filename - it leads to silent data corruption // CreateIssue creates a new issue func (s *SQLiteStorage) CreateIssue(ctx context.Context, issue *types.Issue, actor string) error { @@ -635,8 +623,9 @@ func (s *SQLiteStorage) CreateIssue(ctx context.Context, issue *types.Issue, act var prefix string err := conn.QueryRowContext(ctx, `SELECT value FROM config WHERE key = ?`, "issue_prefix").Scan(&prefix) if err == sql.ErrNoRows || prefix == "" { - // Config not set - derive prefix from database filename - prefix = derivePrefixFromPath(s.dbPath) + // CRITICAL: Reject operation if issue_prefix config is missing (bd-166) + // This prevents duplicate issues with wrong prefix + return fmt.Errorf("database not initialized: issue_prefix config is missing (run 'bd init --prefix ' first)") } else if err != nil { return fmt.Errorf("failed to get config: %w", err) } @@ -770,8 +759,8 @@ func generateBatchIDs(ctx context.Context, conn *sql.Conn, issues []*types.Issue var prefix string err := conn.QueryRowContext(ctx, `SELECT value FROM config WHERE key = ?`, "issue_prefix").Scan(&prefix) if err == sql.ErrNoRows || prefix == "" { - // Config not set - derive prefix from database filename - prefix = derivePrefixFromPath(dbPath) + // CRITICAL: Reject operation if issue_prefix config is missing (bd-166) + return fmt.Errorf("database not initialized: issue_prefix config is missing (run 'bd init --prefix ' first)") } else if err != nil { return fmt.Errorf("failed to get config: %w", err) } diff --git a/internal/storage/sqlite/sqlite_test.go b/internal/storage/sqlite/sqlite_test.go index 876a00cf..701b3d1b 100644 --- a/internal/storage/sqlite/sqlite_test.go +++ b/internal/storage/sqlite/sqlite_test.go @@ -27,6 +27,14 @@ func setupTestDB(t *testing.T) (*SQLiteStorage, func()) { t.Fatalf("failed to create storage: %v", err) } + // CRITICAL (bd-166): Set issue_prefix to prevent "database not initialized" errors + ctx := context.Background() + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + store.Close() + os.RemoveAll(tmpDir) + t.Fatalf("failed to set issue_prefix: %v", err) + } + cleanup := func() { store.Close() os.RemoveAll(tmpDir)