From 1184bd1e5983de974d88597cc89c8f1ff358bbf8 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Thu, 25 Dec 2025 21:35:44 -0400 Subject: [PATCH 01/13] doctor: add git hygiene checks and DB integrity auto-fix Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- cmd/bd/doctor.go | 16 +- cmd/bd/doctor/database.go | 4 +- cmd/bd/doctor/fix/database_integrity.go | 134 +++++++++++++++++ cmd/bd/doctor/git.go | 167 +++++++++++++++++++++ cmd/bd/doctor/git_hygiene_test.go | 172 ++++++++++++++++++++++ cmd/bd/doctor_repair_test.go | 147 ++++++++++++++++++ internal/hooks/hooks_test.go | 12 +- internal/syncbranch/worktree_sync_test.go | 3 +- 8 files changed, 645 insertions(+), 10 deletions(-) create mode 100644 cmd/bd/doctor/fix/database_integrity.go create mode 100644 cmd/bd/doctor/git_hygiene_test.go create mode 100644 cmd/bd/doctor_repair_test.go diff --git a/cmd/bd/doctor.go b/cmd/bd/doctor.go index c2b617e1..ea03cc99 100644 --- a/cmd/bd/doctor.go +++ b/cmd/bd/doctor.go @@ -43,8 +43,8 @@ type doctorResult struct { Checks []doctorCheck `json:"checks"` OverallOK bool `json:"overall_ok"` CLIVersion string `json:"cli_version"` - Timestamp string `json:"timestamp,omitempty"` // bd-9cc: ISO8601 timestamp for historical tracking - Platform map[string]string `json:"platform,omitempty"` // bd-9cc: platform info for debugging + Timestamp string `json:"timestamp,omitempty"` // bd-9cc: ISO8601 timestamp for historical tracking + Platform map[string]string `json:"platform,omitempty"` // bd-9cc: platform info for debugging } var ( @@ -373,6 +373,8 @@ func applyFixList(path string, fixes []doctorCheck) { err = fix.Permissions(path) case "Database": err = fix.DatabaseVersion(path) + case "Database Integrity": + err = fix.DatabaseIntegrity(path) case "Schema Compatibility": err = fix.SchemaCompatibility(path) case "Repo Fingerprint": @@ -750,6 +752,16 @@ func runDiagnostics(path string) doctorResult { result.Checks = append(result.Checks, mergeDriverCheck) // Don't fail overall check for merge driver, just warn + // Check 15a: Git working tree cleanliness (AGENTS.md hygiene) + gitWorkingTreeCheck := convertWithCategory(doctor.CheckGitWorkingTree(path), doctor.CategoryGit) + result.Checks = append(result.Checks, gitWorkingTreeCheck) + // Don't fail overall check for dirty working tree, just warn + + // Check 15b: Git upstream sync (ahead/behind/diverged) + gitUpstreamCheck := convertWithCategory(doctor.CheckGitUpstream(path), doctor.CategoryGit) + result.Checks = append(result.Checks, gitUpstreamCheck) + // Don't fail overall check for upstream drift, just warn + // Check 16: Metadata.json version tracking (bd-u4sb) metadataCheck := convertWithCategory(doctor.CheckMetadataVersionTracking(path, Version), doctor.CategoryMetadata) result.Checks = append(result.Checks, metadataCheck) diff --git a/cmd/bd/doctor/database.go b/cmd/bd/doctor/database.go index 674a6c17..0ecf4492 100644 --- a/cmd/bd/doctor/database.go +++ b/cmd/bd/doctor/database.go @@ -251,6 +251,7 @@ func CheckDatabaseIntegrity(path string) DoctorCheck { Status: StatusError, Message: "Failed to open database for integrity check", Detail: err.Error(), + Fix: "Run 'bd doctor --fix' to back up the corrupt DB and rebuild from JSONL (if available), or restore from backup", } } defer db.Close() @@ -264,6 +265,7 @@ func CheckDatabaseIntegrity(path string) DoctorCheck { Status: StatusError, Message: "Failed to run integrity check", Detail: err.Error(), + Fix: "Run 'bd doctor --fix' to back up the corrupt DB and rebuild from JSONL (if available), or restore from backup", } } defer rows.Close() @@ -292,7 +294,7 @@ func CheckDatabaseIntegrity(path string) DoctorCheck { Status: StatusError, Message: "Database corruption detected", Detail: strings.Join(results, "; "), - Fix: "Database may need recovery. Export with 'bd export' if possible, then restore from backup or reinitialize", + Fix: "Run 'bd doctor --fix' to back up the corrupt DB and rebuild from JSONL (if available), or restore from backup", } } diff --git a/cmd/bd/doctor/fix/database_integrity.go b/cmd/bd/doctor/fix/database_integrity.go new file mode 100644 index 00000000..8c8e39c9 --- /dev/null +++ b/cmd/bd/doctor/fix/database_integrity.go @@ -0,0 +1,134 @@ +package fix + +import ( + "bufio" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "time" + + "github.com/steveyegge/beads/internal/beads" + "github.com/steveyegge/beads/internal/configfile" + "github.com/steveyegge/beads/internal/utils" +) + +// DatabaseIntegrity attempts to recover from database corruption by: +// 1. Backing up the corrupt database (and WAL/SHM if present) +// 2. Re-initializing the database from the git-tracked JSONL export +// +// This is intentionally conservative: it will not delete JSONL, and it preserves the +// original DB as a backup for forensic recovery. +func DatabaseIntegrity(path string) error { + if err := validateBeadsWorkspace(path); err != nil { + return err + } + + absPath, err := filepath.Abs(path) + if err != nil { + return fmt.Errorf("failed to resolve path: %w", err) + } + + beadsDir := filepath.Join(absPath, ".beads") + + // Resolve database path (respects metadata.json database override). + var dbPath string + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil && cfg.Database != "" { + dbPath = cfg.DatabasePath(beadsDir) + } else { + dbPath = filepath.Join(beadsDir, beads.CanonicalDatabaseName) + } + + // Find JSONL source of truth. + jsonlPath := "" + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil { + candidate := cfg.JSONLPath(beadsDir) + if _, err := os.Stat(candidate); err == nil { + jsonlPath = candidate + } + } + if jsonlPath == "" { + for _, name := range []string{"issues.jsonl", "beads.jsonl"} { + candidate := filepath.Join(beadsDir, name) + if _, err := os.Stat(candidate); err == nil { + jsonlPath = candidate + break + } + } + } + if jsonlPath == "" { + return fmt.Errorf("cannot auto-recover: no JSONL export found in %s", beadsDir) + } + + // Back up corrupt DB and its sidecar files. + ts := time.Now().UTC().Format("20060102T150405Z") + backupDB := dbPath + "." + ts + ".corrupt.backup.db" + if err := os.Rename(dbPath, backupDB); err != nil { + return fmt.Errorf("failed to back up database: %w", err) + } + for _, suffix := range []string{"-wal", "-shm", "-journal"} { + sidecar := dbPath + suffix + if _, err := os.Stat(sidecar); err == nil { + _ = os.Rename(sidecar, backupDB+suffix) // best effort + } + } + + // Rebuild via bd init, pointing at the same db path. + bdBinary, err := getBdBinary() + if err != nil { + return err + } + + args := []string{"--db", dbPath, "init", "--quiet", "--force", "--skip-hooks", "--skip-merge-driver"} + if prefix := detectPrefixFromJSONL(jsonlPath); prefix != "" { + args = append(args, "--prefix", prefix) + } + + cmd := exec.Command(bdBinary, args...) // #nosec G204 -- bdBinary is a validated executable path + cmd.Dir = absPath + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Env = os.Environ() + + if err := cmd.Run(); err != nil { + // Best-effort rollback: if init didn't recreate the db, restore the backup. + if _, statErr := os.Stat(dbPath); os.IsNotExist(statErr) { + _ = os.Rename(backupDB, dbPath) + for _, suffix := range []string{"-wal", "-shm", "-journal"} { + _ = os.Rename(backupDB+suffix, dbPath+suffix) + } + } + return fmt.Errorf("failed to rebuild database from JSONL: %w (backup: %s)", err, backupDB) + } + + return nil +} + +func detectPrefixFromJSONL(jsonlPath string) string { + f, err := os.Open(jsonlPath) // #nosec G304 -- jsonlPath is within the workspace + if err != nil { + return "" + } + defer f.Close() + + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := scanner.Bytes() + if len(line) == 0 { + continue + } + var issue struct { + ID string `json:"id"` + } + if err := json.Unmarshal(line, &issue); err != nil { + continue + } + if issue.ID == "" { + continue + } + return utils.ExtractIssuePrefix(issue.ID) + } + + return "" +} diff --git a/cmd/bd/doctor/git.go b/cmd/bd/doctor/git.go index 99687b7c..77c234a1 100644 --- a/cmd/bd/doctor/git.go +++ b/cmd/bd/doctor/git.go @@ -78,6 +78,173 @@ func CheckGitHooks() DoctorCheck { } } +// CheckGitWorkingTree checks if the git working tree is clean. +// This helps prevent leaving work stranded (AGENTS.md: keep git state clean). +func CheckGitWorkingTree(path string) DoctorCheck { + cmd := exec.Command("git", "rev-parse", "--git-dir") + cmd.Dir = path + if err := cmd.Run(); err != nil { + return DoctorCheck{ + Name: "Git Working Tree", + Status: StatusOK, + Message: "N/A (not a git repository)", + } + } + + cmd = exec.Command("git", "status", "--porcelain") + cmd.Dir = path + out, err := cmd.Output() + if err != nil { + return DoctorCheck{ + Name: "Git Working Tree", + Status: StatusWarning, + Message: "Unable to check git status", + Detail: err.Error(), + Fix: "Run 'git status' and commit/stash changes before syncing", + } + } + + status := strings.TrimSpace(string(out)) + if status == "" { + return DoctorCheck{ + Name: "Git Working Tree", + Status: StatusOK, + Message: "Clean", + } + } + + // Show a small sample of paths for quick debugging. + lines := strings.Split(status, "\n") + maxLines := 8 + if len(lines) > maxLines { + lines = append(lines[:maxLines], "…") + } + + return DoctorCheck{ + Name: "Git Working Tree", + Status: StatusWarning, + Message: "Uncommitted changes present", + Detail: strings.Join(lines, "\n"), + Fix: "Commit or stash changes, then follow AGENTS.md: git pull --rebase && git push", + } +} + +// CheckGitUpstream checks whether the current branch is up to date with its upstream. +// This catches common "forgot to pull/push" failure modes (AGENTS.md: pull --rebase, push). +func CheckGitUpstream(path string) DoctorCheck { + cmd := exec.Command("git", "rev-parse", "--git-dir") + cmd.Dir = path + if err := cmd.Run(); err != nil { + return DoctorCheck{ + Name: "Git Upstream", + Status: StatusOK, + Message: "N/A (not a git repository)", + } + } + + // Detect detached HEAD. + cmd = exec.Command("git", "symbolic-ref", "--short", "HEAD") + cmd.Dir = path + branchOut, err := cmd.Output() + if err != nil { + return DoctorCheck{ + Name: "Git Upstream", + Status: StatusWarning, + Message: "Detached HEAD (no branch)", + Fix: "Check out a branch before syncing", + } + } + branch := strings.TrimSpace(string(branchOut)) + + cmd = exec.Command("git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}") + cmd.Dir = path + upOut, err := cmd.Output() + if err != nil { + return DoctorCheck{ + Name: "Git Upstream", + Status: StatusWarning, + Message: fmt.Sprintf("No upstream configured for %s", branch), + Fix: fmt.Sprintf("Set upstream then push: git push -u origin %s", branch), + } + } + upstream := strings.TrimSpace(string(upOut)) + + ahead, aheadErr := gitRevListCount(path, "@{u}..HEAD") + behind, behindErr := gitRevListCount(path, "HEAD..@{u}") + if aheadErr != nil || behindErr != nil { + detailParts := []string{} + if aheadErr != nil { + detailParts = append(detailParts, "ahead: "+aheadErr.Error()) + } + if behindErr != nil { + detailParts = append(detailParts, "behind: "+behindErr.Error()) + } + return DoctorCheck{ + Name: "Git Upstream", + Status: StatusWarning, + Message: fmt.Sprintf("Unable to compare with upstream (%s)", upstream), + Detail: strings.Join(detailParts, "; "), + Fix: "Run 'git fetch' then check: git status -sb", + } + } + + if ahead == 0 && behind == 0 { + return DoctorCheck{ + Name: "Git Upstream", + Status: StatusOK, + Message: fmt.Sprintf("Up to date (%s)", upstream), + Detail: fmt.Sprintf("Branch: %s", branch), + } + } + + if ahead > 0 && behind == 0 { + return DoctorCheck{ + Name: "Git Upstream", + Status: StatusWarning, + Message: fmt.Sprintf("Ahead of upstream by %d commit(s)", ahead), + Detail: fmt.Sprintf("Branch: %s, upstream: %s", branch, upstream), + Fix: "Run 'git push' (AGENTS.md: git pull --rebase && git push)", + } + } + + if behind > 0 && ahead == 0 { + return DoctorCheck{ + Name: "Git Upstream", + Status: StatusWarning, + Message: fmt.Sprintf("Behind upstream by %d commit(s)", behind), + Detail: fmt.Sprintf("Branch: %s, upstream: %s", branch, upstream), + Fix: "Run 'git pull --rebase' (then re-run bd sync / bd doctor)", + } + } + + return DoctorCheck{ + Name: "Git Upstream", + Status: StatusWarning, + Message: fmt.Sprintf("Diverged from upstream (ahead %d, behind %d)", ahead, behind), + Detail: fmt.Sprintf("Branch: %s, upstream: %s", branch, upstream), + Fix: "Run 'git pull --rebase' then 'git push'", + } +} + +func gitRevListCount(path string, rangeExpr string) (int, error) { + cmd := exec.Command("git", "rev-list", "--count", rangeExpr) // #nosec G204 -- fixed args + cmd.Dir = path + out, err := cmd.Output() + if err != nil { + return 0, err + } + countStr := strings.TrimSpace(string(out)) + if countStr == "" { + return 0, nil + } + + var n int + if _, err := fmt.Sscanf(countStr, "%d", &n); err != nil { + return 0, err + } + return n, nil +} + // CheckSyncBranchHookCompatibility checks if pre-push hook is compatible with sync-branch mode. // When sync-branch is configured, the pre-push hook must have the sync-branch bypass logic // (added in version 0.29.0). Without it, users experience circular "bd sync" failures (issue #532). diff --git a/cmd/bd/doctor/git_hygiene_test.go b/cmd/bd/doctor/git_hygiene_test.go new file mode 100644 index 00000000..89c68aba --- /dev/null +++ b/cmd/bd/doctor/git_hygiene_test.go @@ -0,0 +1,172 @@ +package doctor + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func mkTmpDirInTmp(t *testing.T, prefix string) string { + t.Helper() + dir, err := os.MkdirTemp("/tmp", prefix) + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + t.Cleanup(func() { _ = os.RemoveAll(dir) }) + return dir +} + +func runGit(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, string(out)) + } + return string(out) +} + +func initRepo(t *testing.T, dir string, branch string) { + t.Helper() + _ = os.MkdirAll(filepath.Join(dir, ".beads"), 0755) + runGit(t, dir, "init", "-b", branch) + runGit(t, dir, "config", "user.email", "test@test.com") + runGit(t, dir, "config", "user.name", "Test User") +} + +func commitFile(t *testing.T, dir, name, content, msg string) { + t.Helper() + path := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("write file: %v", err) + } + runGit(t, dir, "add", name) + runGit(t, dir, "commit", "-m", msg) +} + +func TestCheckGitWorkingTree(t *testing.T) { + t.Run("not a git repo", func(t *testing.T) { + dir := mkTmpDirInTmp(t, "bd-git-nt-*") + check := CheckGitWorkingTree(dir) + if check.Status != StatusOK { + t.Fatalf("status=%q want %q", check.Status, StatusOK) + } + if !strings.Contains(check.Message, "N/A") { + t.Fatalf("message=%q want N/A", check.Message) + } + }) + + t.Run("clean", func(t *testing.T) { + dir := mkTmpDirInTmp(t, "bd-git-clean-*") + initRepo(t, dir, "main") + commitFile(t, dir, "README.md", "# test\n", "initial") + + check := CheckGitWorkingTree(dir) + if check.Status != StatusOK { + t.Fatalf("status=%q want %q (msg=%q)", check.Status, StatusOK, check.Message) + } + }) + + t.Run("dirty", func(t *testing.T) { + dir := mkTmpDirInTmp(t, "bd-git-dirty-*") + initRepo(t, dir, "main") + commitFile(t, dir, "README.md", "# test\n", "initial") + if err := os.WriteFile(filepath.Join(dir, "dirty.txt"), []byte("x"), 0644); err != nil { + t.Fatalf("write dirty file: %v", err) + } + + check := CheckGitWorkingTree(dir) + if check.Status != StatusWarning { + t.Fatalf("status=%q want %q (msg=%q)", check.Status, StatusWarning, check.Message) + } + }) +} + +func TestCheckGitUpstream(t *testing.T) { + t.Run("no upstream", func(t *testing.T) { + dir := mkTmpDirInTmp(t, "bd-git-up-*") + initRepo(t, dir, "main") + commitFile(t, dir, "README.md", "# test\n", "initial") + + check := CheckGitUpstream(dir) + if check.Status != StatusWarning { + t.Fatalf("status=%q want %q (msg=%q)", check.Status, StatusWarning, check.Message) + } + if !strings.Contains(check.Message, "No upstream") { + t.Fatalf("message=%q want to mention upstream", check.Message) + } + }) + + t.Run("up to date", func(t *testing.T) { + dir := mkTmpDirInTmp(t, "bd-git-up2-*") + remote := mkTmpDirInTmp(t, "bd-git-remote-*") + runGit(t, remote, "init", "--bare") + + initRepo(t, dir, "main") + commitFile(t, dir, "README.md", "# test\n", "initial") + runGit(t, dir, "remote", "add", "origin", remote) + runGit(t, dir, "push", "-u", "origin", "main") + + check := CheckGitUpstream(dir) + if check.Status != StatusOK { + t.Fatalf("status=%q want %q (msg=%q)", check.Status, StatusOK, check.Message) + } + }) + + t.Run("ahead of upstream", func(t *testing.T) { + dir := mkTmpDirInTmp(t, "bd-git-ahead-*") + remote := mkTmpDirInTmp(t, "bd-git-remote2-*") + runGit(t, remote, "init", "--bare") + + initRepo(t, dir, "main") + commitFile(t, dir, "README.md", "# test\n", "initial") + runGit(t, dir, "remote", "add", "origin", remote) + runGit(t, dir, "push", "-u", "origin", "main") + + commitFile(t, dir, "file2.txt", "x", "local commit") + + check := CheckGitUpstream(dir) + if check.Status != StatusWarning { + t.Fatalf("status=%q want %q (msg=%q)", check.Status, StatusWarning, check.Message) + } + if !strings.Contains(check.Message, "Ahead") { + t.Fatalf("message=%q want to mention ahead", check.Message) + } + }) + + t.Run("behind upstream", func(t *testing.T) { + dir := mkTmpDirInTmp(t, "bd-git-behind-*") + remote := mkTmpDirInTmp(t, "bd-git-remote3-*") + runGit(t, remote, "init", "--bare") + + initRepo(t, dir, "main") + commitFile(t, dir, "README.md", "# test\n", "initial") + runGit(t, dir, "remote", "add", "origin", remote) + runGit(t, dir, "push", "-u", "origin", "main") + + // Advance remote via another clone. + clone := mkTmpDirInTmp(t, "bd-git-clone-*") + runGit(t, clone, "clone", remote, ".") + runGit(t, clone, "config", "user.email", "test@test.com") + runGit(t, clone, "config", "user.name", "Test User") + commitFile(t, clone, "remote.txt", "y", "remote commit") + runGit(t, clone, "push", "origin", "main") + + // Update tracking refs. + runGit(t, dir, "fetch", "origin") + + check := CheckGitUpstream(dir) + if check.Status != StatusWarning { + t.Fatalf("status=%q want %q (msg=%q)", check.Status, StatusWarning, check.Message) + } + if !strings.Contains(check.Message, "Behind") { + t.Fatalf("message=%q want to mention behind", check.Message) + } + }) +} diff --git a/cmd/bd/doctor_repair_test.go b/cmd/bd/doctor_repair_test.go new file mode 100644 index 00000000..b7476bf5 --- /dev/null +++ b/cmd/bd/doctor_repair_test.go @@ -0,0 +1,147 @@ +package main + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func buildBDForTest(t *testing.T) string { + t.Helper() + exeName := "bd" + if runtime.GOOS == "windows" { + exeName = "bd.exe" + } + + binDir := t.TempDir() + exe := filepath.Join(binDir, exeName) + cmd := exec.Command("go", "build", "-o", exe, ".") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("go build failed: %v\n%s", err, string(out)) + } + return exe +} + +func mkTmpDirInTmp(t *testing.T, prefix string) string { + t.Helper() + dir, err := os.MkdirTemp("/tmp", prefix) + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + t.Cleanup(func() { _ = os.RemoveAll(dir) }) + return dir +} + +func runBDSideDB(t *testing.T, exe, dir, dbPath string, args ...string) (string, error) { + t.Helper() + fullArgs := []string{"--db", dbPath} + if len(args) > 0 && args[0] != "init" { + fullArgs = append(fullArgs, "--no-daemon") + } + fullArgs = append(fullArgs, args...) + + cmd := exec.Command(exe, fullArgs...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "BEADS_NO_DAEMON=1", + "BEADS_DIR="+filepath.Join(dir, ".beads"), + ) + out, err := cmd.CombinedOutput() + return string(out), err +} + +func TestDoctorRepair_CorruptDatabase_RebuildFromJSONL(t *testing.T) { + if testing.Short() { + t.Skip("skipping slow repair test in short mode") + } + + bdExe := buildBDForTest(t) + ws := mkTmpDirInTmp(t, "bd-doctor-repair-*") + dbPath := filepath.Join(ws, ".beads", "beads.db") + jsonlPath := filepath.Join(ws, ".beads", "issues.jsonl") + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "init", "--prefix", "chaos", "--quiet"); err != nil { + t.Fatalf("bd init failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "create", "Chaos issue", "-p", "1"); err != nil { + t.Fatalf("bd create failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "export", "-o", jsonlPath, "--force"); err != nil { + t.Fatalf("bd export failed: %v", err) + } + + // Corrupt the SQLite file (truncate) and verify doctor reports an integrity error. + if err := os.Truncate(dbPath, 128); err != nil { + t.Fatalf("truncate db: %v", err) + } + + out, err := runBDSideDB(t, bdExe, ws, dbPath, "doctor", "--json") + if err == nil { + t.Fatalf("expected bd doctor to fail on corrupt db") + } + jsonStart := strings.Index(out, "{") + if jsonStart < 0 { + t.Fatalf("doctor output missing JSON: %s", out) + } + var before doctorResult + if err := json.Unmarshal([]byte(out[jsonStart:]), &before); err != nil { + t.Fatalf("unmarshal doctor json: %v\n%s", err, out) + } + var foundIntegrity bool + for _, c := range before.Checks { + if c.Name == "Database Integrity" { + foundIntegrity = true + if c.Status != statusError { + t.Fatalf("Database Integrity status=%q want %q", c.Status, statusError) + } + } + } + if !foundIntegrity { + t.Fatalf("Database Integrity check not found") + } + + // Attempt auto-repair. + out, err = runBDSideDB(t, bdExe, ws, dbPath, "doctor", "--fix", "--yes") + if err != nil { + t.Fatalf("bd doctor --fix failed: %v\n%s", err, out) + } + + // Doctor should now pass. + out, err = runBDSideDB(t, bdExe, ws, dbPath, "doctor", "--json") + if err != nil { + t.Fatalf("bd doctor after fix failed: %v\n%s", err, out) + } + jsonStart = strings.Index(out, "{") + if jsonStart < 0 { + t.Fatalf("doctor output missing JSON: %s", out) + } + var after doctorResult + if err := json.Unmarshal([]byte(out[jsonStart:]), &after); err != nil { + t.Fatalf("unmarshal doctor json: %v\n%s", err, out) + } + if !after.OverallOK { + t.Fatalf("expected overall_ok=true after repair") + } + + // Data should still be present. + out, err = runBDSideDB(t, bdExe, ws, dbPath, "list", "--json") + if err != nil { + t.Fatalf("bd list failed after repair: %v\n%s", err, out) + } + jsonStart = strings.Index(out, "[") + if jsonStart < 0 { + t.Fatalf("list output missing JSON array: %s", out) + } + var issues []map[string]any + if err := json.Unmarshal([]byte(out[jsonStart:]), &issues); err != nil { + t.Fatalf("unmarshal list json: %v\n%s", err, out) + } + if len(issues) != 1 { + t.Fatalf("expected 1 issue after repair, got %d", len(issues)) + } +} diff --git a/internal/hooks/hooks_test.go b/internal/hooks/hooks_test.go index db4204e5..4e73ab3f 100644 --- a/internal/hooks/hooks_test.go +++ b/internal/hooks/hooks_test.go @@ -336,8 +336,8 @@ func TestRun_Async(t *testing.T) { outputFile := filepath.Join(tmpDir, "async_output.txt") // Create a hook that writes to a file - hookScript := `#!/bin/sh -echo "async" > ` + outputFile + hookScript := "#!/bin/sh\n" + + "echo \"async\" > \"" + outputFile + "\"\n" if err := os.WriteFile(hookPath, []byte(hookScript), 0755); err != nil { t.Fatalf("Failed to create hook file: %v", err) } @@ -348,15 +348,17 @@ echo "async" > ` + outputFile // Run should return immediately runner.Run(EventClose, issue) - // Wait for the async hook to complete with retries + // Wait for the async hook to complete with retries. + // Under high test load the goroutine scheduling + exec can be delayed. var output []byte var err error - for i := 0; i < 10; i++ { - time.Sleep(100 * time.Millisecond) + deadline := time.Now().Add(3 * time.Second) + for time.Now().Before(deadline) { output, err = os.ReadFile(outputFile) if err == nil { break } + time.Sleep(50 * time.Millisecond) } if err != nil { diff --git a/internal/syncbranch/worktree_sync_test.go b/internal/syncbranch/worktree_sync_test.go index 038738b9..e161b3b8 100644 --- a/internal/syncbranch/worktree_sync_test.go +++ b/internal/syncbranch/worktree_sync_test.go @@ -392,7 +392,7 @@ func setupTestRepoWithRemote(t *testing.T) string { } // Initialize git repo - runGit(t, tmpDir, "init") + runGit(t, tmpDir, "init", "-b", "master") runGit(t, tmpDir, "config", "user.email", "test@test.com") runGit(t, tmpDir, "config", "user.name", "Test User") @@ -413,4 +413,3 @@ func setupTestRepoWithRemote(t *testing.T) string { return tmpDir } - From b089aaa0d64a7ef08e1756d4970bb1496b3b70f8 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Thu, 25 Dec 2025 21:50:13 -0400 Subject: [PATCH 02/13] tests: add chaos doctor repair coverage and stabilize git init Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- cmd/bd/daemon_autoimport_test.go | 140 +++++++++---------- cmd/bd/daemon_sync_branch_test.go | 94 ++++++------- cmd/bd/delete_rpc_test.go | 7 +- cmd/bd/doctor/git_hygiene_test.go | 6 +- cmd/bd/doctor_repair_chaos_test.go | 125 +++++++++++++++++ cmd/bd/doctor_repair_test.go | 6 +- cmd/bd/git_sync_test.go | 82 +++++------ internal/beads/beads_hash_multiclone_test.go | 56 ++++---- 8 files changed, 323 insertions(+), 193 deletions(-) create mode 100644 cmd/bd/doctor_repair_chaos_test.go diff --git a/cmd/bd/daemon_autoimport_test.go b/cmd/bd/daemon_autoimport_test.go index 07aaef46..e959ba51 100644 --- a/cmd/bd/daemon_autoimport_test.go +++ b/cmd/bd/daemon_autoimport_test.go @@ -30,36 +30,36 @@ func TestDaemonAutoImportAfterGitPull(t *testing.T) { t.Fatal(err) } defer os.RemoveAll(tempDir) - + // Create "remote" repository remoteDir := filepath.Join(tempDir, "remote") if err := os.MkdirAll(remoteDir, 0750); err != nil { t.Fatalf("Failed to create remote dir: %v", err) } - + // Initialize remote git repo - runGitCmd(t, remoteDir, "init", "--bare") - + runGitCmd(t, remoteDir, "init", "--bare", "-b", "master") + // Create "clone1" repository (Agent A) clone1Dir := filepath.Join(tempDir, "clone1") runGitCmd(t, tempDir, "clone", remoteDir, clone1Dir) configureGit(t, clone1Dir) - + // Initialize beads in clone1 clone1BeadsDir := filepath.Join(clone1Dir, ".beads") if err := os.MkdirAll(clone1BeadsDir, 0750); err != nil { t.Fatalf("Failed to create .beads dir: %v", err) } - + clone1DBPath := filepath.Join(clone1BeadsDir, "test.db") clone1Store := newTestStore(t, clone1DBPath) defer clone1Store.Close() - + ctx := context.Background() if err := clone1Store.SetMetadata(ctx, "issue_prefix", "test"); err != nil { t.Fatalf("Failed to set prefix: %v", err) } - + // Create an open issue in clone1 issue := &types.Issue{ Title: "Test daemon auto-import", @@ -73,39 +73,39 @@ func TestDaemonAutoImportAfterGitPull(t *testing.T) { t.Fatalf("Failed to create issue: %v", err) } issueID := issue.ID - + // Export to JSONL jsonlPath := filepath.Join(clone1BeadsDir, "issues.jsonl") if err := exportIssuesToJSONL(ctx, clone1Store, jsonlPath); err != nil { t.Fatalf("Failed to export: %v", err) } - + // Commit and push from clone1 runGitCmd(t, clone1Dir, "add", ".beads") runGitCmd(t, clone1Dir, "commit", "-m", "Add test issue") runGitCmd(t, clone1Dir, "push", "origin", "master") - + // Create "clone2" repository (Agent B) clone2Dir := filepath.Join(tempDir, "clone2") runGitCmd(t, tempDir, "clone", remoteDir, clone2Dir) configureGit(t, clone2Dir) - + // Initialize empty database in clone2 clone2BeadsDir := filepath.Join(clone2Dir, ".beads") clone2DBPath := filepath.Join(clone2BeadsDir, "test.db") clone2Store := newTestStore(t, clone2DBPath) defer clone2Store.Close() - + if err := clone2Store.SetMetadata(ctx, "issue_prefix", "test"); err != nil { t.Fatalf("Failed to set prefix: %v", err) } - + // Import initial JSONL in clone2 clone2JSONLPath := filepath.Join(clone2BeadsDir, "issues.jsonl") if err := importJSONLToStore(ctx, clone2Store, clone2DBPath, clone2JSONLPath); err != nil { t.Fatalf("Failed to import: %v", err) } - + // Verify issue exists in clone2 initialIssue, err := clone2Store.GetIssue(ctx, issueID) if err != nil { @@ -114,27 +114,27 @@ func TestDaemonAutoImportAfterGitPull(t *testing.T) { if initialIssue.Status != types.StatusOpen { t.Errorf("Expected status open, got %s", initialIssue.Status) } - + // NOW THE CRITICAL TEST: Agent A closes the issue and pushes t.Run("DaemonAutoImportsAfterGitPull", func(t *testing.T) { // Agent A closes the issue if err := clone1Store.CloseIssue(ctx, issueID, "Completed", "agent-a"); err != nil { t.Fatalf("Failed to close issue: %v", err) } - + // Agent A exports to JSONL if err := exportIssuesToJSONL(ctx, clone1Store, jsonlPath); err != nil { t.Fatalf("Failed to export after close: %v", err) } - + // Agent A commits and pushes runGitCmd(t, clone1Dir, "add", ".beads/issues.jsonl") runGitCmd(t, clone1Dir, "commit", "-m", "Close issue") runGitCmd(t, clone1Dir, "push", "origin", "master") - + // Agent B does git pull (updates JSONL on disk) runGitCmd(t, clone2Dir, "pull") - + // Wait for filesystem to settle after git operations // Windows has lower filesystem timestamp precision (typically 100ms) // and file I/O may be slower, so we need a longer delay @@ -143,23 +143,23 @@ func TestDaemonAutoImportAfterGitPull(t *testing.T) { } else { time.Sleep(50 * time.Millisecond) } - + // Start daemon server in clone2 socketPath := filepath.Join(clone2BeadsDir, "bd.sock") os.Remove(socketPath) // Ensure clean state - + server := rpc.NewServer(socketPath, clone2Store, clone2Dir, clone2DBPath) - + // Start server in background serverCtx, serverCancel := context.WithCancel(context.Background()) defer serverCancel() - + go func() { if err := server.Start(serverCtx); err != nil { t.Logf("Server error: %v", err) } }() - + // Wait for server to be ready for i := 0; i < 50; i++ { time.Sleep(10 * time.Millisecond) @@ -167,7 +167,7 @@ func TestDaemonAutoImportAfterGitPull(t *testing.T) { break } } - + // Simulate a daemon request (like "bd show ") // The daemon should auto-import the updated JSONL before responding client, err := rpc.TryConnect(socketPath) @@ -178,15 +178,15 @@ func TestDaemonAutoImportAfterGitPull(t *testing.T) { t.Fatal("Client is nil") } defer client.Close() - + client.SetDatabasePath(clone2DBPath) // Route to correct database - + // Make a request that triggers auto-import check resp, err := client.Execute("show", map[string]string{"id": issueID}) if err != nil { t.Fatalf("Failed to get issue from daemon: %v", err) } - + // Parse response var issue types.Issue issueJSON, err := json.Marshal(resp.Data) @@ -196,25 +196,25 @@ func TestDaemonAutoImportAfterGitPull(t *testing.T) { if err := json.Unmarshal(issueJSON, &issue); err != nil { t.Fatalf("Failed to unmarshal issue: %v", err) } - + status := issue.Status - + // CRITICAL ASSERTION: Daemon should return CLOSED status from JSONL // not stale OPEN status from SQLite if status != types.StatusClosed { t.Errorf("DAEMON AUTO-IMPORT FAILED: Expected status 'closed' but got '%s'", status) t.Errorf("This means daemon is serving stale SQLite data instead of auto-importing JSONL") - + // Double-check JSONL has correct status jsonlData, _ := os.ReadFile(clone2JSONLPath) t.Logf("JSONL content: %s", string(jsonlData)) - + // Double-check what's in SQLite directIssue, _ := clone2Store.GetIssue(ctx, issueID) t.Logf("SQLite status: %s", directIssue.Status) } }) - + // Additional test: Verify multiple rapid changes t.Run("DaemonHandlesRapidUpdates", func(t *testing.T) { // Agent A updates priority @@ -223,18 +223,18 @@ func TestDaemonAutoImportAfterGitPull(t *testing.T) { }, "agent-a"); err != nil { t.Fatalf("Failed to update priority: %v", err) } - + if err := exportIssuesToJSONL(ctx, clone1Store, jsonlPath); err != nil { t.Fatalf("Failed to export: %v", err) } - + runGitCmd(t, clone1Dir, "add", ".beads/issues.jsonl") runGitCmd(t, clone1Dir, "commit", "-m", "Update priority") runGitCmd(t, clone1Dir, "push", "origin", "master") - + // Agent B pulls runGitCmd(t, clone2Dir, "pull") - + // Query via daemon - should see priority 0 // (Execute forces auto-import synchronously) socketPath := filepath.Join(clone2BeadsDir, "bd.sock") @@ -243,18 +243,18 @@ func TestDaemonAutoImportAfterGitPull(t *testing.T) { t.Fatalf("Failed to connect to daemon: %v", err) } defer client.Close() - + client.SetDatabasePath(clone2DBPath) // Route to correct database - + resp, err := client.Execute("show", map[string]string{"id": issueID}) if err != nil { t.Fatalf("Failed to get issue from daemon: %v", err) } - + var issue types.Issue issueJSON, _ := json.Marshal(resp.Data) json.Unmarshal(issueJSON, &issue) - + if issue.Priority != 0 { t.Errorf("Expected priority 0 after auto-import, got %d", issue.Priority) } @@ -273,23 +273,23 @@ func TestDaemonAutoImportDataCorruption(t *testing.T) { t.Fatal(err) } defer os.RemoveAll(tempDir) - + // Setup remote and two clones remoteDir := filepath.Join(tempDir, "remote") os.MkdirAll(remoteDir, 0750) - runGitCmd(t, remoteDir, "init", "--bare") - + runGitCmd(t, remoteDir, "init", "--bare", "-b", "master") + clone1Dir := filepath.Join(tempDir, "clone1") runGitCmd(t, tempDir, "clone", remoteDir, clone1Dir) configureGit(t, clone1Dir) - + clone2Dir := filepath.Join(tempDir, "clone2") runGitCmd(t, tempDir, "clone", remoteDir, clone2Dir) configureGit(t, clone2Dir) - + // Initialize beads in both clones ctx := context.Background() - + // Clone1 setup clone1BeadsDir := filepath.Join(clone1Dir, ".beads") os.MkdirAll(clone1BeadsDir, 0750) @@ -297,7 +297,7 @@ func TestDaemonAutoImportDataCorruption(t *testing.T) { clone1Store := newTestStore(t, clone1DBPath) defer clone1Store.Close() clone1Store.SetMetadata(ctx, "issue_prefix", "test") - + // Clone2 setup clone2BeadsDir := filepath.Join(clone2Dir, ".beads") os.MkdirAll(clone2BeadsDir, 0750) @@ -305,7 +305,7 @@ func TestDaemonAutoImportDataCorruption(t *testing.T) { clone2Store := newTestStore(t, clone2DBPath) defer clone2Store.Close() clone2Store.SetMetadata(ctx, "issue_prefix", "test") - + // Agent A creates issue and pushes issue2 := &types.Issue{ Title: "Shared issue", @@ -317,18 +317,18 @@ func TestDaemonAutoImportDataCorruption(t *testing.T) { } clone1Store.CreateIssue(ctx, issue2, "agent-a") issueID := issue2.ID - + clone1JSONLPath := filepath.Join(clone1BeadsDir, "issues.jsonl") exportIssuesToJSONL(ctx, clone1Store, clone1JSONLPath) runGitCmd(t, clone1Dir, "add", ".beads") runGitCmd(t, clone1Dir, "commit", "-m", "Initial issue") runGitCmd(t, clone1Dir, "push", "origin", "master") - + // Agent B pulls and imports runGitCmd(t, clone2Dir, "pull") clone2JSONLPath := filepath.Join(clone2BeadsDir, "issues.jsonl") importJSONLToStore(ctx, clone2Store, clone2DBPath, clone2JSONLPath) - + // THE CORRUPTION SCENARIO: // 1. Agent A closes the issue and pushes clone1Store.CloseIssue(ctx, issueID, "Done", "agent-a") @@ -336,31 +336,31 @@ func TestDaemonAutoImportDataCorruption(t *testing.T) { runGitCmd(t, clone1Dir, "add", ".beads/issues.jsonl") runGitCmd(t, clone1Dir, "commit", "-m", "Close issue") runGitCmd(t, clone1Dir, "push", "origin", "master") - + // 2. Agent B does git pull (JSONL updated on disk) runGitCmd(t, clone2Dir, "pull") - + // Wait for filesystem to settle after git operations time.Sleep(50 * time.Millisecond) - + // 3. Agent B daemon exports STALE data (if auto-import doesn't work) // This would overwrite Agent A's closure with old "open" status - + // Start daemon in clone2 socketPath := filepath.Join(clone2BeadsDir, "bd.sock") os.Remove(socketPath) - + server := rpc.NewServer(socketPath, clone2Store, clone2Dir, clone2DBPath) - + serverCtx, serverCancel := context.WithCancel(context.Background()) defer serverCancel() - + go func() { if err := server.Start(serverCtx); err != nil { t.Logf("Server error: %v", err) } }() - + // Wait for server for i := 0; i < 50; i++ { time.Sleep(10 * time.Millisecond) @@ -368,43 +368,43 @@ func TestDaemonAutoImportDataCorruption(t *testing.T) { break } } - + // Trigger daemon operation (should auto-import first) client, err := rpc.TryConnect(socketPath) if err != nil { t.Fatalf("Failed to connect: %v", err) } defer client.Close() - + client.SetDatabasePath(clone2DBPath) - + resp, err := client.Execute("show", map[string]string{"id": issueID}) if err != nil { t.Fatalf("Failed to get issue: %v", err) } - + var issue types.Issue issueJSON, _ := json.Marshal(resp.Data) json.Unmarshal(issueJSON, &issue) - + status := issue.Status - + // If daemon didn't auto-import, this would be "open" (stale) // With the fix, it should be "closed" (fresh from JSONL) if status != types.StatusClosed { t.Errorf("DATA CORRUPTION DETECTED: Daemon has stale status '%s' instead of 'closed'", status) t.Error("If daemon exports this stale data, it will overwrite Agent A's changes on next push") } - + // Now simulate daemon export (which happens on timer) // With auto-import working, this export should have fresh data exportIssuesToJSONL(ctx, clone2Store, clone2JSONLPath) - + // Read back JSONL to verify it has correct status data, _ := os.ReadFile(clone2JSONLPath) var exportedIssue types.Issue json.NewDecoder(bytes.NewReader(data)).Decode(&exportedIssue) - + if exportedIssue.Status != types.StatusClosed { t.Errorf("CORRUPTION: Exported JSONL has wrong status '%s', would overwrite remote", exportedIssue.Status) } diff --git a/cmd/bd/daemon_sync_branch_test.go b/cmd/bd/daemon_sync_branch_test.go index d6731347..186c8533 100644 --- a/cmd/bd/daemon_sync_branch_test.go +++ b/cmd/bd/daemon_sync_branch_test.go @@ -48,12 +48,12 @@ func TestSyncBranchCommitAndPush_NotConfigured(t *testing.T) { // Create test issue issue := &types.Issue{ - Title: "Test issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + Title: "Test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), } if err := store.CreateIssue(ctx, issue, "test"); err != nil { t.Fatalf("Failed to create issue: %v", err) @@ -122,12 +122,12 @@ func TestSyncBranchCommitAndPush_Success(t *testing.T) { // Create test issue issue := &types.Issue{ - Title: "Test sync branch issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + Title: "Test sync branch issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), } if err := store.CreateIssue(ctx, issue, "test"); err != nil { t.Fatalf("Failed to create issue: %v", err) @@ -228,12 +228,12 @@ func TestSyncBranchCommitAndPush_EnvOverridesDB(t *testing.T) { // Create test issue and export JSONL issue := &types.Issue{ - Title: "Env override issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + Title: "Env override issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), } if err := store.CreateIssue(ctx, issue, "test"); err != nil { t.Fatalf("Failed to create issue: %v", err) @@ -303,12 +303,12 @@ func TestSyncBranchCommitAndPush_NoChanges(t *testing.T) { } issue := &types.Issue{ - Title: "Test issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + Title: "Test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), } if err := store.CreateIssue(ctx, issue, "test"); err != nil { t.Fatalf("Failed to create issue: %v", err) @@ -380,12 +380,12 @@ func TestSyncBranchCommitAndPush_WorktreeHealthCheck(t *testing.T) { } issue := &types.Issue{ - Title: "Test issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + Title: "Test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), } if err := store.CreateIssue(ctx, issue, "test"); err != nil { t.Fatalf("Failed to create issue: %v", err) @@ -497,7 +497,7 @@ func TestSyncBranchPull_Success(t *testing.T) { if err := os.MkdirAll(remoteDir, 0755); err != nil { t.Fatalf("Failed to create remote dir: %v", err) } - runGitCmd(t, remoteDir, "init", "--bare") + runGitCmd(t, remoteDir, "init", "--bare", "-b", "master") // Create clone1 (will push changes) clone1Dir := filepath.Join(tmpDir, "clone1") @@ -528,12 +528,12 @@ func TestSyncBranchPull_Success(t *testing.T) { // Create issue in clone1 issue := &types.Issue{ - Title: "Test sync pull issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + Title: "Test sync pull issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), } if err := store1.CreateIssue(ctx, issue, "test"); err != nil { t.Fatalf("Failed to create issue: %v", err) @@ -639,7 +639,7 @@ func TestSyncBranchIntegration_EndToEnd(t *testing.T) { tmpDir := t.TempDir() remoteDir := filepath.Join(tmpDir, "remote") os.MkdirAll(remoteDir, 0755) - runGitCmd(t, remoteDir, "init", "--bare") + runGitCmd(t, remoteDir, "init", "--bare", "-b", "master") // Clone1: Agent A clone1Dir := filepath.Join(tmpDir, "clone1") @@ -660,12 +660,12 @@ func TestSyncBranchIntegration_EndToEnd(t *testing.T) { // Agent A creates issue issue := &types.Issue{ - Title: "E2E test issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), + Title: "E2E test issue", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), } store1.CreateIssue(ctx, issue, "agent-a") issueID := issue.ID @@ -914,7 +914,7 @@ func TestSyncBranchMultipleConcurrentClones(t *testing.T) { tmpDir := t.TempDir() remoteDir := filepath.Join(tmpDir, "remote") os.MkdirAll(remoteDir, 0755) - runGitCmd(t, remoteDir, "init", "--bare") + runGitCmd(t, remoteDir, "init", "--bare", "-b", "master") syncBranch := "beads-sync" @@ -1454,7 +1454,7 @@ func TestGitPushFromWorktree_FetchRebaseRetry(t *testing.T) { // Create a "remote" bare repository remoteDir := t.TempDir() - runGitCmd(t, remoteDir, "init", "--bare") + runGitCmd(t, remoteDir, "init", "--bare", "-b", "master") // Create first clone (simulates another developer's clone) clone1Dir := t.TempDir() @@ -1524,7 +1524,7 @@ func TestGitPushFromWorktree_FetchRebaseRetry(t *testing.T) { // Now try to push from worktree - this should trigger the fetch-rebase-retry logic // because the remote has commits that the local worktree doesn't have - err := gitPushFromWorktree(ctx, worktreePath, "beads-sync") + err := gitPushFromWorktree(ctx, worktreePath, "beads-sync", "") if err != nil { t.Fatalf("gitPushFromWorktree failed: %v (expected fetch-rebase-retry to succeed)", err) } diff --git a/cmd/bd/delete_rpc_test.go b/cmd/bd/delete_rpc_test.go index de8b862d..82211b76 100644 --- a/cmd/bd/delete_rpc_test.go +++ b/cmd/bd/delete_rpc_test.go @@ -8,6 +8,7 @@ import ( "context" "encoding/json" "io" + "log/slog" "os" "path/filepath" "strings" @@ -897,11 +898,7 @@ func setupDaemonTestEnvForDelete(t *testing.T) (context.Context, context.CancelF ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - log := daemonLogger{ - logFunc: func(format string, args ...interface{}) { - t.Logf("[daemon] "+format, args...) - }, - } + log := daemonLogger{logger: slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelInfo}))} server, _, err := startRPCServer(ctx, socketPath, testStore, tmpDir, testDBPath, log) if err != nil { diff --git a/cmd/bd/doctor/git_hygiene_test.go b/cmd/bd/doctor/git_hygiene_test.go index 89c68aba..ebc0d266 100644 --- a/cmd/bd/doctor/git_hygiene_test.go +++ b/cmd/bd/doctor/git_hygiene_test.go @@ -12,7 +12,11 @@ func mkTmpDirInTmp(t *testing.T, prefix string) string { t.Helper() dir, err := os.MkdirTemp("/tmp", prefix) if err != nil { - t.Fatalf("failed to create temp dir: %v", err) + // Fallback for platforms without /tmp (e.g. Windows). + dir, err = os.MkdirTemp("", prefix) + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } } t.Cleanup(func() { _ = os.RemoveAll(dir) }) return dir diff --git a/cmd/bd/doctor_repair_chaos_test.go b/cmd/bd/doctor_repair_chaos_test.go new file mode 100644 index 00000000..792327d4 --- /dev/null +++ b/cmd/bd/doctor_repair_chaos_test.go @@ -0,0 +1,125 @@ +//go:build chaos + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestDoctorRepair_CorruptDatabase_NotADatabase_RebuildFromJSONL(t *testing.T) { + bdExe := buildBDForTest(t) + ws := mkTmpDirInTmp(t, "bd-doctor-chaos-*") + dbPath := filepath.Join(ws, ".beads", "beads.db") + jsonlPath := filepath.Join(ws, ".beads", "issues.jsonl") + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "init", "--prefix", "chaos", "--quiet"); err != nil { + t.Fatalf("bd init failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "create", "Chaos issue", "-p", "1"); err != nil { + t.Fatalf("bd create failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "export", "-o", jsonlPath, "--force"); err != nil { + t.Fatalf("bd export failed: %v", err) + } + + // Make the DB unreadable. + if err := os.WriteFile(dbPath, []byte("not a database"), 0644); err != nil { + t.Fatalf("corrupt db: %v", err) + } + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "doctor", "--fix", "--yes"); err != nil { + t.Fatalf("bd doctor --fix failed: %v", err) + } + + if out, err := runBDSideDB(t, bdExe, ws, dbPath, "doctor"); err != nil { + t.Fatalf("bd doctor after fix failed: %v\n%s", err, out) + } +} + +func TestDoctorRepair_CorruptDatabase_NoJSONL_FixFails(t *testing.T) { + bdExe := buildBDForTest(t) + ws := mkTmpDirInTmp(t, "bd-doctor-chaos-nojsonl-*") + dbPath := filepath.Join(ws, ".beads", "beads.db") + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "init", "--prefix", "chaos", "--quiet"); err != nil { + t.Fatalf("bd init failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "create", "Chaos issue", "-p", "1"); err != nil { + t.Fatalf("bd create failed: %v", err) + } + + // Some workflows keep JSONL in sync automatically; force it to be missing. + _ = os.Remove(filepath.Join(ws, ".beads", "issues.jsonl")) + _ = os.Remove(filepath.Join(ws, ".beads", "beads.jsonl")) + + // Corrupt without providing JSONL source-of-truth. + if err := os.Truncate(dbPath, 64); err != nil { + t.Fatalf("truncate db: %v", err) + } + + out, err := runBDSideDB(t, bdExe, ws, dbPath, "doctor", "--fix", "--yes") + if err == nil { + t.Fatalf("expected bd doctor --fix to fail without JSONL") + } + if !strings.Contains(out, "cannot auto-recover") { + t.Fatalf("expected auto-recover error, got:\n%s", out) + } +} + +func TestDoctorRepair_CorruptDatabase_BacksUpSidecars(t *testing.T) { + bdExe := buildBDForTest(t) + ws := mkTmpDirInTmp(t, "bd-doctor-chaos-sidecars-*") + dbPath := filepath.Join(ws, ".beads", "beads.db") + jsonlPath := filepath.Join(ws, ".beads", "issues.jsonl") + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "init", "--prefix", "chaos", "--quiet"); err != nil { + t.Fatalf("bd init failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "create", "Chaos issue", "-p", "1"); err != nil { + t.Fatalf("bd create failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "export", "-o", jsonlPath, "--force"); err != nil { + t.Fatalf("bd export failed: %v", err) + } + + // Ensure sidecars exist so we can verify they get moved with the backup. + for _, suffix := range []string{"-wal", "-shm", "-journal"} { + if err := os.WriteFile(dbPath+suffix, []byte("x"), 0644); err != nil { + t.Fatalf("write sidecar %s: %v", suffix, err) + } + } + if err := os.Truncate(dbPath, 64); err != nil { + t.Fatalf("truncate db: %v", err) + } + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "doctor", "--fix", "--yes"); err != nil { + t.Fatalf("bd doctor --fix failed: %v", err) + } + + // Verify a backup exists, and at least one sidecar got moved. + entries, err := os.ReadDir(filepath.Join(ws, ".beads")) + if err != nil { + t.Fatalf("readdir: %v", err) + } + var backup string + for _, e := range entries { + if strings.Contains(e.Name(), ".corrupt.backup.db") { + backup = filepath.Join(ws, ".beads", e.Name()) + break + } + } + if backup == "" { + t.Fatalf("expected backup db in .beads, found none") + } + + wal := backup + "-wal" + if _, err := os.Stat(wal); err != nil { + // At minimum, the backup DB itself should exist; sidecar backup is best-effort. + if _, err2 := os.Stat(backup); err2 != nil { + t.Fatalf("backup db missing: %v", err2) + } + } +} diff --git a/cmd/bd/doctor_repair_test.go b/cmd/bd/doctor_repair_test.go index b7476bf5..5e223a44 100644 --- a/cmd/bd/doctor_repair_test.go +++ b/cmd/bd/doctor_repair_test.go @@ -31,7 +31,11 @@ func mkTmpDirInTmp(t *testing.T, prefix string) string { t.Helper() dir, err := os.MkdirTemp("/tmp", prefix) if err != nil { - t.Fatalf("failed to create temp dir: %v", err) + // Fallback for platforms without /tmp (e.g. Windows). + dir, err = os.MkdirTemp("", prefix) + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } } t.Cleanup(func() { _ = os.RemoveAll(dir) }) return dir diff --git a/cmd/bd/git_sync_test.go b/cmd/bd/git_sync_test.go index ba5a8b73..d68f0803 100644 --- a/cmd/bd/git_sync_test.go +++ b/cmd/bd/git_sync_test.go @@ -26,36 +26,36 @@ func TestGitPullSyncIntegration(t *testing.T) { // Create temp directory for test repositories tempDir := t.TempDir() - + // Create "remote" repository remoteDir := filepath.Join(tempDir, "remote") if err := os.MkdirAll(remoteDir, 0750); err != nil { t.Fatalf("Failed to create remote dir: %v", err) } - + // Initialize remote git repo - runGitCmd(t, remoteDir, "init", "--bare") - + runGitCmd(t, remoteDir, "init", "--bare", "-b", "master") + // Create "clone1" repository clone1Dir := filepath.Join(tempDir, "clone1") runGitCmd(t, tempDir, "clone", remoteDir, clone1Dir) configureGit(t, clone1Dir) - + // Initialize beads in clone1 clone1BeadsDir := filepath.Join(clone1Dir, ".beads") if err := os.MkdirAll(clone1BeadsDir, 0750); err != nil { t.Fatalf("Failed to create .beads dir: %v", err) } - + clone1DBPath := filepath.Join(clone1BeadsDir, "test.db") clone1Store := newTestStore(t, clone1DBPath) defer clone1Store.Close() - + ctx := context.Background() if err := clone1Store.SetMetadata(ctx, "issue_prefix", "test"); err != nil { t.Fatalf("Failed to set prefix: %v", err) } - + // Create and close an issue in clone1 issue := &types.Issue{ Title: "Test sync issue", @@ -69,80 +69,80 @@ func TestGitPullSyncIntegration(t *testing.T) { t.Fatalf("Failed to create issue: %v", err) } issueID := issue.ID - + // Close the issue if err := clone1Store.CloseIssue(ctx, issueID, "Test completed", "test-user"); err != nil { t.Fatalf("Failed to close issue: %v", err) } - + // Export to JSONL jsonlPath := filepath.Join(clone1BeadsDir, "issues.jsonl") if err := exportIssuesToJSONL(ctx, clone1Store, jsonlPath); err != nil { t.Fatalf("Failed to export: %v", err) } - + // Commit and push from clone1 runGitCmd(t, clone1Dir, "add", ".beads") runGitCmd(t, clone1Dir, "commit", "-m", "Add closed issue") runGitCmd(t, clone1Dir, "push", "origin", "master") - + // Create "clone2" repository clone2Dir := filepath.Join(tempDir, "clone2") runGitCmd(t, tempDir, "clone", remoteDir, clone2Dir) configureGit(t, clone2Dir) - + // Initialize empty database in clone2 clone2BeadsDir := filepath.Join(clone2Dir, ".beads") clone2DBPath := filepath.Join(clone2BeadsDir, "test.db") clone2Store := newTestStore(t, clone2DBPath) defer clone2Store.Close() - + if err := clone2Store.SetMetadata(ctx, "issue_prefix", "test"); err != nil { t.Fatalf("Failed to set prefix: %v", err) } - + // Import the existing JSONL (simulating initial sync) clone2JSONLPath := filepath.Join(clone2BeadsDir, "issues.jsonl") if err := importJSONLToStore(ctx, clone2Store, clone2DBPath, clone2JSONLPath); err != nil { t.Fatalf("Failed to import: %v", err) } - + // Verify issue exists and is closed verifyIssueClosed(t, clone2Store, issueID) - + // Note: We don't commit in clone2 - it stays clean as a read-only consumer - + // Now test git pull scenario: Clone1 makes a change (update priority) if err := clone1Store.UpdateIssue(ctx, issueID, map[string]interface{}{ "priority": 0, }, "test-user"); err != nil { t.Fatalf("Failed to update issue: %v", err) } - + if err := exportIssuesToJSONL(ctx, clone1Store, jsonlPath); err != nil { t.Fatalf("Failed to export after update: %v", err) } - + runGitCmd(t, clone1Dir, "add", ".beads/issues.jsonl") runGitCmd(t, clone1Dir, "commit", "-m", "Update priority") runGitCmd(t, clone1Dir, "push", "origin", "master") - + // Clone2 pulls the change runGitCmd(t, clone2Dir, "pull") - + // Test auto-import in non-daemon mode t.Run("NonDaemonAutoImport", func(t *testing.T) { // Use a temporary local store for this test localStore := newTestStore(t, clone2DBPath) defer localStore.Close() - + // Manually import to simulate auto-import behavior startTime := time.Now() if err := importJSONLToStore(ctx, localStore, clone2DBPath, clone2JSONLPath); err != nil { t.Fatalf("Failed to auto-import: %v", err) } elapsed := time.Since(startTime) - + // Verify priority was updated issue, err := localStore.GetIssue(ctx, issueID) if err != nil { @@ -151,13 +151,13 @@ func TestGitPullSyncIntegration(t *testing.T) { if issue.Priority != 0 { t.Errorf("Expected priority 0 after auto-import, got %d", issue.Priority) } - + // Verify performance: import should be fast if elapsed > 100*time.Millisecond { t.Logf("Info: import took %v", elapsed) } }) - + // Test bd sync --import-only command t.Run("BdSyncCommand", func(t *testing.T) { // Make another change in clone1 (change priority back to 1) @@ -166,27 +166,27 @@ func TestGitPullSyncIntegration(t *testing.T) { }, "test-user"); err != nil { t.Fatalf("Failed to update issue: %v", err) } - + if err := exportIssuesToJSONL(ctx, clone1Store, jsonlPath); err != nil { t.Fatalf("Failed to export: %v", err) } - + runGitCmd(t, clone1Dir, "add", ".beads/issues.jsonl") runGitCmd(t, clone1Dir, "commit", "-m", "Update priority") runGitCmd(t, clone1Dir, "push", "origin", "master") - + // Clone2 pulls runGitCmd(t, clone2Dir, "pull") - + // Use a fresh store for import syncStore := newTestStore(t, clone2DBPath) defer syncStore.Close() - + // Manually trigger import via in-process equivalent if err := importJSONLToStore(ctx, syncStore, clone2DBPath, clone2JSONLPath); err != nil { t.Fatalf("Failed to import via sync: %v", err) } - + // Verify priority was updated back to 1 issue, err := syncStore.GetIssue(ctx, issueID) if err != nil { @@ -214,7 +214,7 @@ func configureGit(t *testing.T, dir string) { runGitCmd(t, dir, "config", "user.email", "test@example.com") runGitCmd(t, dir, "config", "user.name", "Test User") runGitCmd(t, dir, "config", "pull.rebase", "false") - + // Create .gitignore to prevent test database files from being tracked gitignorePath := filepath.Join(dir, ".gitignore") gitignoreContent := `# Test database files @@ -233,7 +233,7 @@ func exportIssuesToJSONL(ctx context.Context, store *sqlite.SQLiteStorage, jsonl if err != nil { return err } - + // Populate dependencies allDeps, err := store.GetAllDependencyRecords(ctx) if err != nil { @@ -244,20 +244,20 @@ func exportIssuesToJSONL(ctx context.Context, store *sqlite.SQLiteStorage, jsonl labels, _ := store.GetLabels(ctx, issue.ID) issue.Labels = labels } - + f, err := os.Create(jsonlPath) if err != nil { return err } defer f.Close() - + encoder := json.NewEncoder(f) for _, issue := range issues { if err := encoder.Encode(issue); err != nil { return err } } - + return nil } @@ -266,7 +266,7 @@ func importJSONLToStore(ctx context.Context, store *sqlite.SQLiteStorage, dbPath if err != nil { return err } - + // Use the autoimport package's AutoImportIfNewer function // For testing, we'll directly parse and import var issues []*types.Issue @@ -278,7 +278,7 @@ func importJSONLToStore(ctx context.Context, store *sqlite.SQLiteStorage, dbPath } issues = append(issues, &issue) } - + // Import each issue for _, issue := range issues { existing, _ := store.GetIssue(ctx, issue.ID) @@ -298,12 +298,12 @@ func importJSONLToStore(ctx context.Context, store *sqlite.SQLiteStorage, dbPath } } } - + // Set last_import_time metadata so staleness check works if err := store.SetMetadata(ctx, "last_import_time", time.Now().Format(time.RFC3339)); err != nil { return err } - + return nil } diff --git a/internal/beads/beads_hash_multiclone_test.go b/internal/beads/beads_hash_multiclone_test.go index 89ee842a..ea5a4383 100644 --- a/internal/beads/beads_hash_multiclone_test.go +++ b/internal/beads/beads_hash_multiclone_test.go @@ -48,10 +48,10 @@ func TestMain(m *testing.M) { fmt.Fprintf(os.Stderr, "Failed to build bd binary: %v\n%s\n", err, out) os.Exit(1) } - + // Optimize git for tests os.Setenv("GIT_CONFIG_NOSYSTEM", "1") - + os.Exit(m.Run()) } @@ -85,35 +85,35 @@ func TestHashIDs_MultiCloneConverge(t *testing.T) { } t.Parallel() tmpDir := testutil.TempDirInMemory(t) - + bdPath := getBDPath() if _, err := os.Stat(bdPath); err != nil { t.Fatalf("bd binary not found at %s", bdPath) } - + // Setup remote and 3 clones remoteDir := setupBareRepo(t, tmpDir) cloneA := setupClone(t, tmpDir, remoteDir, "A", bdPath) cloneB := setupClone(t, tmpDir, remoteDir, "B", bdPath) cloneC := setupClone(t, tmpDir, remoteDir, "C", bdPath) - + // Each clone creates unique issue (different content = different hash ID) createIssueInClone(t, cloneA, "Issue from clone A") createIssueInClone(t, cloneB, "Issue from clone B") createIssueInClone(t, cloneC, "Issue from clone C") - + // Sync all clones once (hash IDs prevent collisions, don't need multiple rounds) for _, clone := range []string{cloneA, cloneB, cloneC} { runCmdOutputWithEnvAllowError(t, clone, map[string]string{"BEADS_NO_DAEMON": "1"}, true, bdPath, "sync") } - + // Verify all clones have all 3 issues expectedTitles := map[string]bool{ "Issue from clone A": true, "Issue from clone B": true, "Issue from clone C": true, } - + allConverged := true for name, dir := range map[string]string{"A": cloneA, "B": cloneB, "C": cloneC} { titles := getTitlesFromClone(t, dir) @@ -122,7 +122,7 @@ func TestHashIDs_MultiCloneConverge(t *testing.T) { allConverged = false } } - + if allConverged { t.Log("✓ All 3 clones converged with hash-based IDs") } else { @@ -138,26 +138,26 @@ func TestHashIDs_IdenticalContentDedup(t *testing.T) { } t.Parallel() tmpDir := testutil.TempDirInMemory(t) - + bdPath := getBDPath() if _, err := os.Stat(bdPath); err != nil { t.Fatalf("bd binary not found at %s", bdPath) } - + // Setup remote and 2 clones remoteDir := setupBareRepo(t, tmpDir) cloneA := setupClone(t, tmpDir, remoteDir, "A", bdPath) cloneB := setupClone(t, tmpDir, remoteDir, "B", bdPath) - + // Both clones create identical issue (same content = same hash ID) createIssueInClone(t, cloneA, "Identical issue") createIssueInClone(t, cloneB, "Identical issue") - + // Sync both clones once (hash IDs handle dedup automatically) for _, clone := range []string{cloneA, cloneB} { runCmdOutputWithEnvAllowError(t, clone, map[string]string{"BEADS_NO_DAEMON": "1"}, true, bdPath, "sync") } - + // Verify both clones have exactly 1 issue (deduplication worked) for name, dir := range map[string]string{"A": cloneA, "B": cloneB} { titles := getTitlesFromClone(t, dir) @@ -168,7 +168,7 @@ func TestHashIDs_IdenticalContentDedup(t *testing.T) { t.Errorf("Clone %s missing expected issue: %v", name, sortedKeys(titles)) } } - + t.Log("✓ Identical content deduplicated correctly with hash-based IDs") } @@ -177,36 +177,36 @@ func TestHashIDs_IdenticalContentDedup(t *testing.T) { func setupBareRepo(t *testing.T, tmpDir string) string { t.Helper() remoteDir := filepath.Join(tmpDir, "remote.git") - runCmd(t, tmpDir, "git", "init", "--bare", remoteDir) - + runCmd(t, tmpDir, "git", "init", "--bare", "-b", "master", remoteDir) + tempClone := filepath.Join(tmpDir, "temp-init") runCmd(t, tmpDir, "git", "clone", remoteDir, tempClone) runCmd(t, tempClone, "git", "commit", "--allow-empty", "-m", "Initial commit") runCmd(t, tempClone, "git", "push", "origin", "master") - + return remoteDir } func setupClone(t *testing.T, tmpDir, remoteDir, name, bdPath string) string { t.Helper() cloneDir := filepath.Join(tmpDir, "clone-"+strings.ToLower(name)) - + // Use shallow, shared clones for speed runCmd(t, tmpDir, "git", "clone", "--shared", "--depth=1", "--no-tags", remoteDir, cloneDir) - + // Disable hooks to avoid overhead emptyHooks := filepath.Join(cloneDir, ".empty-hooks") os.MkdirAll(emptyHooks, 0755) runCmd(t, cloneDir, "git", "config", "core.hooksPath", emptyHooks) - + // Speed configs runCmd(t, cloneDir, "git", "config", "gc.auto", "0") runCmd(t, cloneDir, "git", "config", "core.fsync", "false") runCmd(t, cloneDir, "git", "config", "commit.gpgSign", "false") - + bdCmd := getBDCommand() copyFile(t, bdPath, filepath.Join(cloneDir, filepath.Base(bdCmd))) - + if name == "A" { runCmd(t, cloneDir, bdCmd, "init", "--quiet", "--prefix", "test") runCmd(t, cloneDir, "git", "add", ".beads") @@ -216,7 +216,7 @@ func setupClone(t *testing.T, tmpDir, remoteDir, name, bdPath string) string { runCmd(t, cloneDir, "git", "pull", "origin", "master") runCmd(t, cloneDir, bdCmd, "init", "--quiet", "--prefix", "test") } - + return cloneDir } @@ -231,13 +231,13 @@ func getTitlesFromClone(t *testing.T, cloneDir string) map[string]bool { "BEADS_NO_DAEMON": "1", "BD_NO_AUTO_IMPORT": "1", }, getBDCommand(), "list", "--json") - + jsonStart := strings.Index(listJSON, "[") if jsonStart == -1 { return make(map[string]bool) } listJSON = listJSON[jsonStart:] - + var issues []struct { Title string `json:"title"` } @@ -245,7 +245,7 @@ func getTitlesFromClone(t *testing.T, cloneDir string) map[string]bool { t.Logf("Failed to parse JSON: %v", err) return make(map[string]bool) } - + titles := make(map[string]bool) for _, issue := range issues { titles[issue.Title] = true @@ -280,7 +280,7 @@ func installGitHooks(t *testing.T, repoDir string) { hooksDir := filepath.Join(repoDir, ".git", "hooks") // Ensure POSIX-style path for sh scripts (even on Windows) bdCmd := strings.ReplaceAll(getBDCommand(), "\\", "/") - + preCommit := fmt.Sprintf(`#!/bin/sh %s --no-daemon export -o .beads/issues.jsonl >/dev/null 2>&1 || true git add .beads/issues.jsonl >/dev/null 2>&1 || true From 1a4f06ef8c1f2945f256703815d5a690cc902413 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 04:29:29 -0400 Subject: [PATCH 03/13] doctor: harden corruption repair and JSONL config Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- cmd/bd/doctor.go | 22 +++++++++ cmd/bd/doctor/config_values.go | 4 ++ cmd/bd/doctor/config_values_test.go | 15 ++++++ cmd/bd/doctor/fix/common.go | 7 +++ cmd/bd/doctor/fix/daemon.go | 3 +- cmd/bd/doctor/fix/database_config.go | 26 +++++++++- cmd/bd/doctor/fix/database_config_test.go | 50 +++++++++++++++++++ cmd/bd/doctor/fix/database_integrity.go | 60 ++++++----------------- cmd/bd/doctor/fix/hooks.go | 2 +- cmd/bd/doctor/fix/migrate.go | 7 ++- cmd/bd/doctor/fix/repo_fingerprint.go | 11 ++--- cmd/bd/doctor/fix/sync.go | 34 +++++++++---- cmd/bd/doctor/fix/sync_branch.go | 3 +- cmd/bd/doctor/legacy.go | 32 +++++++++--- cmd/bd/doctor/legacy_test.go | 43 ++++++++++++++++ cmd/bd/doctor_repair_chaos_test.go | 8 +++ cmd/bd/main.go | 17 ++++++- 17 files changed, 264 insertions(+), 80 deletions(-) diff --git a/cmd/bd/doctor.go b/cmd/bd/doctor.go index ea03cc99..f113a078 100644 --- a/cmd/bd/doctor.go +++ b/cmd/bd/doctor.go @@ -353,6 +353,28 @@ func applyFixesInteractive(path string, issues []doctorCheck) { // applyFixList applies a list of fixes and reports results func applyFixList(path string, fixes []doctorCheck) { + // Run corruption recovery before any operations that need a healthy SQLite DB. + priority := map[string]int{ + "Database Integrity": 0, + } + slices.SortStableFunc(fixes, func(a, b doctorCheck) int { + pa, oka := priority[a.Name] + if !oka { + pa = 1000 + } + pb, okb := priority[b.Name] + if !okb { + pb = 1000 + } + if pa < pb { + return -1 + } + if pa > pb { + return 1 + } + return 0 + }) + fixedCount := 0 errorCount := 0 diff --git a/cmd/bd/doctor/config_values.go b/cmd/bd/doctor/config_values.go index 45e7c995..1f81c4a0 100644 --- a/cmd/bd/doctor/config_values.go +++ b/cmd/bd/doctor/config_values.go @@ -316,6 +316,10 @@ func checkMetadataConfigValues(repoPath string) []string { // Validate jsonl_export filename if cfg.JSONLExport != "" { + switch cfg.JSONLExport { + case "deletions.jsonl", "interactions.jsonl", "molecules.jsonl": + issues = append(issues, fmt.Sprintf("metadata.json jsonl_export: %q is a system file and should not be configured as a JSONL export (expected issues.jsonl)", cfg.JSONLExport)) + } if strings.Contains(cfg.JSONLExport, string(os.PathSeparator)) || strings.Contains(cfg.JSONLExport, "/") { issues = append(issues, fmt.Sprintf("metadata.json jsonl_export: %q should be a filename, not a path", cfg.JSONLExport)) } diff --git a/cmd/bd/doctor/config_values_test.go b/cmd/bd/doctor/config_values_test.go index 04bf60ba..1fc844d8 100644 --- a/cmd/bd/doctor/config_values_test.go +++ b/cmd/bd/doctor/config_values_test.go @@ -213,6 +213,21 @@ func TestCheckMetadataConfigValues(t *testing.T) { t.Error("expected issues for wrong jsonl extension") } }) + + t.Run("jsonl_export cannot be system file", func(t *testing.T) { + metadataContent := `{ + "database": "beads.db", + "jsonl_export": "interactions.jsonl" +}` + if err := os.WriteFile(filepath.Join(beadsDir, "metadata.json"), []byte(metadataContent), 0644); err != nil { + t.Fatalf("failed to write metadata.json: %v", err) + } + + issues := checkMetadataConfigValues(tmpDir) + if len(issues) == 0 { + t.Error("expected issues for system jsonl_export") + } + }) } func contains(s, substr string) bool { diff --git a/cmd/bd/doctor/fix/common.go b/cmd/bd/doctor/fix/common.go index f7276f3b..771f38f2 100644 --- a/cmd/bd/doctor/fix/common.go +++ b/cmd/bd/doctor/fix/common.go @@ -12,6 +12,13 @@ import ( // This prevents fork bombs when tests call functions that execute bd subcommands. var ErrTestBinary = fmt.Errorf("running as test binary - cannot execute bd subcommands") +func newBdCmd(bdBinary string, args ...string) *exec.Cmd { + fullArgs := append([]string{"--no-daemon"}, args...) + cmd := exec.Command(bdBinary, fullArgs...) // #nosec G204 -- bdBinary from validated executable path + cmd.Env = append(os.Environ(), "BEADS_NO_DAEMON=1") + return cmd +} + // getBdBinary returns the path to the bd binary to use for fix operations. // It prefers the current executable to avoid command injection attacks. // Returns ErrTestBinary if running as a test binary to prevent fork bombs. diff --git a/cmd/bd/doctor/fix/daemon.go b/cmd/bd/doctor/fix/daemon.go index 79a892de..e48c41df 100644 --- a/cmd/bd/doctor/fix/daemon.go +++ b/cmd/bd/doctor/fix/daemon.go @@ -3,7 +3,6 @@ package fix import ( "fmt" "os" - "os/exec" "path/filepath" ) @@ -36,7 +35,7 @@ func Daemon(path string) error { } // Run bd daemons killall to clean up stale daemons - cmd := exec.Command(bdBinary, "daemons", "killall") // #nosec G204 -- bdBinary from validated executable path + cmd := newBdCmd(bdBinary, "daemons", "killall") cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/cmd/bd/doctor/fix/database_config.go b/cmd/bd/doctor/fix/database_config.go index 2c8bc539..34d9686d 100644 --- a/cmd/bd/doctor/fix/database_config.go +++ b/cmd/bd/doctor/fix/database_config.go @@ -32,6 +32,13 @@ func DatabaseConfig(path string) error { fixed := false + // Never treat system JSONL files as a JSONL export configuration. + if isSystemJSONLFilename(cfg.JSONLExport) { + fmt.Printf(" Updating jsonl_export: %s → issues.jsonl\n", cfg.JSONLExport) + cfg.JSONLExport = "issues.jsonl" + fixed = true + } + // Check if configured JSONL exists if cfg.JSONLExport != "" { jsonlPath := cfg.JSONLPath(beadsDir) @@ -99,7 +106,15 @@ func findActualJSONLFile(beadsDir string) string { strings.Contains(lowerName, ".orig") || strings.Contains(lowerName, ".bak") || strings.Contains(lowerName, "~") || - strings.HasPrefix(lowerName, "backup_") { + strings.HasPrefix(lowerName, "backup_") || + // System files are not JSONL exports. + name == "deletions.jsonl" || + name == "interactions.jsonl" || + name == "molecules.jsonl" || + // Git merge conflict artifacts (e.g., issues.base.jsonl, issues.left.jsonl) + strings.Contains(lowerName, ".base.jsonl") || + strings.Contains(lowerName, ".left.jsonl") || + strings.Contains(lowerName, ".right.jsonl") { continue } @@ -121,6 +136,15 @@ func findActualJSONLFile(beadsDir string) string { return candidates[0] } +func isSystemJSONLFilename(name string) bool { + switch name { + case "deletions.jsonl", "interactions.jsonl", "molecules.jsonl": + return true + default: + return false + } +} + // LegacyJSONLConfig migrates from legacy beads.jsonl to canonical issues.jsonl. // This renames the file, updates metadata.json, and updates .gitattributes if present. // bd-6xd: issues.jsonl is the canonical filename diff --git a/cmd/bd/doctor/fix/database_config_test.go b/cmd/bd/doctor/fix/database_config_test.go index 42f2642b..5ae00a2a 100644 --- a/cmd/bd/doctor/fix/database_config_test.go +++ b/cmd/bd/doctor/fix/database_config_test.go @@ -220,3 +220,53 @@ func TestLegacyJSONLConfig_UpdatesGitattributes(t *testing.T) { t.Errorf("Expected .gitattributes to reference issues.jsonl, got: %q", string(content)) } } + +// TestFindActualJSONLFile_SkipsSystemFiles ensures system JSONL files are never treated as JSONL exports. +func TestFindActualJSONLFile_SkipsSystemFiles(t *testing.T) { + tmpDir := t.TempDir() + + // Only system files → no candidates. + if err := os.WriteFile(filepath.Join(tmpDir, "interactions.jsonl"), []byte(`{"id":"x"}`), 0644); err != nil { + t.Fatal(err) + } + if got := findActualJSONLFile(tmpDir); got != "" { + t.Fatalf("expected empty result, got %q", got) + } + + // System + legacy export → legacy wins. + if err := os.WriteFile(filepath.Join(tmpDir, "beads.jsonl"), []byte(`{"id":"x"}`), 0644); err != nil { + t.Fatal(err) + } + if got := findActualJSONLFile(tmpDir); got != "beads.jsonl" { + t.Fatalf("expected beads.jsonl, got %q", got) + } +} + +func TestDatabaseConfigFix_RejectsSystemJSONLExport(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.Mkdir(beadsDir, 0755); err != nil { + t.Fatalf("Failed to create .beads dir: %v", err) + } + + if err := os.WriteFile(filepath.Join(beadsDir, "interactions.jsonl"), []byte(`{"id":"x"}`), 0644); err != nil { + t.Fatalf("Failed to create interactions.jsonl: %v", err) + } + + cfg := &configfile.Config{Database: "beads.db", JSONLExport: "interactions.jsonl"} + if err := cfg.Save(beadsDir); err != nil { + t.Fatalf("Failed to save config: %v", err) + } + + if err := DatabaseConfig(tmpDir); err != nil { + t.Fatalf("DatabaseConfig failed: %v", err) + } + + updated, err := configfile.Load(beadsDir) + if err != nil { + t.Fatalf("Failed to load updated config: %v", err) + } + if updated.JSONLExport != "issues.jsonl" { + t.Fatalf("expected issues.jsonl, got %q", updated.JSONLExport) + } +} diff --git a/cmd/bd/doctor/fix/database_integrity.go b/cmd/bd/doctor/fix/database_integrity.go index 8c8e39c9..5791ae11 100644 --- a/cmd/bd/doctor/fix/database_integrity.go +++ b/cmd/bd/doctor/fix/database_integrity.go @@ -1,22 +1,18 @@ package fix import ( - "bufio" - "encoding/json" "fmt" "os" - "os/exec" "path/filepath" "time" "github.com/steveyegge/beads/internal/beads" "github.com/steveyegge/beads/internal/configfile" - "github.com/steveyegge/beads/internal/utils" ) // DatabaseIntegrity attempts to recover from database corruption by: // 1. Backing up the corrupt database (and WAL/SHM if present) -// 2. Re-initializing the database from the git-tracked JSONL export +// 2. Re-initializing the database from the working tree JSONL export // // This is intentionally conservative: it will not delete JSONL, and it preserves the // original DB as a backup for forensic recovery. @@ -74,61 +70,35 @@ func DatabaseIntegrity(path string) error { } } - // Rebuild via bd init, pointing at the same db path. + // Rebuild by importing from the working tree JSONL into a fresh database. bdBinary, err := getBdBinary() if err != nil { return err } - args := []string{"--db", dbPath, "init", "--quiet", "--force", "--skip-hooks", "--skip-merge-driver"} - if prefix := detectPrefixFromJSONL(jsonlPath); prefix != "" { - args = append(args, "--prefix", prefix) - } - - cmd := exec.Command(bdBinary, args...) // #nosec G204 -- bdBinary is a validated executable path + // Use import (not init) so we always hydrate from the working tree JSONL, not git-tracked blobs. + args := []string{"--db", dbPath, "import", "-i", jsonlPath, "--force", "--no-git-history"} + cmd := newBdCmd(bdBinary, args...) cmd.Dir = absPath cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - cmd.Env = os.Environ() if err := cmd.Run(); err != nil { - // Best-effort rollback: if init didn't recreate the db, restore the backup. - if _, statErr := os.Stat(dbPath); os.IsNotExist(statErr) { - _ = os.Rename(backupDB, dbPath) + // Best-effort rollback: attempt to restore the backup, preserving any partial init output. + failedTS := time.Now().UTC().Format("20060102T150405Z") + if _, statErr := os.Stat(dbPath); statErr == nil { + failedDB := dbPath + "." + failedTS + ".failed.init.db" + _ = os.Rename(dbPath, failedDB) for _, suffix := range []string{"-wal", "-shm", "-journal"} { - _ = os.Rename(backupDB+suffix, dbPath+suffix) + _ = os.Rename(dbPath+suffix, failedDB+suffix) } } + _ = os.Rename(backupDB, dbPath) + for _, suffix := range []string{"-wal", "-shm", "-journal"} { + _ = os.Rename(backupDB+suffix, dbPath+suffix) + } return fmt.Errorf("failed to rebuild database from JSONL: %w (backup: %s)", err, backupDB) } return nil } - -func detectPrefixFromJSONL(jsonlPath string) string { - f, err := os.Open(jsonlPath) // #nosec G304 -- jsonlPath is within the workspace - if err != nil { - return "" - } - defer f.Close() - - scanner := bufio.NewScanner(f) - for scanner.Scan() { - line := scanner.Bytes() - if len(line) == 0 { - continue - } - var issue struct { - ID string `json:"id"` - } - if err := json.Unmarshal(line, &issue); err != nil { - continue - } - if issue.ID == "" { - continue - } - return utils.ExtractIssuePrefix(issue.ID) - } - - return "" -} diff --git a/cmd/bd/doctor/fix/hooks.go b/cmd/bd/doctor/fix/hooks.go index 12cc67fc..d46131b1 100644 --- a/cmd/bd/doctor/fix/hooks.go +++ b/cmd/bd/doctor/fix/hooks.go @@ -28,7 +28,7 @@ func GitHooks(path string) error { } // Run bd hooks install - cmd := exec.Command(bdBinary, "hooks", "install") // #nosec G204 -- bdBinary from validated executable path + cmd := newBdCmd(bdBinary, "hooks", "install") cmd.Dir = path // Set working directory without changing process dir cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/cmd/bd/doctor/fix/migrate.go b/cmd/bd/doctor/fix/migrate.go index f03d112f..b9eca8af 100644 --- a/cmd/bd/doctor/fix/migrate.go +++ b/cmd/bd/doctor/fix/migrate.go @@ -3,7 +3,6 @@ package fix import ( "fmt" "os" - "os/exec" "path/filepath" ) @@ -28,7 +27,7 @@ func DatabaseVersion(path string) error { if _, err := os.Stat(dbPath); os.IsNotExist(err) { // No database - this is a fresh clone, run bd init fmt.Println("→ No database found, running 'bd init' to hydrate from JSONL...") - cmd := exec.Command(bdBinary, "init") // #nosec G204 -- bdBinary from validated executable path + cmd := newBdCmd(bdBinary, "init") cmd.Dir = path cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -41,8 +40,8 @@ func DatabaseVersion(path string) error { } // Database exists - run bd migrate - cmd := exec.Command(bdBinary, "migrate") // #nosec G204 -- bdBinary from validated executable path - cmd.Dir = path // Set working directory without changing process dir + cmd := newBdCmd(bdBinary, "migrate") + cmd.Dir = path // Set working directory without changing process dir cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/cmd/bd/doctor/fix/repo_fingerprint.go b/cmd/bd/doctor/fix/repo_fingerprint.go index 3a689071..4ca9644c 100644 --- a/cmd/bd/doctor/fix/repo_fingerprint.go +++ b/cmd/bd/doctor/fix/repo_fingerprint.go @@ -3,7 +3,6 @@ package fix import ( "fmt" "os" - "os/exec" "path/filepath" "strings" ) @@ -31,9 +30,9 @@ func readLineUnbuffered() (string, error) { // RepoFingerprint fixes repo fingerprint mismatches by prompting the user // for which action to take. This is interactive because the consequences // differ significantly between options: -// 1. Update repo ID (if URL changed or bd upgraded) -// 2. Reinitialize database (if wrong database was copied) -// 3. Skip (do nothing) +// 1. Update repo ID (if URL changed or bd upgraded) +// 2. Reinitialize database (if wrong database was copied) +// 3. Skip (do nothing) func RepoFingerprint(path string) error { // Validate workspace if err := validateBeadsWorkspace(path); err != nil { @@ -67,7 +66,7 @@ func RepoFingerprint(path string) error { case "1": // Run bd migrate --update-repo-id fmt.Println(" → Running 'bd migrate --update-repo-id'...") - cmd := exec.Command(bdBinary, "migrate", "--update-repo-id") // #nosec G204 -- bdBinary from validated executable path + cmd := newBdCmd(bdBinary, "migrate", "--update-repo-id") cmd.Dir = path cmd.Stdin = os.Stdin // Allow user to respond to migrate's confirmation prompt cmd.Stdout = os.Stdout @@ -105,7 +104,7 @@ func RepoFingerprint(path string) error { _ = os.Remove(dbPath + "-shm") fmt.Println(" → Running 'bd init'...") - cmd := exec.Command(bdBinary, "init", "--quiet") // #nosec G204 -- bdBinary from validated executable path + cmd := newBdCmd(bdBinary, "init", "--quiet") cmd.Dir = path cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/cmd/bd/doctor/fix/sync.go b/cmd/bd/doctor/fix/sync.go index 4024cce6..6801f762 100644 --- a/cmd/bd/doctor/fix/sync.go +++ b/cmd/bd/doctor/fix/sync.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "os" - "os/exec" "path/filepath" _ "github.com/ncruces/go-sqlite3/driver" @@ -102,21 +101,36 @@ func DBJSONLSync(path string) error { return err } - // Run the appropriate sync command - var cmd *exec.Cmd if syncDirection == "export" { // Export DB to JSONL file (must specify -o to write to file, not stdout) jsonlOutputPath := filepath.Join(beadsDir, "issues.jsonl") - cmd = exec.Command(bdBinary, "export", "-o", jsonlOutputPath, "--force") // #nosec G204 -- bdBinary from validated executable path - } else { - cmd = exec.Command(bdBinary, "sync", "--import-only") // #nosec G204 -- bdBinary from validated executable path + exportCmd := newBdCmd(bdBinary, "export", "-o", jsonlOutputPath, "--force") + exportCmd.Dir = path // Set working directory without changing process dir + exportCmd.Stdout = os.Stdout + exportCmd.Stderr = os.Stderr + if err := exportCmd.Run(); err != nil { + return fmt.Errorf("failed to export database to JSONL: %w", err) + } + + // Staleness check uses last_import_time. After exporting, JSONL mtime is newer, + // so mark the DB as fresh by running a no-op import (skip existing issues). + markFreshCmd := newBdCmd(bdBinary, "import", "-i", jsonlOutputPath, "--force", "--skip-existing", "--no-git-history") + markFreshCmd.Dir = path + markFreshCmd.Stdout = os.Stdout + markFreshCmd.Stderr = os.Stderr + if err := markFreshCmd.Run(); err != nil { + return fmt.Errorf("failed to mark database as fresh after export: %w", err) + } + + return nil } - cmd.Dir = path // Set working directory without changing process dir - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + importCmd := newBdCmd(bdBinary, "sync", "--import-only") + importCmd.Dir = path // Set working directory without changing process dir + importCmd.Stdout = os.Stdout + importCmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { + if err := importCmd.Run(); err != nil { return fmt.Errorf("failed to sync database with JSONL: %w", err) } diff --git a/cmd/bd/doctor/fix/sync_branch.go b/cmd/bd/doctor/fix/sync_branch.go index 06a2388d..88ac1bcc 100644 --- a/cmd/bd/doctor/fix/sync_branch.go +++ b/cmd/bd/doctor/fix/sync_branch.go @@ -32,8 +32,7 @@ func SyncBranchConfig(path string) error { } // Set sync.branch using bd config set - // #nosec G204 - bdBinary is controlled by getBdBinary() which returns os.Executable() - setCmd := exec.Command(bdBinary, "config", "set", "sync.branch", currentBranch) + setCmd := newBdCmd(bdBinary, "config", "set", "sync.branch", currentBranch) setCmd.Dir = path if output, err := setCmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to set sync.branch: %w\nOutput: %s", err, string(output)) diff --git a/cmd/bd/doctor/legacy.go b/cmd/bd/doctor/legacy.go index 589d3b9b..0098afd5 100644 --- a/cmd/bd/doctor/legacy.go +++ b/cmd/bd/doctor/legacy.go @@ -53,7 +53,7 @@ func CheckLegacyBeadsSlashCommands(repoPath string) DoctorCheck { Name: "Legacy Commands", Status: "warning", Message: fmt.Sprintf("Old beads integration detected in %s", strings.Join(filesWithLegacyCommands, ", ")), - Detail: "Found: /beads:* slash command references (deprecated)\n" + + Detail: "Found: /beads:* slash command references (deprecated)\n" + " These commands are token-inefficient (~10.5k tokens per session)", Fix: "Migrate to bd prime hooks for better token efficiency:\n" + "\n" + @@ -104,7 +104,7 @@ func CheckAgentDocumentation(repoPath string) DoctorCheck { Name: "Agent Documentation", Status: "warning", Message: "No agent documentation found", - Detail: "Missing: AGENTS.md or CLAUDE.md\n" + + Detail: "Missing: AGENTS.md or CLAUDE.md\n" + " Documenting workflow helps AI agents work more effectively", Fix: "Add agent documentation:\n" + " • Run 'bd onboard' to create AGENTS.md with workflow guidance\n" + @@ -187,7 +187,7 @@ func CheckLegacyJSONLFilename(repoPath string) DoctorCheck { Name: "JSONL Files", Status: "warning", Message: fmt.Sprintf("Multiple JSONL files found: %s", strings.Join(realJSONLFiles, ", ")), - Detail: "Having multiple JSONL files can cause sync and merge conflicts.\n" + + Detail: "Having multiple JSONL files can cause sync and merge conflicts.\n" + " Only one JSONL file should be used per repository.", Fix: "Determine which file is current and remove the others:\n" + " 1. Check 'bd stats' to see which file is being used\n" + @@ -235,7 +235,7 @@ func CheckLegacyJSONLConfig(repoPath string) DoctorCheck { Name: "JSONL Config", Status: "warning", Message: "Using legacy beads.jsonl filename", - Detail: "The canonical filename is now issues.jsonl (bd-6xd).\n" + + Detail: "The canonical filename is now issues.jsonl (bd-6xd).\n" + " Legacy beads.jsonl is still supported but should be migrated.", Fix: "Run 'bd doctor --fix' to auto-migrate, or manually:\n" + " 1. git mv .beads/beads.jsonl .beads/issues.jsonl\n" + @@ -251,7 +251,7 @@ func CheckLegacyJSONLConfig(repoPath string) DoctorCheck { Status: "warning", Message: "Config references beads.jsonl but issues.jsonl exists", Detail: "metadata.json says beads.jsonl but the actual file is issues.jsonl", - Fix: "Run 'bd doctor --fix' to update the configuration", + Fix: "Run 'bd doctor --fix' to update the configuration", } } } @@ -303,6 +303,16 @@ func CheckDatabaseConfig(repoPath string) DoctorCheck { // Check if configured JSONL exists if cfg.JSONLExport != "" { + if cfg.JSONLExport == "deletions.jsonl" || cfg.JSONLExport == "interactions.jsonl" || cfg.JSONLExport == "molecules.jsonl" { + return DoctorCheck{ + Name: "Database Config", + Status: "error", + Message: fmt.Sprintf("Invalid jsonl_export %q (system file)", cfg.JSONLExport), + Detail: "metadata.json jsonl_export must reference the git-tracked issues export (typically issues.jsonl), not a system log file.", + Fix: "Run 'bd doctor --fix' to reset metadata.json jsonl_export to issues.jsonl, then commit the change.", + } + } + jsonlPath := cfg.JSONLPath(beadsDir) if _, err := os.Stat(jsonlPath); os.IsNotExist(err) { // Check if other .jsonl files exist @@ -315,7 +325,15 @@ func CheckDatabaseConfig(repoPath string) DoctorCheck { lowerName := strings.ToLower(name) if !strings.Contains(lowerName, "backup") && !strings.Contains(lowerName, ".orig") && - !strings.Contains(lowerName, ".bak") { + !strings.Contains(lowerName, ".bak") && + !strings.Contains(lowerName, "~") && + !strings.HasPrefix(lowerName, "backup_") && + name != "deletions.jsonl" && + name != "interactions.jsonl" && + name != "molecules.jsonl" && + !strings.Contains(lowerName, ".base.jsonl") && + !strings.Contains(lowerName, ".left.jsonl") && + !strings.Contains(lowerName, ".right.jsonl") { otherJSONLs = append(otherJSONLs, name) } } @@ -421,7 +439,7 @@ func CheckFreshClone(repoPath string) DoctorCheck { Name: "Fresh Clone", Status: "warning", Message: fmt.Sprintf("Fresh clone detected (%d issues in %s, no database)", issueCount, jsonlName), - Detail: "This appears to be a freshly cloned repository.\n" + + Detail: "This appears to be a freshly cloned repository.\n" + " The JSONL file contains issues but no local database exists.\n" + " Run 'bd init' to create the database and import existing issues.", Fix: fmt.Sprintf("Run '%s' to initialize the database and import issues", fixCmd), diff --git a/cmd/bd/doctor/legacy_test.go b/cmd/bd/doctor/legacy_test.go index 241c9d75..9c5fb49d 100644 --- a/cmd/bd/doctor/legacy_test.go +++ b/cmd/bd/doctor/legacy_test.go @@ -410,6 +410,49 @@ func TestCheckLegacyJSONLConfig(t *testing.T) { } } +func TestCheckDatabaseConfig_IgnoresSystemJSONLs(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.Mkdir(beadsDir, 0750); err != nil { + t.Fatal(err) + } + + // Configure issues.jsonl, but only create interactions.jsonl. + metadataPath := filepath.Join(beadsDir, "metadata.json") + if err := os.WriteFile(metadataPath, []byte(`{"database":"beads.db","jsonl_export":"issues.jsonl"}`), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(beadsDir, "interactions.jsonl"), []byte(`{"id":"x"}`), 0644); err != nil { + t.Fatal(err) + } + + check := CheckDatabaseConfig(tmpDir) + if check.Status != "ok" { + t.Fatalf("expected ok, got %s: %s\n%s", check.Status, check.Message, check.Detail) + } +} + +func TestCheckDatabaseConfig_SystemJSONLExportIsError(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.Mkdir(beadsDir, 0750); err != nil { + t.Fatal(err) + } + + metadataPath := filepath.Join(beadsDir, "metadata.json") + if err := os.WriteFile(metadataPath, []byte(`{"database":"beads.db","jsonl_export":"interactions.jsonl"}`), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(beadsDir, "interactions.jsonl"), []byte(`{"id":"x"}`), 0644); err != nil { + t.Fatal(err) + } + + check := CheckDatabaseConfig(tmpDir) + if check.Status != "error" { + t.Fatalf("expected error, got %s: %s", check.Status, check.Message) + } +} + func TestCheckFreshClone(t *testing.T) { tests := []struct { name string diff --git a/cmd/bd/doctor_repair_chaos_test.go b/cmd/bd/doctor_repair_chaos_test.go index 792327d4..e9d9ee7f 100644 --- a/cmd/bd/doctor_repair_chaos_test.go +++ b/cmd/bd/doctor_repair_chaos_test.go @@ -67,6 +67,14 @@ func TestDoctorRepair_CorruptDatabase_NoJSONL_FixFails(t *testing.T) { if !strings.Contains(out, "cannot auto-recover") { t.Fatalf("expected auto-recover error, got:\n%s", out) } + + // Ensure we don't mis-configure jsonl_export to a system file during failure. + metadata, readErr := os.ReadFile(filepath.Join(ws, ".beads", "metadata.json")) + if readErr == nil { + if strings.Contains(string(metadata), "interactions.jsonl") { + t.Fatalf("unexpected metadata.json jsonl_export set to interactions.jsonl:\n%s", string(metadata)) + } + } } func TestDoctorRepair_CorruptDatabase_BacksUpSidecars(t *testing.T) { diff --git a/cmd/bd/main.go b/cmd/bd/main.go index 12f27cf5..49f220c8 100644 --- a/cmd/bd/main.go +++ b/cmd/bd/main.go @@ -623,6 +623,13 @@ var rootCmd = &cobra.Command{ FallbackReason: FallbackNone, } + // Doctor should always run in direct mode. It's specifically used to diagnose and + // repair daemon/DB issues, so attempting to connect to (or auto-start) a daemon + // can add noise and timeouts. + if cmd.Name() == "doctor" { + noDaemon = true + } + // Try to connect to daemon first (unless --no-daemon flag is set or worktree safety check fails) if noDaemon { daemonStatus.FallbackReason = FallbackFlagNoDaemon @@ -917,8 +924,14 @@ var rootCmd = &cobra.Command{ if store != nil { _ = store.Close() } - if profileFile != nil { pprof.StopCPUProfile(); _ = profileFile.Close() } - if traceFile != nil { trace.Stop(); _ = traceFile.Close() } + if profileFile != nil { + pprof.StopCPUProfile() + _ = profileFile.Close() + } + if traceFile != nil { + trace.Stop() + _ = traceFile.Close() + } // Cancel the signal context to clean up resources if rootCancel != nil { From 8166207eb4739cdbab03c8870971652f2d33f0d0 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 08:18:25 -0400 Subject: [PATCH 04/13] doctor: add JSONL integrity check/fix and harden repairs Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- cmd/bd/doctor.go | 29 +++- cmd/bd/doctor/database.go | 36 +++-- cmd/bd/doctor/fix/database_integrity.go | 26 +++- cmd/bd/doctor/fix/jsonl_integrity.go | 105 +++++++++++++++ cmd/bd/doctor/fix/migrate.go | 12 +- cmd/bd/doctor/fix/sync.go | 30 +++-- cmd/bd/doctor/jsonl_integrity.go | 123 +++++++++++++++++ cmd/bd/doctor/jsonl_integrity_test.go | 43 ++++++ cmd/bd/doctor_repair_chaos_test.go | 172 ++++++++++++++++++++++++ cmd/bd/test_repo_beads_guard_test.go | 118 ++++++++++++++++ 10 files changed, 663 insertions(+), 31 deletions(-) create mode 100644 cmd/bd/doctor/fix/jsonl_integrity.go create mode 100644 cmd/bd/doctor/jsonl_integrity.go create mode 100644 cmd/bd/doctor/jsonl_integrity_test.go create mode 100644 cmd/bd/test_repo_beads_guard_test.go diff --git a/cmd/bd/doctor.go b/cmd/bd/doctor.go index f113a078..90fcf541 100644 --- a/cmd/bd/doctor.go +++ b/cmd/bd/doctor.go @@ -353,9 +353,23 @@ func applyFixesInteractive(path string, issues []doctorCheck) { // applyFixList applies a list of fixes and reports results func applyFixList(path string, fixes []doctorCheck) { - // Run corruption recovery before any operations that need a healthy SQLite DB. - priority := map[string]int{ - "Database Integrity": 0, + // Apply fixes in a dependency-aware order. + // Rough dependency chain: + // permissions/daemon cleanup → config sanity → DB integrity/migrations → DB↔JSONL sync. + order := []string{ + "Permissions", + "Daemon Health", + "Database Config", + "JSONL Config", + "Database Integrity", + "Database", + "Schema Compatibility", + "JSONL Integrity", + "DB-JSONL Sync", + } + priority := make(map[string]int, len(order)) + for i, name := range order { + priority[name] = i } slices.SortStableFunc(fixes, func(a, b doctorCheck) int { pa, oka := priority[a.Name] @@ -411,6 +425,8 @@ func applyFixList(path string, fixes []doctorCheck) { err = fix.DatabaseConfig(path) case "JSONL Config": err = fix.LegacyJSONLConfig(path) + case "JSONL Integrity": + err = fix.JSONLIntegrity(path) case "Deletions Manifest": err = fix.MigrateTombstones(path) case "Untracked Files": @@ -711,6 +727,13 @@ func runDiagnostics(path string) doctorResult { result.Checks = append(result.Checks, configValuesCheck) // Don't fail overall check for config value warnings, just warn + // Check 7b: JSONL integrity (malformed lines, missing IDs) + jsonlIntegrityCheck := convertWithCategory(doctor.CheckJSONLIntegrity(path), doctor.CategoryData) + result.Checks = append(result.Checks, jsonlIntegrityCheck) + if jsonlIntegrityCheck.Status == statusWarning || jsonlIntegrityCheck.Status == statusError { + result.OverallOK = false + } + // Check 8: Daemon health daemonCheck := convertWithCategory(doctor.CheckDaemonStatus(path, Version), doctor.CategoryRuntime) result.Checks = append(result.Checks, daemonCheck) diff --git a/cmd/bd/doctor/database.go b/cmd/bd/doctor/database.go index 0ecf4492..d0952984 100644 --- a/cmd/bd/doctor/database.go +++ b/cmd/bd/doctor/database.go @@ -301,15 +301,30 @@ func CheckDatabaseIntegrity(path string) DoctorCheck { // CheckDatabaseJSONLSync checks if database and JSONL are in sync func CheckDatabaseJSONLSync(path string) DoctorCheck { beadsDir := filepath.Join(path, ".beads") - dbPath := filepath.Join(beadsDir, beads.CanonicalDatabaseName) - // Find JSONL file - var jsonlPath string - for _, name := range []string{"issues.jsonl", "beads.jsonl"} { - testPath := filepath.Join(beadsDir, name) - if _, err := os.Stat(testPath); err == nil { - jsonlPath = testPath - break + // Resolve database path (respects metadata.json override). + dbPath := filepath.Join(beadsDir, beads.CanonicalDatabaseName) + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil && cfg.Database != "" { + dbPath = cfg.DatabasePath(beadsDir) + } + + // Find JSONL file (respects metadata.json override when set). + jsonlPath := "" + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil { + if cfg.JSONLExport != "" && !isSystemJSONLFilename(cfg.JSONLExport) { + p := cfg.JSONLPath(beadsDir) + if _, err := os.Stat(p); err == nil { + jsonlPath = p + } + } + } + if jsonlPath == "" { + for _, name := range []string{"issues.jsonl", "beads.jsonl"} { + testPath := filepath.Join(beadsDir, name) + if _, err := os.Stat(testPath); err == nil { + jsonlPath = testPath + break + } } } @@ -392,11 +407,16 @@ func CheckDatabaseJSONLSync(path string) DoctorCheck { // Use JSONL error if we got it earlier if jsonlErr != nil { + fixMsg := "Run 'bd doctor --fix' to attempt recovery" + if strings.Contains(jsonlErr.Error(), "malformed") { + fixMsg = "Run 'bd doctor --fix' to back up and regenerate the JSONL from the database" + } return DoctorCheck{ Name: "DB-JSONL Sync", Status: StatusWarning, Message: "Unable to read JSONL file", Detail: jsonlErr.Error(), + Fix: fixMsg, } } diff --git a/cmd/bd/doctor/fix/database_integrity.go b/cmd/bd/doctor/fix/database_integrity.go index 5791ae11..aadcbd05 100644 --- a/cmd/bd/doctor/fix/database_integrity.go +++ b/cmd/bd/doctor/fix/database_integrity.go @@ -28,6 +28,9 @@ func DatabaseIntegrity(path string) error { beadsDir := filepath.Join(absPath, ".beads") + // Best-effort: stop any running daemon to reduce the chance of DB file locks. + _ = Daemon(absPath) + // Resolve database path (respects metadata.json database override). var dbPath string if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil && cfg.Database != "" { @@ -39,9 +42,11 @@ func DatabaseIntegrity(path string) error { // Find JSONL source of truth. jsonlPath := "" if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil { - candidate := cfg.JSONLPath(beadsDir) - if _, err := os.Stat(candidate); err == nil { - jsonlPath = candidate + if cfg.JSONLExport != "" && !isSystemJSONLFilename(cfg.JSONLExport) { + candidate := cfg.JSONLPath(beadsDir) + if _, err := os.Stat(candidate); err == nil { + jsonlPath = candidate + } } } if jsonlPath == "" { @@ -61,7 +66,12 @@ func DatabaseIntegrity(path string) error { ts := time.Now().UTC().Format("20060102T150405Z") backupDB := dbPath + "." + ts + ".corrupt.backup.db" if err := os.Rename(dbPath, backupDB); err != nil { - return fmt.Errorf("failed to back up database: %w", err) + // Retry once after attempting to kill daemons again (helps on platforms with strict file locks). + _ = Daemon(absPath) + if err2 := os.Rename(dbPath, backupDB); err2 != nil { + // Prefer the original error (more likely root cause). + return fmt.Errorf("failed to back up database: %w", err) + } } for _, suffix := range []string{"-wal", "-shm", "-journal"} { sidecar := dbPath + suffix @@ -84,7 +94,7 @@ func DatabaseIntegrity(path string) error { cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { - // Best-effort rollback: attempt to restore the backup, preserving any partial init output. + // Best-effort rollback: attempt to restore the original DB, while preserving the backup. failedTS := time.Now().UTC().Format("20060102T150405Z") if _, statErr := os.Stat(dbPath); statErr == nil { failedDB := dbPath + "." + failedTS + ".failed.init.db" @@ -93,9 +103,11 @@ func DatabaseIntegrity(path string) error { _ = os.Rename(dbPath+suffix, failedDB+suffix) } } - _ = os.Rename(backupDB, dbPath) + _ = copyFile(backupDB, dbPath) for _, suffix := range []string{"-wal", "-shm", "-journal"} { - _ = os.Rename(backupDB+suffix, dbPath+suffix) + if _, statErr := os.Stat(backupDB + suffix); statErr == nil { + _ = copyFile(backupDB+suffix, dbPath+suffix) + } } return fmt.Errorf("failed to rebuild database from JSONL: %w (backup: %s)", err, backupDB) } diff --git a/cmd/bd/doctor/fix/jsonl_integrity.go b/cmd/bd/doctor/fix/jsonl_integrity.go new file mode 100644 index 00000000..20f14923 --- /dev/null +++ b/cmd/bd/doctor/fix/jsonl_integrity.go @@ -0,0 +1,105 @@ +package fix + +import ( + "fmt" + "io" + "os" + "path/filepath" + "time" + + "github.com/steveyegge/beads/internal/beads" + "github.com/steveyegge/beads/internal/configfile" + "github.com/steveyegge/beads/internal/utils" +) + +// JSONLIntegrity backs up a malformed JSONL export and regenerates it from the database. +// This is safe only when a database exists and is readable. +func JSONLIntegrity(path string) error { + if err := validateBeadsWorkspace(path); err != nil { + return err + } + + absPath, err := filepath.Abs(path) + if err != nil { + return fmt.Errorf("failed to resolve path: %w", err) + } + + beadsDir := filepath.Join(absPath, ".beads") + + // Resolve db path. + dbPath := filepath.Join(beadsDir, beads.CanonicalDatabaseName) + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil && cfg.Database != "" { + dbPath = cfg.DatabasePath(beadsDir) + } + if _, err := os.Stat(dbPath); os.IsNotExist(err) { + return fmt.Errorf("cannot auto-repair JSONL: no database found") + } + + // Resolve JSONL export path. + jsonlPath := "" + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil { + if cfg.JSONLExport != "" && !isSystemJSONLFilename(cfg.JSONLExport) { + p := cfg.JSONLPath(beadsDir) + if _, err := os.Stat(p); err == nil { + jsonlPath = p + } + } + } + if jsonlPath == "" { + p := utils.FindJSONLInDir(beadsDir) + if _, err := os.Stat(p); err == nil { + jsonlPath = p + } + } + if jsonlPath == "" { + return fmt.Errorf("cannot auto-repair JSONL: no JSONL file found") + } + + // Back up the JSONL. + ts := time.Now().UTC().Format("20060102T150405Z") + backup := jsonlPath + "." + ts + ".corrupt.backup.jsonl" + if err := os.Rename(jsonlPath, backup); err != nil { + return fmt.Errorf("failed to back up JSONL: %w", err) + } + + binary, err := getBdBinary() + if err != nil { + _ = os.Rename(backup, jsonlPath) + return err + } + + // Re-export from DB. + cmd := newBdCmd(binary, "--db", dbPath, "export", "-o", jsonlPath, "--force") + cmd.Dir = absPath + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := cmd.Run(); err != nil { + // Best-effort rollback: restore the original JSONL, but keep the backup. + failedTS := time.Now().UTC().Format("20060102T150405Z") + if _, statErr := os.Stat(jsonlPath); statErr == nil { + failed := jsonlPath + "." + failedTS + ".failed.regen.jsonl" + _ = os.Rename(jsonlPath, failed) + } + _ = copyFile(backup, jsonlPath) + return fmt.Errorf("failed to regenerate JSONL from database: %w (backup: %s)", err, backup) + } + + return nil +} + +func copyFile(src, dst string) error { + in, err := os.Open(src) // #nosec G304 -- src is within the workspace + if err != nil { + return err + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) + if err != nil { + return err + } + defer func() { _ = out.Close() }() + if _, err := io.Copy(out, in); err != nil { + return err + } + return out.Close() +} diff --git a/cmd/bd/doctor/fix/migrate.go b/cmd/bd/doctor/fix/migrate.go index b9eca8af..2c20abb4 100644 --- a/cmd/bd/doctor/fix/migrate.go +++ b/cmd/bd/doctor/fix/migrate.go @@ -4,6 +4,9 @@ import ( "fmt" "os" "path/filepath" + + "github.com/steveyegge/beads/internal/beads" + "github.com/steveyegge/beads/internal/configfile" ) // DatabaseVersion fixes database version mismatches by running bd migrate, @@ -22,12 +25,15 @@ func DatabaseVersion(path string) error { // Check if database exists - if not, run init instead of migrate (bd-4h9) beadsDir := filepath.Join(path, ".beads") - dbPath := filepath.Join(beadsDir, "beads.db") + dbPath := filepath.Join(beadsDir, beads.CanonicalDatabaseName) + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil && cfg.Database != "" { + dbPath = cfg.DatabasePath(beadsDir) + } if _, err := os.Stat(dbPath); os.IsNotExist(err) { // No database - this is a fresh clone, run bd init fmt.Println("→ No database found, running 'bd init' to hydrate from JSONL...") - cmd := newBdCmd(bdBinary, "init") + cmd := newBdCmd(bdBinary, "--db", dbPath, "init") cmd.Dir = path cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -40,7 +46,7 @@ func DatabaseVersion(path string) error { } // Database exists - run bd migrate - cmd := newBdCmd(bdBinary, "migrate") + cmd := newBdCmd(bdBinary, "--db", dbPath, "migrate") cmd.Dir = path // Set working directory without changing process dir cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/cmd/bd/doctor/fix/sync.go b/cmd/bd/doctor/fix/sync.go index 6801f762..dfc3a1e3 100644 --- a/cmd/bd/doctor/fix/sync.go +++ b/cmd/bd/doctor/fix/sync.go @@ -37,13 +37,23 @@ func DBJSONLSync(path string) error { // Find JSONL file var jsonlPath string - issuesJSONL := filepath.Join(beadsDir, "issues.jsonl") - beadsJSONL := filepath.Join(beadsDir, "beads.jsonl") + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil { + if cfg.JSONLExport != "" && !isSystemJSONLFilename(cfg.JSONLExport) { + p := cfg.JSONLPath(beadsDir) + if _, err := os.Stat(p); err == nil { + jsonlPath = p + } + } + } + if jsonlPath == "" { + issuesJSONL := filepath.Join(beadsDir, "issues.jsonl") + beadsJSONL := filepath.Join(beadsDir, "beads.jsonl") - if _, err := os.Stat(issuesJSONL); err == nil { - jsonlPath = issuesJSONL - } else if _, err := os.Stat(beadsJSONL); err == nil { - jsonlPath = beadsJSONL + if _, err := os.Stat(issuesJSONL); err == nil { + jsonlPath = issuesJSONL + } else if _, err := os.Stat(beadsJSONL); err == nil { + jsonlPath = beadsJSONL + } } // Check if both database and JSONL exist @@ -103,8 +113,8 @@ func DBJSONLSync(path string) error { if syncDirection == "export" { // Export DB to JSONL file (must specify -o to write to file, not stdout) - jsonlOutputPath := filepath.Join(beadsDir, "issues.jsonl") - exportCmd := newBdCmd(bdBinary, "export", "-o", jsonlOutputPath, "--force") + jsonlOutputPath := jsonlPath + exportCmd := newBdCmd(bdBinary, "--db", dbPath, "export", "-o", jsonlOutputPath, "--force") exportCmd.Dir = path // Set working directory without changing process dir exportCmd.Stdout = os.Stdout exportCmd.Stderr = os.Stderr @@ -114,7 +124,7 @@ func DBJSONLSync(path string) error { // Staleness check uses last_import_time. After exporting, JSONL mtime is newer, // so mark the DB as fresh by running a no-op import (skip existing issues). - markFreshCmd := newBdCmd(bdBinary, "import", "-i", jsonlOutputPath, "--force", "--skip-existing", "--no-git-history") + markFreshCmd := newBdCmd(bdBinary, "--db", dbPath, "import", "-i", jsonlOutputPath, "--force", "--skip-existing", "--no-git-history") markFreshCmd.Dir = path markFreshCmd.Stdout = os.Stdout markFreshCmd.Stderr = os.Stderr @@ -125,7 +135,7 @@ func DBJSONLSync(path string) error { return nil } - importCmd := newBdCmd(bdBinary, "sync", "--import-only") + importCmd := newBdCmd(bdBinary, "--db", dbPath, "sync", "--import-only") importCmd.Dir = path // Set working directory without changing process dir importCmd.Stdout = os.Stdout importCmd.Stderr = os.Stderr diff --git a/cmd/bd/doctor/jsonl_integrity.go b/cmd/bd/doctor/jsonl_integrity.go new file mode 100644 index 00000000..1c84f862 --- /dev/null +++ b/cmd/bd/doctor/jsonl_integrity.go @@ -0,0 +1,123 @@ +package doctor + +import ( + "bufio" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/steveyegge/beads/internal/beads" + "github.com/steveyegge/beads/internal/configfile" + "github.com/steveyegge/beads/internal/utils" +) + +func CheckJSONLIntegrity(path string) DoctorCheck { + beadsDir := filepath.Join(path, ".beads") + + // Resolve JSONL path. + jsonlPath := "" + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil { + if cfg.JSONLExport != "" && !isSystemJSONLFilename(cfg.JSONLExport) { + p := cfg.JSONLPath(beadsDir) + if _, err := os.Stat(p); err == nil { + jsonlPath = p + } + } + } + if jsonlPath == "" { + // Fall back to a best-effort discovery within .beads/. + p := utils.FindJSONLInDir(beadsDir) + if _, err := os.Stat(p); err == nil { + jsonlPath = p + } + } + if jsonlPath == "" { + return DoctorCheck{Name: "JSONL Integrity", Status: StatusOK, Message: "N/A (no JSONL file)"} + } + + // Best-effort scan for malformed lines. + f, err := os.Open(jsonlPath) // #nosec G304 -- jsonlPath is within the workspace + if err != nil { + return DoctorCheck{ + Name: "JSONL Integrity", + Status: StatusWarning, + Message: "Unable to read JSONL file", + Detail: err.Error(), + } + } + defer f.Close() + + var malformed int + var examples []string + scanner := bufio.NewScanner(f) + lineNo := 0 + for scanner.Scan() { + lineNo++ + line := strings.TrimSpace(scanner.Text()) + if line == "" { + continue + } + var v struct { + ID string `json:"id"` + } + if err := json.Unmarshal([]byte(line), &v); err != nil || v.ID == "" { + malformed++ + if len(examples) < 5 { + if err != nil { + examples = append(examples, fmt.Sprintf("line %d: %v", lineNo, err)) + } else { + examples = append(examples, fmt.Sprintf("line %d: missing id", lineNo)) + } + } + } + } + if err := scanner.Err(); err != nil { + return DoctorCheck{ + Name: "JSONL Integrity", + Status: StatusWarning, + Message: "Unable to scan JSONL file", + Detail: err.Error(), + } + } + if malformed == 0 { + return DoctorCheck{ + Name: "JSONL Integrity", + Status: StatusOK, + Message: fmt.Sprintf("%s looks valid", filepath.Base(jsonlPath)), + } + } + + // If we have a database, we can auto-repair by re-exporting from DB. + dbPath := filepath.Join(beadsDir, beads.CanonicalDatabaseName) + if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil && cfg.Database != "" { + dbPath = cfg.DatabasePath(beadsDir) + } + if _, err := os.Stat(dbPath); os.IsNotExist(err) { + return DoctorCheck{ + Name: "JSONL Integrity", + Status: StatusError, + Message: fmt.Sprintf("%s has %d malformed line(s)", filepath.Base(jsonlPath), malformed), + Detail: strings.Join(examples, "\n"), + Fix: "Restore the JSONL file from git or from a backup (no database available for auto-repair).", + } + } + + return DoctorCheck{ + Name: "JSONL Integrity", + Status: StatusError, + Message: fmt.Sprintf("%s has %d malformed line(s)", filepath.Base(jsonlPath), malformed), + Detail: strings.Join(examples, "\n"), + Fix: "Run 'bd doctor --fix' to back up the JSONL and regenerate it from the database.", + } +} + +func isSystemJSONLFilename(name string) bool { + switch name { + case "deletions.jsonl", "interactions.jsonl", "molecules.jsonl": + return true + default: + return false + } +} diff --git a/cmd/bd/doctor/jsonl_integrity_test.go b/cmd/bd/doctor/jsonl_integrity_test.go new file mode 100644 index 00000000..772e16a5 --- /dev/null +++ b/cmd/bd/doctor/jsonl_integrity_test.go @@ -0,0 +1,43 @@ +package doctor + +import ( + "os" + "path/filepath" + "testing" +) + +func TestCheckJSONLIntegrity_MalformedLine(t *testing.T) { + ws := t.TempDir() + beadsDir := filepath.Join(ws, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + if err := os.WriteFile(jsonlPath, []byte("{\"id\":\"t-1\"}\n{not json}\n"), 0644); err != nil { + t.Fatal(err) + } + // Ensure DB exists so check suggests auto-repair. + if err := os.WriteFile(filepath.Join(beadsDir, "beads.db"), []byte("x"), 0644); err != nil { + t.Fatal(err) + } + + check := CheckJSONLIntegrity(ws) + if check.Status != StatusError { + t.Fatalf("expected StatusError, got %v (%s)", check.Status, check.Message) + } + if check.Fix == "" { + t.Fatalf("expected Fix guidance") + } +} + +func TestCheckJSONLIntegrity_NoJSONL(t *testing.T) { + ws := t.TempDir() + beadsDir := filepath.Join(ws, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + check := CheckJSONLIntegrity(ws) + if check.Status != StatusOK { + t.Fatalf("expected StatusOK, got %v (%s)", check.Status, check.Message) + } +} diff --git a/cmd/bd/doctor_repair_chaos_test.go b/cmd/bd/doctor_repair_chaos_test.go index e9d9ee7f..fa469c6f 100644 --- a/cmd/bd/doctor_repair_chaos_test.go +++ b/cmd/bd/doctor_repair_chaos_test.go @@ -3,10 +3,14 @@ package main import ( + "bytes" + "io" "os" + "os/exec" "path/filepath" "strings" "testing" + "time" ) func TestDoctorRepair_CorruptDatabase_NotADatabase_RebuildFromJSONL(t *testing.T) { @@ -131,3 +135,171 @@ func TestDoctorRepair_CorruptDatabase_BacksUpSidecars(t *testing.T) { } } } + +func TestDoctorRepair_CorruptDatabase_WithRunningDaemon_FixSucceeds(t *testing.T) { + bdExe := buildBDForTest(t) + ws := mkTmpDirInTmp(t, "bd-doctor-chaos-daemon-*") + dbPath := filepath.Join(ws, ".beads", "beads.db") + jsonlPath := filepath.Join(ws, ".beads", "issues.jsonl") + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "init", "--prefix", "chaos", "--quiet"); err != nil { + t.Fatalf("bd init failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "create", "Chaos issue", "-p", "1"); err != nil { + t.Fatalf("bd create failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "export", "-o", jsonlPath, "--force"); err != nil { + t.Fatalf("bd export failed: %v", err) + } + + cmd := startDaemonForChaosTest(t, bdExe, ws, dbPath) + defer func() { + if cmd.Process != nil && (cmd.ProcessState == nil || !cmd.ProcessState.Exited()) { + _ = cmd.Process.Kill() + _, _ = cmd.Process.Wait() + } + }() + + // Corrupt the DB. + if err := os.WriteFile(dbPath, []byte("not a database"), 0644); err != nil { + t.Fatalf("corrupt db: %v", err) + } + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "doctor", "--fix", "--yes"); err != nil { + t.Fatalf("bd doctor --fix failed: %v", err) + } + + // Ensure we can cleanly stop the daemon afterwards (repair shouldn't wedge it). + if cmd.Process != nil { + _ = cmd.Process.Kill() + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + select { + case <-time.After(3 * time.Second): + t.Fatalf("expected daemon to exit when killed") + case <-done: + // ok + } + } +} + +func TestDoctorRepair_JSONLIntegrity_MalformedLine_ReexportFromDB(t *testing.T) { + bdExe := buildBDForTest(t) + ws := mkTmpDirInTmp(t, "bd-doctor-chaos-jsonl-*") + dbPath := filepath.Join(ws, ".beads", "beads.db") + jsonlPath := filepath.Join(ws, ".beads", "issues.jsonl") + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "init", "--prefix", "chaos", "--quiet"); err != nil { + t.Fatalf("bd init failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "create", "Chaos issue", "-p", "1"); err != nil { + t.Fatalf("bd create failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "export", "-o", jsonlPath, "--force"); err != nil { + t.Fatalf("bd export failed: %v", err) + } + + // Corrupt JSONL (leave DB intact). + f, err := os.OpenFile(jsonlPath, os.O_APPEND|os.O_WRONLY, 0644) + if err != nil { + t.Fatalf("open jsonl: %v", err) + } + if _, err := f.WriteString("{not json}\n"); err != nil { + _ = f.Close() + t.Fatalf("append corrupt jsonl: %v", err) + } + _ = f.Close() + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "doctor", "--fix", "--yes"); err != nil { + t.Fatalf("bd doctor --fix failed: %v", err) + } + + data, err := os.ReadFile(jsonlPath) + if err != nil { + t.Fatalf("read jsonl: %v", err) + } + if strings.Contains(string(data), "{not json}") { + t.Fatalf("expected JSONL to be regenerated without corrupt line") + } +} + +func TestDoctorRepair_CorruptDatabase_ReadOnlyBeadsDir_PermissionsFixMakesWritable(t *testing.T) { + bdExe := buildBDForTest(t) + ws := mkTmpDirInTmp(t, "bd-doctor-chaos-readonly-*") + beadsDir := filepath.Join(ws, ".beads") + dbPath := filepath.Join(beadsDir, "beads.db") + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "init", "--prefix", "chaos", "--quiet"); err != nil { + t.Fatalf("bd init failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "create", "Chaos issue", "-p", "1"); err != nil { + t.Fatalf("bd create failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "export", "-o", jsonlPath, "--force"); err != nil { + t.Fatalf("bd export failed: %v", err) + } + + // Corrupt the DB. + if err := os.Truncate(dbPath, 64); err != nil { + t.Fatalf("truncate db: %v", err) + } + + // Make .beads read-only; the Permissions fix should make it writable again. + if err := os.Chmod(beadsDir, 0555); err != nil { + t.Fatalf("chmod beads dir: %v", err) + } + t.Cleanup(func() { _ = os.Chmod(beadsDir, 0755) }) + + if out, err := runBDSideDB(t, bdExe, ws, dbPath, "doctor", "--fix", "--yes"); err != nil { + t.Fatalf("expected bd doctor --fix to succeed (permissions auto-fix), got: %v\n%s", err, out) + } + info, err := os.Stat(beadsDir) + if err != nil { + t.Fatalf("stat beads dir: %v", err) + } + if info.Mode().Perm()&0200 == 0 { + t.Fatalf("expected .beads to be writable after permissions fix, mode=%v", info.Mode().Perm()) + } +} + +func startDaemonForChaosTest(t *testing.T, bdExe, ws, dbPath string) *exec.Cmd { + t.Helper() + cmd := exec.Command(bdExe, "--db", dbPath, "daemon", "--start", "--foreground", "--local", "--interval", "10m") + cmd.Dir = ws + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + // Inherit environment, but explicitly ensure daemon mode is allowed. + env := make([]string, 0, len(os.Environ())+1) + for _, e := range os.Environ() { + if strings.HasPrefix(e, "BEADS_NO_DAEMON=") { + continue + } + env = append(env, e) + } + cmd.Env = env + + if err := cmd.Start(); err != nil { + t.Fatalf("start daemon: %v", err) + } + + // Wait for socket to appear. + sock := filepath.Join(ws, ".beads", "bd.sock") + deadline := time.Now().Add(8 * time.Second) + for time.Now().Before(deadline) { + if _, err := os.Stat(sock); err == nil { + // Put the process back into the caller's control. + cmd.Stdout = io.Discard + cmd.Stderr = io.Discard + return cmd + } + time.Sleep(50 * time.Millisecond) + } + + _ = cmd.Process.Kill() + _ = cmd.Wait() + t.Fatalf("daemon failed to start (no socket: %s)\nstdout:\n%s\nstderr:\n%s", sock, stdout.String(), stderr.String()) + return nil +} diff --git a/cmd/bd/test_repo_beads_guard_test.go b/cmd/bd/test_repo_beads_guard_test.go new file mode 100644 index 00000000..9f7adddf --- /dev/null +++ b/cmd/bd/test_repo_beads_guard_test.go @@ -0,0 +1,118 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "testing" + "time" +) + +// Guardrail: ensure the cmd/bd test suite does not touch the real repo .beads state. +// Disable with BEADS_TEST_GUARD_DISABLE=1 (useful when running tests while actively using beads). +func TestMain(m *testing.M) { + if os.Getenv("BEADS_TEST_GUARD_DISABLE") != "" { + os.Exit(m.Run()) + } + + repoRoot := findRepoRoot() + if repoRoot == "" { + os.Exit(m.Run()) + } + + repoBeadsDir := filepath.Join(repoRoot, ".beads") + if _, err := os.Stat(repoBeadsDir); err != nil { + os.Exit(m.Run()) + } + + watch := []string{ + "beads.db", + "beads.db-wal", + "beads.db-shm", + "beads.db-journal", + "issues.jsonl", + "beads.jsonl", + "metadata.json", + "interactions.jsonl", + "deletions.jsonl", + "molecules.jsonl", + "daemon.lock", + "daemon.pid", + "bd.sock", + } + + before := snapshotFiles(repoBeadsDir, watch) + code := m.Run() + after := snapshotFiles(repoBeadsDir, watch) + + if diff := diffSnapshots(before, after); diff != "" { + fmt.Fprintf(os.Stderr, "ERROR: test suite modified repo .beads state:\n%s\n", diff) + if code == 0 { + code = 1 + } + } + + os.Exit(code) +} + +type fileSnap struct { + exists bool + size int64 + modUnix int64 +} + +func snapshotFiles(dir string, names []string) map[string]fileSnap { + out := make(map[string]fileSnap, len(names)) + for _, name := range names { + p := filepath.Join(dir, name) + info, err := os.Stat(p) + if err != nil { + out[name] = fileSnap{exists: false} + continue + } + out[name] = fileSnap{exists: true, size: info.Size(), modUnix: info.ModTime().UnixNano()} + } + return out +} + +func diffSnapshots(before, after map[string]fileSnap) string { + var out string + for name, b := range before { + a := after[name] + if b.exists != a.exists { + out += fmt.Sprintf("- %s: exists %v → %v\n", name, b.exists, a.exists) + continue + } + if !b.exists { + continue + } + if b.size != a.size || b.modUnix != a.modUnix { + out += fmt.Sprintf("- %s: size %d → %d, mtime %s → %s\n", + name, + b.size, + a.size, + time.Unix(0, b.modUnix).UTC().Format(time.RFC3339Nano), + time.Unix(0, a.modUnix).UTC().Format(time.RFC3339Nano), + ) + } + } + return out +} + +func findRepoRoot() string { + wd, err := os.Getwd() + if err != nil { + return "" + } + for i := 0; i < 25; i++ { + if _, err := os.Stat(filepath.Join(wd, "go.mod")); err == nil { + return wd + } + parent := filepath.Dir(wd) + if parent == wd { + break + } + wd = parent + } + return "" +} From 7af31066105bdc18582ccbace5f7ea95b61b8364 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 09:22:45 -0400 Subject: [PATCH 05/13] doctor: add fs fault injection and lock contention coverage Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- cmd/bd/doctor/config_values.go | 2 +- cmd/bd/doctor/database.go | 10 ++-- cmd/bd/doctor/fix/database_integrity.go | 10 ++-- cmd/bd/doctor/fix/fs.go | 57 +++++++++++++++++++ cmd/bd/doctor/fix/fs_test.go | 71 ++++++++++++++++++++++++ cmd/bd/doctor/fix/jsonl_integrity.go | 24 +------- cmd/bd/doctor/fix/sqlite_open.go | 52 ++++++++++++++++++ cmd/bd/doctor/fix/sync.go | 2 +- cmd/bd/doctor/fix/validation.go | 2 +- cmd/bd/doctor/git.go | 2 +- cmd/bd/doctor/installation.go | 4 +- cmd/bd/doctor/integrity.go | 8 +-- cmd/bd/doctor/sqlite_open.go | 54 ++++++++++++++++++ cmd/bd/doctor_repair_chaos_test.go | 73 +++++++++++++++++++++++++ cmd/bd/export.go | 54 +++++++++--------- 15 files changed, 357 insertions(+), 68 deletions(-) create mode 100644 cmd/bd/doctor/fix/fs.go create mode 100644 cmd/bd/doctor/fix/fs_test.go create mode 100644 cmd/bd/doctor/fix/sqlite_open.go create mode 100644 cmd/bd/doctor/sqlite_open.go diff --git a/cmd/bd/doctor/config_values.go b/cmd/bd/doctor/config_values.go index 1f81c4a0..71ebe5f9 100644 --- a/cmd/bd/doctor/config_values.go +++ b/cmd/bd/doctor/config_values.go @@ -357,7 +357,7 @@ func checkDatabaseConfigValues(repoPath string) []string { } // Open database in read-only mode - db, err := sql.Open("sqlite3", "file:"+dbPath+"?mode=ro") + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return issues // Can't open database, skip } diff --git a/cmd/bd/doctor/database.go b/cmd/bd/doctor/database.go index d0952984..2a1b0b9a 100644 --- a/cmd/bd/doctor/database.go +++ b/cmd/bd/doctor/database.go @@ -155,9 +155,9 @@ func CheckSchemaCompatibility(path string) DoctorCheck { } } - // Open database (bd-ckvw: This will run migrations and schema probe) + // Open database (bd-ckvw: schema probe) // Note: We can't use the global 'store' because doctor can check arbitrary paths - db, err := sql.Open("sqlite3", "file:"+dbPath+"?_pragma=foreign_keys(ON)&_pragma=busy_timeout(30000)") + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return DoctorCheck{ Name: "Schema Compatibility", @@ -244,7 +244,7 @@ func CheckDatabaseIntegrity(path string) DoctorCheck { } // Open database in read-only mode for integrity check - db, err := sql.Open("sqlite3", "file:"+dbPath+"?mode=ro&_pragma=busy_timeout(30000)") + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return DoctorCheck{ Name: "Database Integrity", @@ -350,7 +350,7 @@ func CheckDatabaseJSONLSync(path string) DoctorCheck { jsonlCount, jsonlPrefixes, jsonlErr := CountJSONLIssues(jsonlPath) // Single database open for all queries (instead of 3 separate opens) - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { // Database can't be opened. If JSONL has issues, suggest recovery. if jsonlErr == nil && jsonlCount > 0 { @@ -523,7 +523,7 @@ func FixDBJSONLSync(path string) error { // getDatabaseVersionFromPath reads the database version from the given path func getDatabaseVersionFromPath(dbPath string) string { - db, err := sql.Open("sqlite3", "file:"+dbPath+"?mode=ro") + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return "unknown" } diff --git a/cmd/bd/doctor/fix/database_integrity.go b/cmd/bd/doctor/fix/database_integrity.go index aadcbd05..23d048b6 100644 --- a/cmd/bd/doctor/fix/database_integrity.go +++ b/cmd/bd/doctor/fix/database_integrity.go @@ -65,10 +65,10 @@ func DatabaseIntegrity(path string) error { // Back up corrupt DB and its sidecar files. ts := time.Now().UTC().Format("20060102T150405Z") backupDB := dbPath + "." + ts + ".corrupt.backup.db" - if err := os.Rename(dbPath, backupDB); err != nil { + if err := moveFile(dbPath, backupDB); err != nil { // Retry once after attempting to kill daemons again (helps on platforms with strict file locks). _ = Daemon(absPath) - if err2 := os.Rename(dbPath, backupDB); err2 != nil { + if err2 := moveFile(dbPath, backupDB); err2 != nil { // Prefer the original error (more likely root cause). return fmt.Errorf("failed to back up database: %w", err) } @@ -76,7 +76,7 @@ func DatabaseIntegrity(path string) error { for _, suffix := range []string{"-wal", "-shm", "-journal"} { sidecar := dbPath + suffix if _, err := os.Stat(sidecar); err == nil { - _ = os.Rename(sidecar, backupDB+suffix) // best effort + _ = moveFile(sidecar, backupDB+suffix) // best effort } } @@ -98,9 +98,9 @@ func DatabaseIntegrity(path string) error { failedTS := time.Now().UTC().Format("20060102T150405Z") if _, statErr := os.Stat(dbPath); statErr == nil { failedDB := dbPath + "." + failedTS + ".failed.init.db" - _ = os.Rename(dbPath, failedDB) + _ = moveFile(dbPath, failedDB) for _, suffix := range []string{"-wal", "-shm", "-journal"} { - _ = os.Rename(dbPath+suffix, failedDB+suffix) + _ = moveFile(dbPath+suffix, failedDB+suffix) } } _ = copyFile(backupDB, dbPath) diff --git a/cmd/bd/doctor/fix/fs.go b/cmd/bd/doctor/fix/fs.go new file mode 100644 index 00000000..fddb48e1 --- /dev/null +++ b/cmd/bd/doctor/fix/fs.go @@ -0,0 +1,57 @@ +package fix + +import ( + "errors" + "fmt" + "io" + "os" + "syscall" +) + +var ( + renameFile = os.Rename + removeFile = os.Remove + openFileRO = os.Open + openFileRW = os.OpenFile +) + +func moveFile(src, dst string) error { + if err := renameFile(src, dst); err == nil { + return nil + } else if isEXDEV(err) { + if err := copyFile(src, dst); err != nil { + return err + } + if err := removeFile(src); err != nil { + return fmt.Errorf("failed to remove source after copy: %w", err) + } + return nil + } else { + return err + } +} + +func copyFile(src, dst string) error { + in, err := openFileRO(src) // #nosec G304 -- src is within the workspace + if err != nil { + return err + } + defer in.Close() + out, err := openFileRW(dst, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) + if err != nil { + return err + } + defer func() { _ = out.Close() }() + if _, err := io.Copy(out, in); err != nil { + return err + } + return out.Close() +} + +func isEXDEV(err error) bool { + var linkErr *os.LinkError + if errors.As(err, &linkErr) { + return errors.Is(linkErr.Err, syscall.EXDEV) + } + return errors.Is(err, syscall.EXDEV) +} diff --git a/cmd/bd/doctor/fix/fs_test.go b/cmd/bd/doctor/fix/fs_test.go new file mode 100644 index 00000000..db242f3c --- /dev/null +++ b/cmd/bd/doctor/fix/fs_test.go @@ -0,0 +1,71 @@ +package fix + +import ( + "errors" + "os" + "path/filepath" + "syscall" + "testing" +) + +func TestMoveFile_EXDEV_FallsBackToCopy(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "src.txt") + dst := filepath.Join(root, "dst.txt") + if err := os.WriteFile(src, []byte("hello"), 0644); err != nil { + t.Fatal(err) + } + + oldRename := renameFile + defer func() { renameFile = oldRename }() + renameFile = func(oldpath, newpath string) error { + return &os.LinkError{Op: "rename", Old: oldpath, New: newpath, Err: syscall.EXDEV} + } + + if err := moveFile(src, dst); err != nil { + t.Fatalf("moveFile failed: %v", err) + } + if _, err := os.Stat(src); !os.IsNotExist(err) { + t.Fatalf("expected src to be removed, stat err=%v", err) + } + data, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("read dst: %v", err) + } + if string(data) != "hello" { + t.Fatalf("dst contents=%q", string(data)) + } +} + +func TestMoveFile_EXDEV_CopyFails_LeavesSource(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "src.txt") + dst := filepath.Join(root, "dst.txt") + if err := os.WriteFile(src, []byte("hello"), 0644); err != nil { + t.Fatal(err) + } + + oldRename := renameFile + oldOpenRW := openFileRW + defer func() { + renameFile = oldRename + openFileRW = oldOpenRW + }() + renameFile = func(oldpath, newpath string) error { + return &os.LinkError{Op: "rename", Old: oldpath, New: newpath, Err: syscall.EXDEV} + } + openFileRW = func(name string, flag int, perm os.FileMode) (*os.File, error) { + return nil, &os.PathError{Op: "open", Path: name, Err: syscall.ENOSPC} + } + + err := moveFile(src, dst) + if err == nil { + t.Fatalf("expected error") + } + if !errors.Is(err, syscall.ENOSPC) { + t.Fatalf("expected ENOSPC, got %v", err) + } + if _, err := os.Stat(src); err != nil { + t.Fatalf("expected src to remain, stat err=%v", err) + } +} diff --git a/cmd/bd/doctor/fix/jsonl_integrity.go b/cmd/bd/doctor/fix/jsonl_integrity.go index 20f14923..11273298 100644 --- a/cmd/bd/doctor/fix/jsonl_integrity.go +++ b/cmd/bd/doctor/fix/jsonl_integrity.go @@ -2,7 +2,6 @@ package fix import ( "fmt" - "io" "os" "path/filepath" "time" @@ -58,13 +57,13 @@ func JSONLIntegrity(path string) error { // Back up the JSONL. ts := time.Now().UTC().Format("20060102T150405Z") backup := jsonlPath + "." + ts + ".corrupt.backup.jsonl" - if err := os.Rename(jsonlPath, backup); err != nil { + if err := moveFile(jsonlPath, backup); err != nil { return fmt.Errorf("failed to back up JSONL: %w", err) } binary, err := getBdBinary() if err != nil { - _ = os.Rename(backup, jsonlPath) + _ = moveFile(backup, jsonlPath) return err } @@ -78,7 +77,7 @@ func JSONLIntegrity(path string) error { failedTS := time.Now().UTC().Format("20060102T150405Z") if _, statErr := os.Stat(jsonlPath); statErr == nil { failed := jsonlPath + "." + failedTS + ".failed.regen.jsonl" - _ = os.Rename(jsonlPath, failed) + _ = moveFile(jsonlPath, failed) } _ = copyFile(backup, jsonlPath) return fmt.Errorf("failed to regenerate JSONL from database: %w (backup: %s)", err, backup) @@ -86,20 +85,3 @@ func JSONLIntegrity(path string) error { return nil } - -func copyFile(src, dst string) error { - in, err := os.Open(src) // #nosec G304 -- src is within the workspace - if err != nil { - return err - } - defer in.Close() - out, err := os.OpenFile(dst, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) - if err != nil { - return err - } - defer func() { _ = out.Close() }() - if _, err := io.Copy(out, in); err != nil { - return err - } - return out.Close() -} diff --git a/cmd/bd/doctor/fix/sqlite_open.go b/cmd/bd/doctor/fix/sqlite_open.go new file mode 100644 index 00000000..373b81c8 --- /dev/null +++ b/cmd/bd/doctor/fix/sqlite_open.go @@ -0,0 +1,52 @@ +package fix + +import ( + "fmt" + "os" + "strings" + "time" +) + +func sqliteConnString(path string, readOnly bool) string { + path = strings.TrimSpace(path) + if path == "" { + return "" + } + + busy := 30 * time.Second + if v := strings.TrimSpace(os.Getenv("BD_LOCK_TIMEOUT")); v != "" { + if d, err := time.ParseDuration(v); err == nil { + busy = d + } + } + busyMs := int64(busy / time.Millisecond) + + if strings.HasPrefix(path, "file:") { + conn := path + sep := "?" + if strings.Contains(conn, "?") { + sep = "&" + } + if readOnly && !strings.Contains(conn, "mode=") { + conn += sep + "mode=ro" + sep = "&" + } + if !strings.Contains(conn, "_pragma=busy_timeout") { + conn += fmt.Sprintf("%s_pragma=busy_timeout(%d)", sep, busyMs) + sep = "&" + } + if !strings.Contains(conn, "_pragma=foreign_keys") { + conn += sep + "_pragma=foreign_keys(ON)" + sep = "&" + } + if !strings.Contains(conn, "_time_format=") { + conn += sep + "_time_format=sqlite" + } + return conn + } + + if readOnly { + return fmt.Sprintf("file:%s?mode=ro&_pragma=foreign_keys(ON)&_pragma=busy_timeout(%d)&_time_format=sqlite", path, busyMs) + } + return fmt.Sprintf("file:%s?_pragma=foreign_keys(ON)&_pragma=busy_timeout(%d)&_time_format=sqlite", path, busyMs) +} diff --git a/cmd/bd/doctor/fix/sync.go b/cmd/bd/doctor/fix/sync.go index dfc3a1e3..7224326e 100644 --- a/cmd/bd/doctor/fix/sync.go +++ b/cmd/bd/doctor/fix/sync.go @@ -149,7 +149,7 @@ func DBJSONLSync(path string) error { // countDatabaseIssues counts the number of issues in the database. func countDatabaseIssues(dbPath string) (int, error) { - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return 0, fmt.Errorf("failed to open database: %w", err) } diff --git a/cmd/bd/doctor/fix/validation.go b/cmd/bd/doctor/fix/validation.go index 1b5beb0d..a34e52cf 100644 --- a/cmd/bd/doctor/fix/validation.go +++ b/cmd/bd/doctor/fix/validation.go @@ -229,5 +229,5 @@ func ChildParentDependencies(path string) error { // openDB opens a SQLite database for read-write access func openDB(dbPath string) (*sql.DB, error) { - return sql.Open("sqlite3", dbPath) + return sql.Open("sqlite3", sqliteConnString(dbPath, false)) } diff --git a/cmd/bd/doctor/git.go b/cmd/bd/doctor/git.go index 77c234a1..46845111 100644 --- a/cmd/bd/doctor/git.go +++ b/cmd/bd/doctor/git.go @@ -829,5 +829,5 @@ func CheckOrphanedIssues(path string) DoctorCheck { // openDBReadOnly opens a SQLite database in read-only mode func openDBReadOnly(dbPath string) (*sql.DB, error) { - return sql.Open("sqlite3", "file:"+dbPath+"?mode=ro") + return sql.Open("sqlite3", sqliteConnString(dbPath, true)) } diff --git a/cmd/bd/doctor/installation.go b/cmd/bd/doctor/installation.go index c5b94eeb..478c1638 100644 --- a/cmd/bd/doctor/installation.go +++ b/cmd/bd/doctor/installation.go @@ -106,7 +106,7 @@ func CheckPermissions(path string) DoctorCheck { dbPath := filepath.Join(beadsDir, beads.CanonicalDatabaseName) if _, err := os.Stat(dbPath); err == nil { // Try to open database - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return DoctorCheck{ Name: "Permissions", @@ -118,7 +118,7 @@ func CheckPermissions(path string) DoctorCheck { _ = db.Close() // Intentionally ignore close error // Try a write test - db, err = sql.Open("sqlite", dbPath) + db, err = sql.Open("sqlite", sqliteConnString(dbPath, true)) if err == nil { _, err = db.Exec("SELECT 1") _ = db.Close() // Intentionally ignore close error diff --git a/cmd/bd/doctor/integrity.go b/cmd/bd/doctor/integrity.go index df9c3375..35aecabc 100644 --- a/cmd/bd/doctor/integrity.go +++ b/cmd/bd/doctor/integrity.go @@ -51,7 +51,7 @@ func CheckIDFormat(path string) DoctorCheck { } // Open database - db, err := sql.Open("sqlite3", "file:"+dbPath+"?mode=ro") + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return DoctorCheck{ Name: "Issue IDs", @@ -121,7 +121,7 @@ func CheckDependencyCycles(path string) DoctorCheck { } // Open database to check for cycles - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return DoctorCheck{ Name: "Dependency Cycles", @@ -216,7 +216,7 @@ func CheckTombstones(path string) DoctorCheck { } } - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return DoctorCheck{ Name: "Tombstones", @@ -420,7 +420,7 @@ func CheckRepoFingerprint(path string) DoctorCheck { } // Open database - db, err := sql.Open("sqlite3", "file:"+dbPath+"?mode=ro") + db, err := sql.Open("sqlite3", sqliteConnString(dbPath, true)) if err != nil { return DoctorCheck{ Name: "Repo Fingerprint", diff --git a/cmd/bd/doctor/sqlite_open.go b/cmd/bd/doctor/sqlite_open.go new file mode 100644 index 00000000..da982233 --- /dev/null +++ b/cmd/bd/doctor/sqlite_open.go @@ -0,0 +1,54 @@ +package doctor + +import ( + "fmt" + "os" + "strings" + "time" +) + +func sqliteConnString(path string, readOnly bool) string { + path = strings.TrimSpace(path) + if path == "" { + return "" + } + + // Best-effort: honor the same env var viper uses (BD_LOCK_TIMEOUT). + busy := 30 * time.Second + if v := strings.TrimSpace(os.Getenv("BD_LOCK_TIMEOUT")); v != "" { + if d, err := time.ParseDuration(v); err == nil { + busy = d + } + } + busyMs := int64(busy / time.Millisecond) + + // If it's already a URI, append pragmas if absent. + if strings.HasPrefix(path, "file:") { + conn := path + sep := "?" + if strings.Contains(conn, "?") { + sep = "&" + } + if readOnly && !strings.Contains(conn, "mode=") { + conn += sep + "mode=ro" + sep = "&" + } + if !strings.Contains(conn, "_pragma=busy_timeout") { + conn += fmt.Sprintf("%s_pragma=busy_timeout(%d)", sep, busyMs) + sep = "&" + } + if !strings.Contains(conn, "_pragma=foreign_keys") { + conn += sep + "_pragma=foreign_keys(ON)" + sep = "&" + } + if !strings.Contains(conn, "_time_format=") { + conn += sep + "_time_format=sqlite" + } + return conn + } + + if readOnly { + return fmt.Sprintf("file:%s?mode=ro&_pragma=foreign_keys(ON)&_pragma=busy_timeout(%d)&_time_format=sqlite", path, busyMs) + } + return fmt.Sprintf("file:%s?_pragma=foreign_keys(ON)&_pragma=busy_timeout(%d)&_time_format=sqlite", path, busyMs) +} diff --git a/cmd/bd/doctor_repair_chaos_test.go b/cmd/bd/doctor_repair_chaos_test.go index fa469c6f..5af6ffd3 100644 --- a/cmd/bd/doctor_repair_chaos_test.go +++ b/cmd/bd/doctor_repair_chaos_test.go @@ -4,6 +4,8 @@ package main import ( "bytes" + "context" + "database/sql" "io" "os" "os/exec" @@ -11,6 +13,8 @@ import ( "strings" "testing" "time" + + _ "github.com/ncruces/go-sqlite3/driver" ) func TestDoctorRepair_CorruptDatabase_NotADatabase_RebuildFromJSONL(t *testing.T) { @@ -223,6 +227,55 @@ func TestDoctorRepair_JSONLIntegrity_MalformedLine_ReexportFromDB(t *testing.T) } } +func TestDoctorRepair_DatabaseIntegrity_DBWriteLocked_ImportFailsFast(t *testing.T) { + bdExe := buildBDForTest(t) + ws := mkTmpDirInTmp(t, "bd-doctor-chaos-db-locked-*") + dbPath := filepath.Join(ws, ".beads", "beads.db") + jsonlPath := filepath.Join(ws, ".beads", "issues.jsonl") + + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "init", "--prefix", "chaos", "--quiet"); err != nil { + t.Fatalf("bd init failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "create", "Chaos issue", "-p", "1"); err != nil { + t.Fatalf("bd create failed: %v", err) + } + if _, err := runBDSideDB(t, bdExe, ws, dbPath, "export", "-o", jsonlPath, "--force"); err != nil { + t.Fatalf("bd export failed: %v", err) + } + + // Lock the DB for writes in-process. + db, err := sql.Open("sqlite3", dbPath) + if err != nil { + t.Fatalf("open db: %v", err) + } + defer db.Close() + tx, err := db.Begin() + if err != nil { + t.Fatalf("begin tx: %v", err) + } + if _, err := tx.Exec("INSERT INTO issues (id, title, status) VALUES ('lock-test', 'Lock Test', 'open')"); err != nil { + _ = tx.Rollback() + t.Fatalf("insert lock row: %v", err) + } + defer func() { _ = tx.Rollback() }() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + out, err := runBDWithEnv(ctx, bdExe, ws, dbPath, map[string]string{ + "BD_LOCK_TIMEOUT": "200ms", + }, "import", "-i", jsonlPath, "--force", "--skip-existing", "--no-git-history") + if err == nil { + t.Fatalf("expected bd import to fail under DB write lock") + } + if ctx.Err() == context.DeadlineExceeded { + t.Fatalf("import exceeded timeout (likely hung); output:\n%s", out) + } + low := strings.ToLower(out) + if !strings.Contains(low, "locked") && !strings.Contains(low, "busy") && !strings.Contains(low, "timeout") { + t.Fatalf("expected lock/busy/timeout error, got:\n%s", out) + } +} + func TestDoctorRepair_CorruptDatabase_ReadOnlyBeadsDir_PermissionsFixMakesWritable(t *testing.T) { bdExe := buildBDForTest(t) ws := mkTmpDirInTmp(t, "bd-doctor-chaos-readonly-*") @@ -303,3 +356,23 @@ func startDaemonForChaosTest(t *testing.T, bdExe, ws, dbPath string) *exec.Cmd { t.Fatalf("daemon failed to start (no socket: %s)\nstdout:\n%s\nstderr:\n%s", sock, stdout.String(), stderr.String()) return nil } + +func runBDWithEnv(ctx context.Context, exe, dir, dbPath string, env map[string]string, args ...string) (string, error) { + fullArgs := []string{"--db", dbPath} + if len(args) > 0 && args[0] != "init" { + fullArgs = append(fullArgs, "--no-daemon") + } + fullArgs = append(fullArgs, args...) + + cmd := exec.CommandContext(ctx, exe, fullArgs...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "BEADS_NO_DAEMON=1", + "BEADS_DIR="+filepath.Join(dir, ".beads"), + ) + for k, v := range env { + cmd.Env = append(cmd.Env, k+"="+v) + } + out, err := cmd.CombinedOutput() + return string(out), err +} diff --git a/cmd/bd/export.go b/cmd/bd/export.go index bca6fd17..b669f174 100644 --- a/cmd/bd/export.go +++ b/cmd/bd/export.go @@ -156,7 +156,7 @@ Examples: _ = daemonClient.Close() daemonClient = nil } - + // Note: We used to check database file timestamps here, but WAL files // get created when opening the DB, making timestamp checks unreliable. // Instead, we check issue counts after loading (see below). @@ -168,7 +168,7 @@ Examples: fmt.Fprintf(os.Stderr, "Error: no database path found\n") os.Exit(1) } - store, err = sqlite.New(rootCtx, dbPath) + store, err = sqlite.NewWithTimeout(rootCtx, dbPath, lockTimeout) if err != nil { fmt.Fprintf(os.Stderr, "Error: failed to open database: %v\n", err) os.Exit(1) @@ -302,20 +302,20 @@ Examples: // Safety check: prevent exporting stale database that would lose issues if output != "" && !force { debug.Logf("Debug: checking staleness - output=%s, force=%v\n", output, force) - + // Read existing JSONL to get issue IDs jsonlIDs, err := getIssueIDsFromJSONL(output) if err != nil && !os.IsNotExist(err) { fmt.Fprintf(os.Stderr, "Warning: failed to read existing JSONL for staleness check: %v\n", err) } - + if err == nil && len(jsonlIDs) > 0 { // Build set of DB issue IDs dbIDs := make(map[string]bool) for _, issue := range issues { dbIDs[issue.ID] = true } - + // Check if JSONL has any issues that DB doesn't have var missingIDs []string for id := range jsonlIDs { @@ -323,17 +323,17 @@ Examples: missingIDs = append(missingIDs, id) } } - - debug.Logf("Debug: JSONL has %d issues, DB has %d issues, missing %d\n", + + debug.Logf("Debug: JSONL has %d issues, DB has %d issues, missing %d\n", len(jsonlIDs), len(issues), len(missingIDs)) - + if len(missingIDs) > 0 { slices.Sort(missingIDs) fmt.Fprintf(os.Stderr, "Error: refusing to export stale database that would lose issues\n") fmt.Fprintf(os.Stderr, " Database has %d issues\n", len(issues)) fmt.Fprintf(os.Stderr, " JSONL has %d issues\n", len(jsonlIDs)) fmt.Fprintf(os.Stderr, " Export would lose %d issue(s):\n", len(missingIDs)) - + // Show first 10 missing issues showCount := len(missingIDs) if showCount > 10 { @@ -345,7 +345,7 @@ Examples: if len(missingIDs) > 10 { fmt.Fprintf(os.Stderr, " ... and %d more\n", len(missingIDs)-10) } - + fmt.Fprintf(os.Stderr, "\n") fmt.Fprintf(os.Stderr, "This usually means:\n") fmt.Fprintf(os.Stderr, " 1. You need to run 'bd import -i %s' to sync the latest changes\n", output) @@ -434,8 +434,8 @@ Examples: skippedCount := 0 for _, issue := range issues { if err := encoder.Encode(issue); err != nil { - fmt.Fprintf(os.Stderr, "Error encoding issue %s: %v\n", issue.ID, err) - os.Exit(1) + fmt.Fprintf(os.Stderr, "Error encoding issue %s: %v\n", issue.ID, err) + os.Exit(1) } exportedIDs = append(exportedIDs, issue.ID) @@ -495,19 +495,19 @@ Examples: } } - // Verify JSONL file integrity after export - actualCount, err := countIssuesInJSONL(finalPath) - if err != nil { - fmt.Fprintf(os.Stderr, "Error: Export verification failed: %v\n", err) - os.Exit(1) - } - if actualCount != len(exportedIDs) { - fmt.Fprintf(os.Stderr, "Error: Export verification failed\n") - fmt.Fprintf(os.Stderr, " Expected: %d issues\n", len(exportedIDs)) - fmt.Fprintf(os.Stderr, " JSONL file: %d lines\n", actualCount) - fmt.Fprintf(os.Stderr, " Mismatch indicates export failed to write all issues\n") - os.Exit(1) - } + // Verify JSONL file integrity after export + actualCount, err := countIssuesInJSONL(finalPath) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: Export verification failed: %v\n", err) + os.Exit(1) + } + if actualCount != len(exportedIDs) { + fmt.Fprintf(os.Stderr, "Error: Export verification failed\n") + fmt.Fprintf(os.Stderr, " Expected: %d issues\n", len(exportedIDs)) + fmt.Fprintf(os.Stderr, " JSONL file: %d lines\n", actualCount) + fmt.Fprintf(os.Stderr, " Mismatch indicates export failed to write all issues\n") + os.Exit(1) + } // Update database mtime to be >= JSONL mtime (fixes #278, #301, #321) // Only do this when exporting to default JSONL path (not arbitrary outputs) @@ -520,9 +520,9 @@ Examples: fmt.Fprintf(os.Stderr, "Warning: failed to update database mtime: %v\n", err) } } - } + } - // Output statistics if JSON format requested + // Output statistics if JSON format requested if jsonOutput { stats := map[string]interface{}{ "success": true, From 6c37a58148ae8f607b1e648d474ff3958fcea6d0 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 09:47:48 -0400 Subject: [PATCH 06/13] test: report total coverage from make test Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- Makefile | 2 +- scripts/test.sh | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 4ea2c17c..cc0cd528 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ build: # Run all tests (skips known broken tests listed in .test-skip) test: @echo "Running tests..." - @./scripts/test.sh + @TEST_COVER=1 ./scripts/test.sh # Run performance benchmarks (10K and 20K issue databases with automatic CPU profiling) # Generates CPU profile: internal/storage/sqlite/bench-cpu-.prof diff --git a/scripts/test.sh b/scripts/test.sh index dc826936..ac5729bf 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -24,6 +24,9 @@ TIMEOUT="${TEST_TIMEOUT:-3m}" SKIP_PATTERN=$(build_skip_pattern) VERBOSE="${TEST_VERBOSE:-}" RUN_PATTERN="${TEST_RUN:-}" +COVERAGE="${TEST_COVER:-}" +COVERPROFILE="${TEST_COVERPROFILE:-/tmp/beads.coverage.out}" +COVERPKG="${TEST_COVERPKG:-./...}" # Parse arguments PACKAGES=() @@ -77,10 +80,22 @@ if [[ -n "$RUN_PATTERN" ]]; then CMD+=(-run "$RUN_PATTERN") fi +if [[ -n "$COVERAGE" ]]; then + CMD+=(-covermode=atomic -coverpkg "$COVERPKG" -coverprofile "$COVERPROFILE") +fi + CMD+=("${PACKAGES[@]}") echo "Running: ${CMD[*]}" >&2 echo "Skipping: $SKIP_PATTERN" >&2 echo "" >&2 -exec "${CMD[@]}" +"${CMD[@]}" +status=$? + +if [[ -n "$COVERAGE" ]]; then + total=$(go tool cover -func="$COVERPROFILE" | awk '/^total:/ {print $NF}') + echo "Total coverage: ${total} (profile: ${COVERPROFILE})" >&2 +fi + +exit $status From eab0b3ac4114112e46e06d56ba64bb96b48e7522 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 10:14:28 -0400 Subject: [PATCH 07/13] test: raise memory storage coverage --- .../memory/memory_more_coverage_test.go | 921 ++++++++++++++++++ 1 file changed, 921 insertions(+) create mode 100644 internal/storage/memory/memory_more_coverage_test.go diff --git a/internal/storage/memory/memory_more_coverage_test.go b/internal/storage/memory/memory_more_coverage_test.go new file mode 100644 index 00000000..401be669 --- /dev/null +++ b/internal/storage/memory/memory_more_coverage_test.go @@ -0,0 +1,921 @@ +package memory + +import ( + "context" + "testing" + "time" + + "github.com/steveyegge/beads/internal/storage" + "github.com/steveyegge/beads/internal/types" +) + +func TestMemoryStorage_LoadFromIssues_IndexesAndCounters(t *testing.T) { + store := New("/tmp/example.jsonl") + defer store.Close() + + extRef := "ext-1" + issues := []*types.Issue{ + nil, + { + ID: "bd-10", + Title: "Ten", + Status: types.StatusOpen, + Priority: 1, + IssueType: types.TypeTask, + ExternalRef: &extRef, + Dependencies: []*types.Dependency{{ + IssueID: "bd-10", + DependsOnID: "bd-2", + Type: types.DepBlocks, + }}, + Labels: []string{"l1"}, + Comments: []*types.Comment{{ID: 1, IssueID: "bd-10", Author: "a", Text: "c"}}, + }, + {ID: "bd-2", Title: "Two", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}, + {ID: "bd-a3f8e9", Title: "Parent", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}, + {ID: "bd-a3f8e9.3", Title: "Child", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}, + } + + if err := store.LoadFromIssues(issues); err != nil { + t.Fatalf("LoadFromIssues: %v", err) + } + + ctx := context.Background() + + got, err := store.GetIssueByExternalRef(ctx, "ext-1") + if err != nil { + t.Fatalf("GetIssueByExternalRef: %v", err) + } + if got == nil || got.ID != "bd-10" { + t.Fatalf("GetIssueByExternalRef got=%v", got) + } + if len(got.Dependencies) != 1 || got.Dependencies[0].DependsOnID != "bd-2" { + t.Fatalf("expected deps attached") + } + if len(got.Labels) != 1 || got.Labels[0] != "l1" { + t.Fatalf("expected labels attached") + } + + // Exercise CreateIssue ID generation based on the loaded counter (bd-10 => next should be bd-11). + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("SetConfig: %v", err) + } + newIssue := &types.Issue{Title: "New", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + if err := store.CreateIssue(ctx, newIssue, "actor"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + if newIssue.ID != "bd-11" { + t.Fatalf("expected generated id bd-11, got %q", newIssue.ID) + } + + // Hierarchical counter for parent extracted from bd-a3f8e9.3. + childID, err := store.GetNextChildID(ctx, "bd-a3f8e9") + if err != nil { + t.Fatalf("GetNextChildID: %v", err) + } + if childID != "bd-a3f8e9.4" { + t.Fatalf("expected bd-a3f8e9.4, got %q", childID) + } +} + +func TestMemoryStorage_GetAllIssues_SortsAndCopies(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + // Create out-of-order IDs. + a := &types.Issue{ID: "bd-2", Title: "A", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + b := &types.Issue{ID: "bd-1", Title: "B", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + if err := store.CreateIssue(ctx, a, "actor"); err != nil { + t.Fatalf("CreateIssue a: %v", err) + } + if err := store.CreateIssue(ctx, b, "actor"); err != nil { + t.Fatalf("CreateIssue b: %v", err) + } + + if err := store.AddLabel(ctx, a.ID, "l1", "actor"); err != nil { + t.Fatalf("AddLabel: %v", err) + } + + all := store.GetAllIssues() + if len(all) != 2 { + t.Fatalf("expected 2 issues, got %d", len(all)) + } + if all[0].ID != "bd-1" || all[1].ID != "bd-2" { + t.Fatalf("expected sorted by ID, got %q then %q", all[0].ID, all[1].ID) + } + + // Returned issues must be copies (mutating should not affect stored issue struct). + all[1].Title = "mutated" + got, err := store.GetIssue(ctx, "bd-2") + if err != nil { + t.Fatalf("GetIssue: %v", err) + } + if got.Title != "A" { + t.Fatalf("expected stored title unchanged, got %q", got.Title) + } +} + +func TestMemoryStorage_CreateIssues_DefaultPrefix_DuplicateExisting_ExternalRef(t *testing.T) { + store := New("") + defer store.Close() + ctx := context.Background() + + // Default prefix should be "bd" when unset. + issues := []*types.Issue{{Title: "A", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}} + if err := store.CreateIssues(ctx, issues, "actor"); err != nil { + t.Fatalf("CreateIssues: %v", err) + } + if issues[0].ID != "bd-1" { + t.Fatalf("expected bd-1, got %q", issues[0].ID) + } + + ext := "ext" + batch := []*types.Issue{{ID: "bd-x", Title: "B", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask, ExternalRef: &ext}} + if err := store.CreateIssues(ctx, batch, "actor"); err != nil { + t.Fatalf("CreateIssues: %v", err) + } + if got, _ := store.GetIssueByExternalRef(ctx, "ext"); got == nil || got.ID != "bd-x" { + t.Fatalf("expected external ref indexed") + } + + // Duplicate existing issue ID branch. + dup := []*types.Issue{{ID: "bd-x", Title: "Dup", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}} + if err := store.CreateIssues(ctx, dup, "actor"); err == nil { + t.Fatalf("expected duplicate existing issue error") + } +} + +func TestMemoryStorage_GetIssueByExternalRef_IndexPointsToMissingIssue(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + store.mu.Lock() + store.externalRefToID["dangling"] = "bd-nope" + store.mu.Unlock() + + got, err := store.GetIssueByExternalRef(ctx, "dangling") + if err != nil { + t.Fatalf("GetIssueByExternalRef: %v", err) + } + if got != nil { + t.Fatalf("expected nil for dangling ref") + } +} + +func TestMemoryStorage_DependencyCounts_Records_Tree_Cycles(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + a := &types.Issue{ID: "bd-1", Title: "A", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + b := &types.Issue{ID: "bd-2", Title: "B", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + c := &types.Issue{ID: "bd-3", Title: "C", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + d := &types.Issue{ID: "bd-4", Title: "D", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + for _, iss := range []*types.Issue{a, b, c, d} { + if err := store.CreateIssue(ctx, iss, "actor"); err != nil { + t.Fatalf("CreateIssue %s: %v", iss.ID, err) + } + } + + if err := store.AddDependency(ctx, &types.Dependency{IssueID: a.ID, DependsOnID: b.ID, Type: types.DepBlocks}, "actor"); err != nil { + t.Fatalf("AddDependency: %v", err) + } + if err := store.AddDependency(ctx, &types.Dependency{IssueID: a.ID, DependsOnID: c.ID, Type: types.DepBlocks}, "actor"); err != nil { + t.Fatalf("AddDependency: %v", err) + } + if err := store.AddDependency(ctx, &types.Dependency{IssueID: d.ID, DependsOnID: b.ID, Type: types.DepBlocks}, "actor"); err != nil { + t.Fatalf("AddDependency: %v", err) + } + + counts, err := store.GetDependencyCounts(ctx, []string{a.ID, b.ID, "bd-missing"}) + if err != nil { + t.Fatalf("GetDependencyCounts: %v", err) + } + if counts[a.ID].DependencyCount != 2 || counts[a.ID].DependentCount != 0 { + t.Fatalf("unexpected counts for A: %+v", counts[a.ID]) + } + if counts[b.ID].DependencyCount != 0 || counts[b.ID].DependentCount != 2 { + t.Fatalf("unexpected counts for B: %+v", counts[b.ID]) + } + if counts["bd-missing"].DependencyCount != 0 || counts["bd-missing"].DependentCount != 0 { + t.Fatalf("unexpected counts for missing: %+v", counts["bd-missing"]) + } + + deps, err := store.GetDependencyRecords(ctx, a.ID) + if err != nil { + t.Fatalf("GetDependencyRecords: %v", err) + } + if len(deps) != 2 { + t.Fatalf("expected 2 deps, got %d", len(deps)) + } + + allDeps, err := store.GetAllDependencyRecords(ctx) + if err != nil { + t.Fatalf("GetAllDependencyRecords: %v", err) + } + if len(allDeps[a.ID]) != 2 { + t.Fatalf("expected all deps for A") + } + + nodes, err := store.GetDependencyTree(ctx, a.ID, 3, false, false) + if err != nil { + t.Fatalf("GetDependencyTree: %v", err) + } + if len(nodes) != 2 || nodes[0].Depth != 1 { + t.Fatalf("unexpected tree: %+v", nodes) + } + + cycles, err := store.DetectCycles(ctx) + if err != nil { + t.Fatalf("DetectCycles: %v", err) + } + if cycles != nil { + t.Fatalf("expected nil cycles, got %+v", cycles) + } +} + +func TestMemoryStorage_HashTracking_NoOps(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + if hash, err := store.GetDirtyIssueHash(ctx, "bd-1"); err != nil || hash != "" { + t.Fatalf("GetDirtyIssueHash: hash=%q err=%v", hash, err) + } + if hash, err := store.GetExportHash(ctx, "bd-1"); err != nil || hash != "" { + t.Fatalf("GetExportHash: hash=%q err=%v", hash, err) + } + if err := store.SetExportHash(ctx, "bd-1", "h"); err != nil { + t.Fatalf("SetExportHash: %v", err) + } + if err := store.ClearAllExportHashes(ctx); err != nil { + t.Fatalf("ClearAllExportHashes: %v", err) + } + if hash, err := store.GetJSONLFileHash(ctx); err != nil || hash != "" { + t.Fatalf("GetJSONLFileHash: hash=%q err=%v", hash, err) + } + if err := store.SetJSONLFileHash(ctx, "h"); err != nil { + t.Fatalf("SetJSONLFileHash: %v", err) + } +} + +func TestMemoryStorage_LabelsAndCommentsHelpers(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + a := &types.Issue{ID: "bd-1", Title: "A", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + b := &types.Issue{ID: "bd-2", Title: "B", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + if err := store.CreateIssue(ctx, a, "actor"); err != nil { + t.Fatalf("CreateIssue a: %v", err) + } + if err := store.CreateIssue(ctx, b, "actor"); err != nil { + t.Fatalf("CreateIssue b: %v", err) + } + + if err := store.AddLabel(ctx, a.ID, "l1", "actor"); err != nil { + t.Fatalf("AddLabel: %v", err) + } + if err := store.AddLabel(ctx, b.ID, "l2", "actor"); err != nil { + t.Fatalf("AddLabel: %v", err) + } + + labels, err := store.GetLabelsForIssues(ctx, []string{a.ID, b.ID, "bd-missing"}) + if err != nil { + t.Fatalf("GetLabelsForIssues: %v", err) + } + if len(labels) != 2 { + t.Fatalf("expected 2 entries, got %d", len(labels)) + } + if labels[a.ID][0] != "l1" { + t.Fatalf("unexpected labels for A: %+v", labels[a.ID]) + } + + issues, err := store.GetIssuesByLabel(ctx, "l1") + if err != nil { + t.Fatalf("GetIssuesByLabel: %v", err) + } + if len(issues) != 1 || issues[0].ID != a.ID { + t.Fatalf("unexpected issues: %+v", issues) + } + + if _, err := store.AddIssueComment(ctx, a.ID, "author", "text"); err != nil { + t.Fatalf("AddIssueComment: %v", err) + } + comments, err := store.GetCommentsForIssues(ctx, []string{a.ID, b.ID}) + if err != nil { + t.Fatalf("GetCommentsForIssues: %v", err) + } + if len(comments[a.ID]) != 1 { + t.Fatalf("expected comments for A") + } +} + +func TestMemoryStorage_StaleEventsCustomStatusAndLifecycleHelpers(t *testing.T) { + store := New("/tmp/x.jsonl") + defer store.Close() + ctx := context.Background() + + if store.Path() != "/tmp/x.jsonl" { + t.Fatalf("Path mismatch") + } + if store.UnderlyingDB() != nil { + t.Fatalf("expected nil UnderlyingDB") + } + if _, err := store.UnderlyingConn(ctx); err == nil { + t.Fatalf("expected UnderlyingConn error") + } + if err := store.RunInTransaction(ctx, func(tx storage.Transaction) error { return nil }); err == nil { + t.Fatalf("expected RunInTransaction error") + } + + if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil { + t.Fatalf("SetConfig: %v", err) + } + a := &types.Issue{ID: "bd-1", Title: "A", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + if err := store.CreateIssue(ctx, a, "actor"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + + // Force updated_at into the past for stale detection. + store.mu.Lock() + a.UpdatedAt = time.Now().Add(-10 * 24 * time.Hour) + store.mu.Unlock() + + stale, err := store.GetStaleIssues(ctx, types.StaleFilter{Days: 7, Limit: 10}) + if err != nil { + t.Fatalf("GetStaleIssues: %v", err) + } + if len(stale) != 1 || stale[0].ID != a.ID { + t.Fatalf("unexpected stale: %+v", stale) + } + + if err := store.AddComment(ctx, a.ID, "actor", "c"); err != nil { + t.Fatalf("AddComment: %v", err) + } + if err := store.MarkIssueDirty(ctx, a.ID); err != nil { + t.Fatalf("MarkIssueDirty: %v", err) + } + + // Generate multiple events and ensure limiting returns the last N. + if err := store.UpdateIssue(ctx, a.ID, map[string]interface{}{"title": "t1"}, "actor"); err != nil { + t.Fatalf("UpdateIssue: %v", err) + } + if err := store.UpdateIssue(ctx, a.ID, map[string]interface{}{"title": "t2"}, "actor"); err != nil { + t.Fatalf("UpdateIssue: %v", err) + } + evs, err := store.GetEvents(ctx, a.ID, 2) + if err != nil { + t.Fatalf("GetEvents: %v", err) + } + if len(evs) != 2 { + t.Fatalf("expected 2 events, got %d", len(evs)) + } + + if err := store.SetConfig(ctx, "status.custom", " triage, blocked , ,done "); err != nil { + t.Fatalf("SetConfig: %v", err) + } + statuses, err := store.GetCustomStatuses(ctx) + if err != nil { + t.Fatalf("GetCustomStatuses: %v", err) + } + if len(statuses) != 3 || statuses[0] != "triage" || statuses[1] != "blocked" || statuses[2] != "done" { + t.Fatalf("unexpected statuses: %+v", statuses) + } + if got := parseCustomStatuses(""); got != nil { + t.Fatalf("expected nil for empty parseCustomStatuses") + } + + // Empty custom statuses. + if err := store.DeleteConfig(ctx, "status.custom"); err != nil { + t.Fatalf("DeleteConfig: %v", err) + } + statuses, err = store.GetCustomStatuses(ctx) + if err != nil { + t.Fatalf("GetCustomStatuses(empty): %v", err) + } + if statuses != nil { + t.Fatalf("expected nil statuses when unset, got %+v", statuses) + } + + if _, err := store.GetEpicsEligibleForClosure(ctx); err != nil { + t.Fatalf("GetEpicsEligibleForClosure: %v", err) + } + + if err := store.UpdateIssueID(ctx, "old", "new", nil, "actor"); err == nil { + t.Fatalf("expected UpdateIssueID error") + } + if err := store.RenameDependencyPrefix(ctx, "old", "new"); err != nil { + t.Fatalf("RenameDependencyPrefix: %v", err) + } + if err := store.RenameCounterPrefix(ctx, "old", "new"); err != nil { + t.Fatalf("RenameCounterPrefix: %v", err) + } +} + +func TestMemoryStorage_AddLabelAndAddDependency_ErrorPaths(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + issue := &types.Issue{ID: "bd-1", Title: "A", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + if err := store.CreateIssue(ctx, issue, "actor"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + + if err := store.AddLabel(ctx, "bd-missing", "l", "actor"); err == nil { + t.Fatalf("expected AddLabel error for missing issue") + } + if err := store.AddLabel(ctx, issue.ID, "l", "actor"); err != nil { + t.Fatalf("AddLabel: %v", err) + } + // Duplicate label is a no-op. + if err := store.AddLabel(ctx, issue.ID, "l", "actor"); err != nil { + t.Fatalf("AddLabel duplicate: %v", err) + } + + // AddDependency error paths. + if err := store.AddDependency(ctx, &types.Dependency{IssueID: "bd-missing", DependsOnID: issue.ID, Type: types.DepBlocks}, "actor"); err == nil { + t.Fatalf("expected AddDependency error for missing IssueID") + } + if err := store.AddDependency(ctx, &types.Dependency{IssueID: issue.ID, DependsOnID: "bd-missing", Type: types.DepBlocks}, "actor"); err == nil { + t.Fatalf("expected AddDependency error for missing DependsOnID") + } +} + +func TestMemoryStorage_GetNextChildID_Errors(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + if _, err := store.GetNextChildID(ctx, "bd-missing"); err == nil { + t.Fatalf("expected error for missing parent") + } + + deep := &types.Issue{ID: "bd-1.1.1.1", Title: "Deep", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + if err := store.CreateIssue(ctx, deep, "actor"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + if _, err := store.GetNextChildID(ctx, deep.ID); err == nil { + t.Fatalf("expected max depth error") + } +} + +func TestMemoryStorage_GetAllIssues_AttachesDependenciesAndComments(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + a := &types.Issue{ID: "bd-1", Title: "A", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + b := &types.Issue{ID: "bd-2", Title: "B", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + if err := store.CreateIssue(ctx, a, "actor"); err != nil { + t.Fatalf("CreateIssue a: %v", err) + } + if err := store.CreateIssue(ctx, b, "actor"); err != nil { + t.Fatalf("CreateIssue b: %v", err) + } + if err := store.AddDependency(ctx, &types.Dependency{IssueID: a.ID, DependsOnID: b.ID, Type: types.DepBlocks}, "actor"); err != nil { + t.Fatalf("AddDependency: %v", err) + } + if _, err := store.AddIssueComment(ctx, a.ID, "author", "text"); err != nil { + t.Fatalf("AddIssueComment: %v", err) + } + + all := store.GetAllIssues() + var gotA *types.Issue + for _, iss := range all { + if iss.ID == a.ID { + gotA = iss + break + } + } + if gotA == nil { + t.Fatalf("expected to find issue A") + } + if len(gotA.Dependencies) != 1 || gotA.Dependencies[0].DependsOnID != b.ID { + t.Fatalf("expected deps attached") + } + if len(gotA.Comments) != 1 || gotA.Comments[0].Text != "text" { + t.Fatalf("expected comments attached") + } +} + +func TestMemoryStorage_GetStaleIssues_FilteringAndLimit(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + old := &types.Issue{ID: "bd-1", Title: "Old", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + newer := &types.Issue{ID: "bd-2", Title: "Newer", Status: types.StatusInProgress, Priority: 1, IssueType: types.TypeTask} + closed := &types.Issue{ID: "bd-3", Title: "Closed", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + for _, iss := range []*types.Issue{old, newer, closed} { + if err := store.CreateIssue(ctx, iss, "actor"); err != nil { + t.Fatalf("CreateIssue %s: %v", iss.ID, err) + } + } + if err := store.CloseIssue(ctx, closed.ID, "done", "actor"); err != nil { + t.Fatalf("CloseIssue: %v", err) + } + + store.mu.Lock() + store.issues[old.ID].UpdatedAt = time.Now().Add(-20 * 24 * time.Hour) + store.issues[newer.ID].UpdatedAt = time.Now().Add(-10 * 24 * time.Hour) + store.issues[closed.ID].UpdatedAt = time.Now().Add(-30 * 24 * time.Hour) + store.mu.Unlock() + + stale, err := store.GetStaleIssues(ctx, types.StaleFilter{Days: 7, Status: "in_progress"}) + if err != nil { + t.Fatalf("GetStaleIssues: %v", err) + } + if len(stale) != 1 || stale[0].ID != newer.ID { + t.Fatalf("unexpected stale filtered: %+v", stale) + } + + stale, err = store.GetStaleIssues(ctx, types.StaleFilter{Days: 7, Limit: 1}) + if err != nil { + t.Fatalf("GetStaleIssues: %v", err) + } + if len(stale) != 1 || stale[0].ID != old.ID { + t.Fatalf("expected oldest stale first, got %+v", stale) + } +} + +func TestMemoryStorage_Statistics_EpicsEligibleForClosure_Counting(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + ep := &types.Issue{ID: "bd-1", Title: "Epic", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + c1 := &types.Issue{ID: "bd-2", Title: "Child1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + c2 := &types.Issue{ID: "bd-3", Title: "Child2", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + for _, iss := range []*types.Issue{ep, c1, c2} { + if err := store.CreateIssue(ctx, iss, "actor"); err != nil { + t.Fatalf("CreateIssue %s: %v", iss.ID, err) + } + } + if err := store.CloseIssue(ctx, c1.ID, "done", "actor"); err != nil { + t.Fatalf("CloseIssue c1: %v", err) + } + if err := store.CloseIssue(ctx, c2.ID, "done", "actor"); err != nil { + t.Fatalf("CloseIssue c2: %v", err) + } + // Parent-child deps: child -> epic. + if err := store.AddDependency(ctx, &types.Dependency{IssueID: c1.ID, DependsOnID: ep.ID, Type: types.DepParentChild}, "actor"); err != nil { + t.Fatalf("AddDependency: %v", err) + } + if err := store.AddDependency(ctx, &types.Dependency{IssueID: c2.ID, DependsOnID: ep.ID, Type: types.DepParentChild}, "actor"); err != nil { + t.Fatalf("AddDependency: %v", err) + } + + stats, err := store.GetStatistics(ctx) + if err != nil { + t.Fatalf("GetStatistics: %v", err) + } + if stats.EpicsEligibleForClosure != 1 { + t.Fatalf("expected 1 epic eligible, got %d", stats.EpicsEligibleForClosure) + } +} + +func TestMemoryStorage_UpdateIssue_SearchIssues_ReadyWork_BlockedIssues(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + now := time.Now() + assignee := "alice" + + parent := &types.Issue{ID: "bd-1", Title: "Parent", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + child := &types.Issue{ID: "bd-2", Title: "Child", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask, Assignee: assignee} + blocker := &types.Issue{ID: "bd-3", Title: "Blocker", Status: types.StatusOpen, Priority: 3, IssueType: types.TypeTask} + pinned := &types.Issue{ID: "bd-4", Title: "Pinned", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask, Pinned: true} + workflow := &types.Issue{ID: "bd-5", Title: "Workflow", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeMergeRequest} + for _, iss := range []*types.Issue{parent, child, blocker, pinned, workflow} { + if err := store.CreateIssue(ctx, iss, "actor"); err != nil { + t.Fatalf("CreateIssue %s: %v", iss.ID, err) + } + } + + // Make created_at deterministic for sorting. + store.mu.Lock() + store.issues[parent.ID].CreatedAt = now.Add(-100 * time.Hour) + store.issues[child.ID].CreatedAt = now.Add(-1 * time.Hour) + store.issues[blocker.ID].CreatedAt = now.Add(-2 * time.Hour) + store.issues[pinned.ID].CreatedAt = now.Add(-3 * time.Hour) + store.issues[workflow.ID].CreatedAt = now.Add(-4 * time.Hour) + store.mu.Unlock() + + // Dependencies: child is a child of parent; child is blocked by blocker. + if err := store.AddDependency(ctx, &types.Dependency{IssueID: child.ID, DependsOnID: parent.ID, Type: types.DepParentChild}, "actor"); err != nil { + t.Fatalf("AddDependency parent-child: %v", err) + } + if err := store.AddDependency(ctx, &types.Dependency{IssueID: child.ID, DependsOnID: blocker.ID, Type: types.DepBlocks}, "actor"); err != nil { + t.Fatalf("AddDependency blocks: %v", err) + } + + // AddDependency duplicate error path. + if err := store.AddDependency(ctx, &types.Dependency{IssueID: child.ID, DependsOnID: blocker.ID, Type: types.DepBlocks}, "actor"); err == nil { + t.Fatalf("expected duplicate dependency error") + } + + // UpdateIssue: exercise assignee nil, external_ref update+clear, and closed_at behavior. + ext := "old-ext" + store.mu.Lock() + store.issues[child.ID].ExternalRef = &ext + store.externalRefToID[ext] = child.ID + store.mu.Unlock() + + if err := store.UpdateIssue(ctx, child.ID, map[string]interface{}{"assignee": nil, "external_ref": "new-ext"}, "actor"); err != nil { + t.Fatalf("UpdateIssue: %v", err) + } + if got, _ := store.GetIssueByExternalRef(ctx, "old-ext"); got != nil { + t.Fatalf("expected old-ext removed") + } + if got, _ := store.GetIssueByExternalRef(ctx, "new-ext"); got == nil || got.ID != child.ID { + t.Fatalf("expected new-ext mapping") + } + + if err := store.UpdateIssue(ctx, child.ID, map[string]interface{}{"status": string(types.StatusClosed)}, "actor"); err != nil { + t.Fatalf("UpdateIssue close: %v", err) + } + closed, _ := store.GetIssue(ctx, child.ID) + if closed.ClosedAt == nil { + t.Fatalf("expected ClosedAt set") + } + if err := store.UpdateIssue(ctx, child.ID, map[string]interface{}{"status": string(types.StatusOpen), "external_ref": nil}, "actor"); err != nil { + t.Fatalf("UpdateIssue reopen: %v", err) + } + reopened, _ := store.GetIssue(ctx, child.ID) + if reopened.ClosedAt != nil { + t.Fatalf("expected ClosedAt cleared") + } + if got, _ := store.GetIssueByExternalRef(ctx, "new-ext"); got != nil { + t.Fatalf("expected new-ext cleared") + } + + // SearchIssues: query, label AND/OR, IDs filter, ParentID filter, limit. + if err := store.AddLabel(ctx, parent.ID, "l1", "actor"); err != nil { + t.Fatalf("AddLabel: %v", err) + } + if err := store.AddLabel(ctx, child.ID, "l1", "actor"); err != nil { + t.Fatalf("AddLabel: %v", err) + } + if err := store.AddLabel(ctx, child.ID, "l2", "actor"); err != nil { + t.Fatalf("AddLabel: %v", err) + } + + st := types.StatusOpen + res, err := store.SearchIssues(ctx, "parent", types.IssueFilter{Status: &st}) + if err != nil { + t.Fatalf("SearchIssues: %v", err) + } + if len(res) != 1 || res[0].ID != parent.ID { + t.Fatalf("unexpected SearchIssues results: %+v", res) + } + + res, err = store.SearchIssues(ctx, "", types.IssueFilter{Labels: []string{"l1", "l2"}}) + if err != nil { + t.Fatalf("SearchIssues labels AND: %v", err) + } + if len(res) != 1 || res[0].ID != child.ID { + t.Fatalf("unexpected labels AND results: %+v", res) + } + + res, err = store.SearchIssues(ctx, "", types.IssueFilter{IDs: []string{child.ID}}) + if err != nil { + t.Fatalf("SearchIssues IDs: %v", err) + } + if len(res) != 1 || res[0].ID != child.ID { + t.Fatalf("unexpected IDs results: %+v", res) + } + + res, err = store.SearchIssues(ctx, "", types.IssueFilter{ParentID: &parent.ID}) + if err != nil { + t.Fatalf("SearchIssues ParentID: %v", err) + } + if len(res) != 1 || res[0].ID != child.ID { + t.Fatalf("unexpected ParentID results: %+v", res) + } + + res, err = store.SearchIssues(ctx, "", types.IssueFilter{LabelsAny: []string{"l2", "missing"}, Limit: 1}) + if err != nil { + t.Fatalf("SearchIssues labels OR: %v", err) + } + if len(res) != 1 { + t.Fatalf("expected limit 1") + } + + // Ready work: child is blocked, pinned excluded, workflow excluded by default. + ready, err := store.GetReadyWork(ctx, types.WorkFilter{}) + if err != nil { + t.Fatalf("GetReadyWork: %v", err) + } + if len(ready) != 2 { // parent + blocker + t.Fatalf("expected 2 ready issues, got %d: %+v", len(ready), ready) + } + + // Filter by workflow type explicitly. + ready, err = store.GetReadyWork(ctx, types.WorkFilter{Type: string(types.TypeMergeRequest)}) + if err != nil { + t.Fatalf("GetReadyWork type: %v", err) + } + if len(ready) != 1 || ready[0].ID != workflow.ID { + t.Fatalf("expected only workflow issue, got %+v", ready) + } + + // Status + priority filters. + prio := 3 + ready, err = store.GetReadyWork(ctx, types.WorkFilter{Status: types.StatusOpen, Priority: &prio}) + if err != nil { + t.Fatalf("GetReadyWork status+priority: %v", err) + } + if len(ready) != 1 || ready[0].ID != blocker.ID { + t.Fatalf("expected blocker only, got %+v", ready) + } + + // Label filters. + ready, err = store.GetReadyWork(ctx, types.WorkFilter{Labels: []string{"l1"}}) + if err != nil { + t.Fatalf("GetReadyWork labels AND: %v", err) + } + if len(ready) != 1 || ready[0].ID != parent.ID { + t.Fatalf("expected parent only, got %+v", ready) + } + ready, err = store.GetReadyWork(ctx, types.WorkFilter{LabelsAny: []string{"l2"}}) + if err != nil { + t.Fatalf("GetReadyWork labels OR: %v", err) + } + if len(ready) != 0 { + t.Fatalf("expected 0 because only l2 issue is blocked") + } + + // Assignee filter vs Unassigned precedence. + ready, err = store.GetReadyWork(ctx, types.WorkFilter{Assignee: &assignee}) + if err != nil { + t.Fatalf("GetReadyWork assignee: %v", err) + } + if len(ready) != 0 { + t.Fatalf("expected 0 due to child being blocked") + } + ready, err = store.GetReadyWork(ctx, types.WorkFilter{Unassigned: true}) + if err != nil { + t.Fatalf("GetReadyWork unassigned: %v", err) + } + for _, iss := range ready { + if iss.Assignee != "" { + t.Fatalf("expected unassigned only") + } + } + + // Sort policies + limit. + ready, err = store.GetReadyWork(ctx, types.WorkFilter{SortPolicy: types.SortPolicyOldest, Limit: 1}) + if err != nil { + t.Fatalf("GetReadyWork oldest: %v", err) + } + if len(ready) != 1 || ready[0].ID != parent.ID { + t.Fatalf("expected oldest=parent, got %+v", ready) + } + ready, err = store.GetReadyWork(ctx, types.WorkFilter{SortPolicy: types.SortPolicyPriority}) + if err != nil { + t.Fatalf("GetReadyWork priority: %v", err) + } + if len(ready) < 2 || ready[0].Priority > ready[1].Priority { + t.Fatalf("expected priority sort") + } + // Hybrid: recent issues first. + ready, err = store.GetReadyWork(ctx, types.WorkFilter{SortPolicy: types.SortPolicyHybrid}) + if err != nil { + t.Fatalf("GetReadyWork hybrid: %v", err) + } + if len(ready) != 2 || ready[0].ID != blocker.ID { + t.Fatalf("expected recent (blocker) first in hybrid, got %+v", ready) + } + + // Blocked issues: child is blocked by an open blocker. + blocked, err := store.GetBlockedIssues(ctx) + if err != nil { + t.Fatalf("GetBlockedIssues: %v", err) + } + if len(blocked) != 1 || blocked[0].ID != child.ID || blocked[0].BlockedByCount != 1 { + t.Fatalf("unexpected blocked issues: %+v", blocked) + } + + // Cover getOpenBlockers missing-blocker branch. + missing := &types.Issue{ID: "bd-6", Title: "Missing blocker dep", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + if err := store.CreateIssue(ctx, missing, "actor"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + // Bypass AddDependency validation to cover the missing-blocker branch in getOpenBlockers. + store.mu.Lock() + store.dependencies[missing.ID] = append(store.dependencies[missing.ID], &types.Dependency{IssueID: missing.ID, DependsOnID: "bd-does-not-exist", Type: types.DepBlocks}) + store.mu.Unlock() + blocked, err = store.GetBlockedIssues(ctx) + if err != nil { + t.Fatalf("GetBlockedIssues: %v", err) + } + if len(blocked) != 2 { + t.Fatalf("expected 2 blocked issues, got %d", len(blocked)) + } +} + +func TestMemoryStorage_UpdateIssue_CoversMoreFields(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + iss := &types.Issue{ID: "bd-1", Title: "A", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + if err := store.CreateIssue(ctx, iss, "actor"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + + if err := store.UpdateIssue(ctx, iss.ID, map[string]interface{}{ + "description": "d", + "design": "design", + "acceptance_criteria": "ac", + "notes": "n", + "priority": 2, + "issue_type": string(types.TypeBug), + "assignee": "bob", + "status": string(types.StatusInProgress), + }, "actor"); err != nil { + t.Fatalf("UpdateIssue: %v", err) + } + + got, _ := store.GetIssue(ctx, iss.ID) + if got.Description != "d" || got.Design != "design" || got.AcceptanceCriteria != "ac" || got.Notes != "n" { + t.Fatalf("expected text fields updated") + } + if got.Priority != 2 || got.IssueType != types.TypeBug || got.Assignee != "bob" || got.Status != types.StatusInProgress { + t.Fatalf("expected fields updated") + } + + // Status closed when already closed should not clear ClosedAt. + if err := store.CloseIssue(ctx, iss.ID, "done", "actor"); err != nil { + t.Fatalf("CloseIssue: %v", err) + } + closedOnce, _ := store.GetIssue(ctx, iss.ID) + if closedOnce.ClosedAt == nil { + t.Fatalf("expected ClosedAt") + } + if err := store.UpdateIssue(ctx, iss.ID, map[string]interface{}{"status": string(types.StatusClosed)}, "actor"); err != nil { + t.Fatalf("UpdateIssue closed->closed: %v", err) + } + closedTwice, _ := store.GetIssue(ctx, iss.ID) + if closedTwice.ClosedAt == nil { + t.Fatalf("expected ClosedAt preserved") + } +} + +func TestMemoryStorage_CountEpicsEligibleForClosure_CoversBranches(t *testing.T) { + store := setupTestMemory(t) + defer store.Close() + ctx := context.Background() + + ep1 := &types.Issue{ID: "bd-1", Title: "Epic1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + epClosed := &types.Issue{ID: "bd-2", Title: "EpicClosed", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic} + nonEpic := &types.Issue{ID: "bd-3", Title: "NotEpic", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + c := &types.Issue{ID: "bd-4", Title: "Child", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask} + for _, iss := range []*types.Issue{ep1, epClosed, nonEpic, c} { + if err := store.CreateIssue(ctx, iss, "actor"); err != nil { + t.Fatalf("CreateIssue %s: %v", iss.ID, err) + } + } + if err := store.CloseIssue(ctx, epClosed.ID, "done", "actor"); err != nil { + t.Fatalf("CloseIssue: %v", err) + } + // Child -> ep1 (eligible once child is closed). + if err := store.AddDependency(ctx, &types.Dependency{IssueID: c.ID, DependsOnID: ep1.ID, Type: types.DepParentChild}, "actor"); err != nil { + t.Fatalf("AddDependency: %v", err) + } + // Child -> nonEpic should not count. + if err := store.AddDependency(ctx, &types.Dependency{IssueID: c.ID, DependsOnID: nonEpic.ID, Type: types.DepParentChild}, "actor"); err != nil { + t.Fatalf("AddDependency: %v", err) + } + // Child -> missing epic should not count. + store.mu.Lock() + store.dependencies[c.ID] = append(store.dependencies[c.ID], &types.Dependency{IssueID: c.ID, DependsOnID: "bd-missing", Type: types.DepParentChild}) + store.mu.Unlock() + + // Close child to make ep1 eligible. + if err := store.CloseIssue(ctx, c.ID, "done", "actor"); err != nil { + t.Fatalf("CloseIssue child: %v", err) + } + + stats, err := store.GetStatistics(ctx) + if err != nil { + t.Fatalf("GetStatistics: %v", err) + } + if stats.EpicsEligibleForClosure != 1 { + t.Fatalf("expected 1 eligible epic, got %d", stats.EpicsEligibleForClosure) + } +} + +func TestExtractParentAndChildNumber_CoversFailures(t *testing.T) { + if _, _, ok := extractParentAndChildNumber("no-dot"); ok { + t.Fatalf("expected ok=false") + } + if _, _, ok := extractParentAndChildNumber("parent.bad"); ok { + t.Fatalf("expected ok=false") + } +} From 6790fb2f2d7771907cb53aaffb28b2cf4eb9c11f Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 10:19:30 -0400 Subject: [PATCH 08/13] test: cover rpc client gate + shutdown Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- internal/rpc/client_gate_shutdown_test.go | 107 ++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 internal/rpc/client_gate_shutdown_test.go diff --git a/internal/rpc/client_gate_shutdown_test.go b/internal/rpc/client_gate_shutdown_test.go new file mode 100644 index 00000000..66cacfe5 --- /dev/null +++ b/internal/rpc/client_gate_shutdown_test.go @@ -0,0 +1,107 @@ +package rpc + +import ( + "encoding/json" + "testing" + "time" + + "github.com/steveyegge/beads/internal/types" +) + +func TestClient_GateLifecycleAndShutdown(t *testing.T) { + _, client, cleanup := setupTestServer(t) + defer cleanup() + + createResp, err := client.GateCreate(&GateCreateArgs{ + Title: "Test Gate", + AwaitType: "human", + AwaitID: "", + Timeout: 5 * time.Minute, + Waiters: []string{"mayor/"}, + }) + if err != nil { + t.Fatalf("GateCreate: %v", err) + } + + var created GateCreateResult + if err := json.Unmarshal(createResp.Data, &created); err != nil { + t.Fatalf("unmarshal GateCreateResult: %v", err) + } + if created.ID == "" { + t.Fatalf("expected created gate ID") + } + + listResp, err := client.GateList(&GateListArgs{All: false}) + if err != nil { + t.Fatalf("GateList: %v", err) + } + var openGates []*types.Issue + if err := json.Unmarshal(listResp.Data, &openGates); err != nil { + t.Fatalf("unmarshal GateList: %v", err) + } + if len(openGates) != 1 || openGates[0].ID != created.ID { + t.Fatalf("unexpected open gates: %+v", openGates) + } + + showResp, err := client.GateShow(&GateShowArgs{ID: created.ID}) + if err != nil { + t.Fatalf("GateShow: %v", err) + } + var gate types.Issue + if err := json.Unmarshal(showResp.Data, &gate); err != nil { + t.Fatalf("unmarshal GateShow: %v", err) + } + if gate.ID != created.ID || gate.IssueType != types.TypeGate { + t.Fatalf("unexpected gate: %+v", gate) + } + + waitResp, err := client.GateWait(&GateWaitArgs{ID: created.ID, Waiters: []string{"deacon/"}}) + if err != nil { + t.Fatalf("GateWait: %v", err) + } + var waitResult GateWaitResult + if err := json.Unmarshal(waitResp.Data, &waitResult); err != nil { + t.Fatalf("unmarshal GateWaitResult: %v", err) + } + if waitResult.AddedCount != 1 { + t.Fatalf("expected 1 waiter added, got %d", waitResult.AddedCount) + } + + closeResp, err := client.GateClose(&GateCloseArgs{ID: created.ID, Reason: "done"}) + if err != nil { + t.Fatalf("GateClose: %v", err) + } + var closedGate types.Issue + if err := json.Unmarshal(closeResp.Data, &closedGate); err != nil { + t.Fatalf("unmarshal GateClose: %v", err) + } + if closedGate.Status != types.StatusClosed { + t.Fatalf("expected closed status, got %q", closedGate.Status) + } + + listResp, err = client.GateList(&GateListArgs{All: false}) + if err != nil { + t.Fatalf("GateList open: %v", err) + } + if err := json.Unmarshal(listResp.Data, &openGates); err != nil { + t.Fatalf("unmarshal GateList open: %v", err) + } + if len(openGates) != 0 { + t.Fatalf("expected no open gates, got %+v", openGates) + } + + listResp, err = client.GateList(&GateListArgs{All: true}) + if err != nil { + t.Fatalf("GateList all: %v", err) + } + if err := json.Unmarshal(listResp.Data, &openGates); err != nil { + t.Fatalf("unmarshal GateList all: %v", err) + } + if len(openGates) != 1 || openGates[0].ID != created.ID { + t.Fatalf("expected 1 total gate, got %+v", openGates) + } + + if err := client.Shutdown(); err != nil { + t.Fatalf("Shutdown: %v", err) + } +} From 1f5ed550fe84541e6c6a2e1ad022d26f5727718b Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 10:45:42 -0400 Subject: [PATCH 09/13] test: cover daemon autostart paths Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- cmd/bd/daemon_autostart.go | 52 +++-- cmd/bd/daemon_autostart_unit_test.go | 331 +++++++++++++++++++++++++++ 2 files changed, 363 insertions(+), 20 deletions(-) create mode 100644 cmd/bd/daemon_autostart_unit_test.go diff --git a/cmd/bd/daemon_autostart.go b/cmd/bd/daemon_autostart.go index d858c7d8..4fdd0cc0 100644 --- a/cmd/bd/daemon_autostart.go +++ b/cmd/bd/daemon_autostart.go @@ -31,6 +31,19 @@ var ( daemonStartFailures int ) +var ( + executableFn = os.Executable + execCommandFn = exec.Command + openFileFn = os.OpenFile + findProcessFn = os.FindProcess + removeFileFn = os.Remove + configureDaemonProcessFn = configureDaemonProcess + waitForSocketReadinessFn = waitForSocketReadiness + startDaemonProcessFn = startDaemonProcess + isDaemonRunningFn = isDaemonRunning + sendStopSignalFn = sendStopSignal +) + // shouldAutoStartDaemon checks if daemon auto-start is enabled func shouldAutoStartDaemon() bool { // Check BEADS_NO_DAEMON first (escape hatch for single-user workflows) @@ -53,7 +66,6 @@ func shouldAutoStartDaemon() bool { return config.GetBool("auto-start-daemon") // Defaults to true } - // restartDaemonForVersionMismatch stops the old daemon and starts a new one // Returns true if restart was successful func restartDaemonForVersionMismatch() bool { @@ -67,17 +79,17 @@ func restartDaemonForVersionMismatch() bool { // Check if daemon is running and stop it forcedKill := false - if isRunning, pid := isDaemonRunning(pidFile); isRunning { + if isRunning, pid := isDaemonRunningFn(pidFile); isRunning { debug.Logf("stopping old daemon (PID %d)", pid) - process, err := os.FindProcess(pid) + process, err := findProcessFn(pid) if err != nil { debug.Logf("failed to find process: %v", err) return false } // Send stop signal - if err := sendStopSignal(process); err != nil { + if err := sendStopSignalFn(process); err != nil { debug.Logf("failed to signal daemon: %v", err) return false } @@ -85,14 +97,14 @@ func restartDaemonForVersionMismatch() bool { // Wait for daemon to stop, then force kill for i := 0; i < daemonShutdownAttempts; i++ { time.Sleep(daemonShutdownPollInterval) - if isRunning, _ := isDaemonRunning(pidFile); !isRunning { + if isRunning, _ := isDaemonRunningFn(pidFile); !isRunning { debug.Logf("old daemon stopped successfully") break } } // Force kill if still running - if isRunning, _ := isDaemonRunning(pidFile); isRunning { + if isRunning, _ := isDaemonRunningFn(pidFile); isRunning { debug.Logf("force killing old daemon") _ = process.Kill() forcedKill = true @@ -101,19 +113,19 @@ func restartDaemonForVersionMismatch() bool { // Clean up stale socket and PID file after force kill or if not running if forcedKill || !isDaemonRunningQuiet(pidFile) { - _ = os.Remove(socketPath) - _ = os.Remove(pidFile) + _ = removeFileFn(socketPath) + _ = removeFileFn(pidFile) } // Start new daemon with current binary version - exe, err := os.Executable() + exe, err := executableFn() if err != nil { debug.Logf("failed to get executable path: %v", err) return false } args := []string{"daemon", "--start"} - cmd := exec.Command(exe, args...) + cmd := execCommandFn(exe, args...) cmd.Env = append(os.Environ(), "BD_DAEMON_FOREGROUND=1") // Set working directory to database directory so daemon finds correct DB @@ -121,9 +133,9 @@ func restartDaemonForVersionMismatch() bool { cmd.Dir = filepath.Dir(dbPath) } - configureDaemonProcess(cmd) + configureDaemonProcessFn(cmd) - devNull, err := os.OpenFile(os.DevNull, os.O_RDWR, 0) + devNull, err := openFileFn(os.DevNull, os.O_RDWR, 0) if err == nil { cmd.Stdin = devNull cmd.Stdout = devNull @@ -140,7 +152,7 @@ func restartDaemonForVersionMismatch() bool { go func() { _ = cmd.Wait() }() // Wait for daemon to be ready using shared helper - if waitForSocketReadiness(socketPath, 5*time.Second) { + if waitForSocketReadinessFn(socketPath, 5*time.Second) { debug.Logf("new daemon started successfully") return true } @@ -153,7 +165,7 @@ func restartDaemonForVersionMismatch() bool { // isDaemonRunningQuiet checks if daemon is running without output func isDaemonRunningQuiet(pidFile string) bool { - isRunning, _ := isDaemonRunning(pidFile) + isRunning, _ := isDaemonRunningFn(pidFile) return isRunning } @@ -185,7 +197,7 @@ func tryAutoStartDaemon(socketPath string) bool { } socketPath = determineSocketPath(socketPath) - return startDaemonProcess(socketPath) + return startDaemonProcessFn(socketPath) } func debugLog(msg string, args ...interface{}) { @@ -269,21 +281,21 @@ func determineSocketPath(socketPath string) string { } func startDaemonProcess(socketPath string) bool { - binPath, err := os.Executable() + binPath, err := executableFn() if err != nil { binPath = os.Args[0] } args := []string{"daemon", "--start"} - cmd := exec.Command(binPath, args...) + cmd := execCommandFn(binPath, args...) setupDaemonIO(cmd) if dbPath != "" { cmd.Dir = filepath.Dir(dbPath) } - configureDaemonProcess(cmd) + configureDaemonProcessFn(cmd) if err := cmd.Start(); err != nil { recordDaemonStartFailure() debugLog("failed to start daemon: %v", err) @@ -292,7 +304,7 @@ func startDaemonProcess(socketPath string) bool { go func() { _ = cmd.Wait() }() - if waitForSocketReadiness(socketPath, 5*time.Second) { + if waitForSocketReadinessFn(socketPath, 5*time.Second) { recordDaemonStartSuccess() return true } @@ -306,7 +318,7 @@ func startDaemonProcess(socketPath string) bool { } func setupDaemonIO(cmd *exec.Cmd) { - devNull, err := os.OpenFile(os.DevNull, os.O_RDWR, 0) + devNull, err := openFileFn(os.DevNull, os.O_RDWR, 0) if err == nil { cmd.Stdout = devNull cmd.Stderr = devNull diff --git a/cmd/bd/daemon_autostart_unit_test.go b/cmd/bd/daemon_autostart_unit_test.go new file mode 100644 index 00000000..625cedf6 --- /dev/null +++ b/cmd/bd/daemon_autostart_unit_test.go @@ -0,0 +1,331 @@ +package main + +import ( + "bytes" + "context" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + "time" + + "github.com/steveyegge/beads/internal/config" +) + +func tempSockDir(t *testing.T) string { + t.Helper() + + base := "/tmp" + if runtime.GOOS == windowsOS { + base = os.TempDir() + } else if _, err := os.Stat(base); err != nil { + base = os.TempDir() + } + + d, err := os.MkdirTemp(base, "bd-sock-*") + if err != nil { + t.Fatalf("MkdirTemp: %v", err) + } + t.Cleanup(func() { _ = os.RemoveAll(d) }) + return d +} + +func startTestRPCServer(t *testing.T) (socketPath string, cleanup func()) { + t.Helper() + + tmpDir := tempSockDir(t) + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0o750); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + socketPath = filepath.Join(beadsDir, "bd.sock") + db := filepath.Join(beadsDir, "test.db") + store := newTestStore(t, db) + + ctx, cancel := context.WithCancel(context.Background()) + log := newTestLogger() + + server, _, err := startRPCServer(ctx, socketPath, store, tmpDir, db, log) + if err != nil { + cancel() + t.Fatalf("startRPCServer: %v", err) + } + + cleanup = func() { + cancel() + if server != nil { + _ = server.Stop() + } + } + + return socketPath, cleanup +} + +func captureStderr(t *testing.T, fn func()) string { + t.Helper() + + old := os.Stderr + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + os.Stderr = w + + var buf bytes.Buffer + done := make(chan struct{}) + go func() { + _, _ = io.Copy(&buf, r) + close(done) + }() + + fn() + _ = w.Close() + os.Stderr = old + <-done + _ = r.Close() + + return buf.String() +} + +func TestDaemonAutostart_AcquireStartLock_CreatesAndCleansStale(t *testing.T) { + tmpDir := t.TempDir() + lockPath := filepath.Join(tmpDir, "bd.sock.startlock") + pid, err := readPIDFromFile(lockPath) + if err == nil || pid != 0 { + // lock doesn't exist yet; expect read to fail. + } + + if !acquireStartLock(lockPath, filepath.Join(tmpDir, "bd.sock")) { + t.Fatalf("expected acquireStartLock to succeed") + } + got, err := readPIDFromFile(lockPath) + if err != nil { + t.Fatalf("readPIDFromFile: %v", err) + } + if got != os.Getpid() { + t.Fatalf("expected lock PID %d, got %d", os.Getpid(), got) + } + + // Stale lock: dead/unreadable PID should be removed and recreated. + if err := os.WriteFile(lockPath, []byte("0\n"), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + if !acquireStartLock(lockPath, filepath.Join(tmpDir, "bd.sock")) { + t.Fatalf("expected acquireStartLock to succeed on stale lock") + } + got, err = readPIDFromFile(lockPath) + if err != nil { + t.Fatalf("readPIDFromFile: %v", err) + } + if got != os.Getpid() { + t.Fatalf("expected recreated lock PID %d, got %d", os.Getpid(), got) + } +} + +func TestDaemonAutostart_SocketHealthAndReadiness(t *testing.T) { + socketPath, cleanup := startTestRPCServer(t) + defer cleanup() + + if !canDialSocket(socketPath, 500*time.Millisecond) { + t.Fatalf("expected canDialSocket to succeed") + } + if !isDaemonHealthy(socketPath) { + t.Fatalf("expected isDaemonHealthy to succeed") + } + if !waitForSocketReadiness(socketPath, 500*time.Millisecond) { + t.Fatalf("expected waitForSocketReadiness to succeed") + } + + missing := filepath.Join(tempSockDir(t), "missing.sock") + if canDialSocket(missing, 50*time.Millisecond) { + t.Fatalf("expected canDialSocket to fail") + } + if waitForSocketReadiness(missing, 200*time.Millisecond) { + t.Fatalf("expected waitForSocketReadiness to time out") + } +} + +func TestDaemonAutostart_HandleExistingSocket(t *testing.T) { + socketPath, cleanup := startTestRPCServer(t) + defer cleanup() + + if !handleExistingSocket(socketPath) { + t.Fatalf("expected handleExistingSocket true for running daemon") + } +} + +func TestDaemonAutostart_HandleExistingSocket_StaleCleansUp(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0o750); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + socketPath := filepath.Join(beadsDir, "bd.sock") + pidFile := filepath.Join(beadsDir, "daemon.pid") + if err := os.WriteFile(socketPath, []byte("not-a-socket"), 0o600); err != nil { + t.Fatalf("WriteFile socket: %v", err) + } + if err := os.WriteFile(pidFile, []byte("0\n"), 0o600); err != nil { + t.Fatalf("WriteFile pid: %v", err) + } + + if handleExistingSocket(socketPath) { + t.Fatalf("expected false for stale socket") + } + if _, err := os.Stat(socketPath); !os.IsNotExist(err) { + t.Fatalf("expected socket removed") + } + if _, err := os.Stat(pidFile); !os.IsNotExist(err) { + t.Fatalf("expected pidfile removed") + } +} + +func TestDaemonAutostart_TryAutoStartDaemon_EarlyExits(t *testing.T) { + oldFailures := daemonStartFailures + oldLast := lastDaemonStartAttempt + defer func() { + daemonStartFailures = oldFailures + lastDaemonStartAttempt = oldLast + }() + + daemonStartFailures = 1 + lastDaemonStartAttempt = time.Now() + if tryAutoStartDaemon(filepath.Join(t.TempDir(), "bd.sock")) { + t.Fatalf("expected tryAutoStartDaemon to skip due to backoff") + } + + daemonStartFailures = 0 + lastDaemonStartAttempt = time.Time{} + socketPath, cleanup := startTestRPCServer(t) + defer cleanup() + if !tryAutoStartDaemon(socketPath) { + t.Fatalf("expected tryAutoStartDaemon true when daemon already healthy") + } +} + +func TestDaemonAutostart_MiscHelpers(t *testing.T) { + if determineSocketPath("/x") != "/x" { + t.Fatalf("determineSocketPath should be identity") + } + + if err := config.Initialize(); err != nil { + t.Fatalf("config.Initialize: %v", err) + } + old := config.GetDuration("flush-debounce") + defer config.Set("flush-debounce", old) + + config.Set("flush-debounce", 0) + if got := getDebounceDuration(); got != 5*time.Second { + t.Fatalf("expected default debounce 5s, got %v", got) + } + config.Set("flush-debounce", 2*time.Second) + if got := getDebounceDuration(); got != 2*time.Second { + t.Fatalf("expected debounce 2s, got %v", got) + } +} + +func TestDaemonAutostart_EmitVerboseWarning(t *testing.T) { + old := daemonStatus + defer func() { daemonStatus = old }() + + daemonStatus.SocketPath = "/tmp/bd.sock" + for _, tt := range []struct { + reason string + shouldWrite bool + }{ + {FallbackConnectFailed, true}, + {FallbackHealthFailed, true}, + {FallbackAutoStartDisabled, true}, + {FallbackAutoStartFailed, true}, + {FallbackDaemonUnsupported, true}, + {FallbackWorktreeSafety, false}, + {FallbackFlagNoDaemon, false}, + } { + t.Run(tt.reason, func(t *testing.T) { + daemonStatus.FallbackReason = tt.reason + out := captureStderr(t, emitVerboseWarning) + if tt.shouldWrite && out == "" { + t.Fatalf("expected output") + } + if !tt.shouldWrite && out != "" { + t.Fatalf("expected no output, got %q", out) + } + }) + } +} + +func TestDaemonAutostart_StartDaemonProcess_Stubbed(t *testing.T) { + oldExec := execCommandFn + oldWait := waitForSocketReadinessFn + oldCfg := configureDaemonProcessFn + defer func() { + execCommandFn = oldExec + waitForSocketReadinessFn = oldWait + configureDaemonProcessFn = oldCfg + }() + + execCommandFn = func(string, ...string) *exec.Cmd { + return exec.Command(os.Args[0], "-test.run=^$") + } + waitForSocketReadinessFn = func(string, time.Duration) bool { return true } + configureDaemonProcessFn = func(*exec.Cmd) {} + + if !startDaemonProcess(filepath.Join(t.TempDir(), "bd.sock")) { + t.Fatalf("expected startDaemonProcess true when readiness stubbed") + } +} + +func TestDaemonAutostart_RestartDaemonForVersionMismatch_Stubbed(t *testing.T) { + oldExec := execCommandFn + oldWait := waitForSocketReadinessFn + oldRun := isDaemonRunningFn + oldCfg := configureDaemonProcessFn + defer func() { + execCommandFn = oldExec + waitForSocketReadinessFn = oldWait + isDaemonRunningFn = oldRun + configureDaemonProcessFn = oldCfg + }() + + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0o750); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + oldDB := dbPath + defer func() { dbPath = oldDB }() + dbPath = filepath.Join(beadsDir, "test.db") + + pidFile, err := getPIDFilePath() + if err != nil { + t.Fatalf("getPIDFilePath: %v", err) + } + sock := getSocketPath() + if err := os.WriteFile(pidFile, []byte("999999\n"), 0o600); err != nil { + t.Fatalf("WriteFile pid: %v", err) + } + if err := os.WriteFile(sock, []byte("stale"), 0o600); err != nil { + t.Fatalf("WriteFile sock: %v", err) + } + + execCommandFn = func(string, ...string) *exec.Cmd { + return exec.Command(os.Args[0], "-test.run=^$") + } + waitForSocketReadinessFn = func(string, time.Duration) bool { return true } + isDaemonRunningFn = func(string) (bool, int) { return false, 0 } + configureDaemonProcessFn = func(*exec.Cmd) {} + + if !restartDaemonForVersionMismatch() { + t.Fatalf("expected restartDaemonForVersionMismatch true when stubbed") + } + if _, err := os.Stat(pidFile); !os.IsNotExist(err) { + t.Fatalf("expected pidfile removed") + } + if _, err := os.Stat(sock); !os.IsNotExist(err) { + t.Fatalf("expected socket removed") + } +} From 59413baac28930658388c6f0624ba7e49140c5a3 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 12:04:06 -0400 Subject: [PATCH 10/13] sqlite: stop migrations clobbering pinned/template Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- .../sqlite/migrations/019_messaging_fields.go | 10 - .../migrations/021_migrate_edge_fields.go | 249 ++++++++++-------- .../migrations/022_drop_edge_columns.go | 64 ++++- .../sqlite/migrations/023_pinned_column.go | 5 + .../migrations/024_is_template_column.go | 5 + ...rations_template_pinned_regression_test.go | 59 +++++ 6 files changed, 275 insertions(+), 117 deletions(-) create mode 100644 internal/storage/sqlite/migrations_template_pinned_regression_test.go diff --git a/internal/storage/sqlite/migrations/019_messaging_fields.go b/internal/storage/sqlite/migrations/019_messaging_fields.go index d5eddd65..6b44d237 100644 --- a/internal/storage/sqlite/migrations/019_messaging_fields.go +++ b/internal/storage/sqlite/migrations/019_messaging_fields.go @@ -20,10 +20,6 @@ func MigrateMessagingFields(db *sql.DB) error { }{ {"sender", "TEXT DEFAULT ''"}, {"ephemeral", "INTEGER DEFAULT 0"}, - {"replies_to", "TEXT DEFAULT ''"}, - {"relates_to", "TEXT DEFAULT ''"}, - {"duplicate_of", "TEXT DEFAULT ''"}, - {"superseded_by", "TEXT DEFAULT ''"}, } for _, col := range columns { @@ -59,11 +55,5 @@ func MigrateMessagingFields(db *sql.DB) error { return fmt.Errorf("failed to create sender index: %w", err) } - // Add index for replies_to (for efficient thread queries) - _, err = db.Exec(`CREATE INDEX IF NOT EXISTS idx_issues_replies_to ON issues(replies_to) WHERE replies_to != ''`) - if err != nil { - return fmt.Errorf("failed to create replies_to index: %w", err) - } - return nil } diff --git a/internal/storage/sqlite/migrations/021_migrate_edge_fields.go b/internal/storage/sqlite/migrations/021_migrate_edge_fields.go index 35f1b295..69013a54 100644 --- a/internal/storage/sqlite/migrations/021_migrate_edge_fields.go +++ b/internal/storage/sqlite/migrations/021_migrate_edge_fields.go @@ -21,137 +21,176 @@ import ( func MigrateEdgeFields(db *sql.DB) error { now := time.Now() + hasColumn := func(name string) (bool, error) { + var exists bool + err := db.QueryRow(` + SELECT COUNT(*) > 0 + FROM pragma_table_info('issues') + WHERE name = ? + `, name).Scan(&exists) + return exists, err + } + + hasRepliesTo, err := hasColumn("replies_to") + if err != nil { + return fmt.Errorf("failed to check replies_to column: %w", err) + } + hasRelatesTo, err := hasColumn("relates_to") + if err != nil { + return fmt.Errorf("failed to check relates_to column: %w", err) + } + hasDuplicateOf, err := hasColumn("duplicate_of") + if err != nil { + return fmt.Errorf("failed to check duplicate_of column: %w", err) + } + hasSupersededBy, err := hasColumn("superseded_by") + if err != nil { + return fmt.Errorf("failed to check superseded_by column: %w", err) + } + + if !hasRepliesTo && !hasRelatesTo && !hasDuplicateOf && !hasSupersededBy { + return nil + } + // Migrate replies_to fields to replies-to edges // For thread_id, use the parent's ID as the thread root for first-level replies // (more sophisticated thread detection would require recursive queries) - rows, err := db.Query(` - SELECT id, replies_to - FROM issues - WHERE replies_to != '' AND replies_to IS NOT NULL - `) - if err != nil { - return fmt.Errorf("failed to query replies_to fields: %w", err) - } - defer rows.Close() - - for rows.Next() { - var issueID, repliesTo string - if err := rows.Scan(&issueID, &repliesTo); err != nil { - return fmt.Errorf("failed to scan replies_to row: %w", err) - } - - // Use repliesTo as thread_id (the root of the thread) - // This is a simplification - existing threads will have the parent as thread root - _, err := db.Exec(` - INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id) - VALUES (?, ?, 'replies-to', ?, 'migration', '{}', ?) - `, issueID, repliesTo, now, repliesTo) + if hasRepliesTo { + rows, err := db.Query(` + SELECT id, replies_to + FROM issues + WHERE replies_to != '' AND replies_to IS NOT NULL + `) if err != nil { - return fmt.Errorf("failed to create replies-to edge for %s: %w", issueID, err) + return fmt.Errorf("failed to query replies_to fields: %w", err) + } + defer rows.Close() + + for rows.Next() { + var issueID, repliesTo string + if err := rows.Scan(&issueID, &repliesTo); err != nil { + return fmt.Errorf("failed to scan replies_to row: %w", err) + } + + // Use repliesTo as thread_id (the root of the thread) + // This is a simplification - existing threads will have the parent as thread root + _, err := db.Exec(` + INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id) + VALUES (?, ?, 'replies-to', ?, 'migration', '{}', ?) + `, issueID, repliesTo, now, repliesTo) + if err != nil { + return fmt.Errorf("failed to create replies-to edge for %s: %w", issueID, err) + } + } + if err := rows.Err(); err != nil { + return fmt.Errorf("error iterating replies_to rows: %w", err) } - } - if err := rows.Err(); err != nil { - return fmt.Errorf("error iterating replies_to rows: %w", err) } // Migrate relates_to fields to relates-to edges // relates_to is stored as JSON array string - rows, err = db.Query(` - SELECT id, relates_to - FROM issues - WHERE relates_to != '' AND relates_to != '[]' AND relates_to IS NOT NULL - `) - if err != nil { - return fmt.Errorf("failed to query relates_to fields: %w", err) - } - defer rows.Close() - - for rows.Next() { - var issueID, relatesTo string - if err := rows.Scan(&issueID, &relatesTo); err != nil { - return fmt.Errorf("failed to scan relates_to row: %w", err) + if hasRelatesTo { + rows, err := db.Query(` + SELECT id, relates_to + FROM issues + WHERE relates_to != '' AND relates_to != '[]' AND relates_to IS NOT NULL + `) + if err != nil { + return fmt.Errorf("failed to query relates_to fields: %w", err) } + defer rows.Close() - // Parse JSON array - var relatedIDs []string - if err := json.Unmarshal([]byte(relatesTo), &relatedIDs); err != nil { - // Skip malformed JSON - continue - } + for rows.Next() { + var issueID, relatesTo string + if err := rows.Scan(&issueID, &relatesTo); err != nil { + return fmt.Errorf("failed to scan relates_to row: %w", err) + } - for _, relatedID := range relatedIDs { - if relatedID == "" { + // Parse JSON array + var relatedIDs []string + if err := json.Unmarshal([]byte(relatesTo), &relatedIDs); err != nil { + // Skip malformed JSON continue } - _, err := db.Exec(` - INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id) - VALUES (?, ?, 'relates-to', ?, 'migration', '{}', '') - `, issueID, relatedID, now) - if err != nil { - return fmt.Errorf("failed to create relates-to edge for %s -> %s: %w", issueID, relatedID, err) + + for _, relatedID := range relatedIDs { + if relatedID == "" { + continue + } + _, err := db.Exec(` + INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id) + VALUES (?, ?, 'relates-to', ?, 'migration', '{}', '') + `, issueID, relatedID, now) + if err != nil { + return fmt.Errorf("failed to create relates-to edge for %s -> %s: %w", issueID, relatedID, err) + } } } - } - if err := rows.Err(); err != nil { - return fmt.Errorf("error iterating relates_to rows: %w", err) + if err := rows.Err(); err != nil { + return fmt.Errorf("error iterating relates_to rows: %w", err) + } } // Migrate duplicate_of fields to duplicates edges - rows, err = db.Query(` - SELECT id, duplicate_of - FROM issues - WHERE duplicate_of != '' AND duplicate_of IS NOT NULL - `) - if err != nil { - return fmt.Errorf("failed to query duplicate_of fields: %w", err) - } - defer rows.Close() - - for rows.Next() { - var issueID, duplicateOf string - if err := rows.Scan(&issueID, &duplicateOf); err != nil { - return fmt.Errorf("failed to scan duplicate_of row: %w", err) - } - - _, err := db.Exec(` - INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id) - VALUES (?, ?, 'duplicates', ?, 'migration', '{}', '') - `, issueID, duplicateOf, now) + if hasDuplicateOf { + rows, err := db.Query(` + SELECT id, duplicate_of + FROM issues + WHERE duplicate_of != '' AND duplicate_of IS NOT NULL + `) if err != nil { - return fmt.Errorf("failed to create duplicates edge for %s: %w", issueID, err) + return fmt.Errorf("failed to query duplicate_of fields: %w", err) + } + defer rows.Close() + + for rows.Next() { + var issueID, duplicateOf string + if err := rows.Scan(&issueID, &duplicateOf); err != nil { + return fmt.Errorf("failed to scan duplicate_of row: %w", err) + } + + _, err := db.Exec(` + INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id) + VALUES (?, ?, 'duplicates', ?, 'migration', '{}', '') + `, issueID, duplicateOf, now) + if err != nil { + return fmt.Errorf("failed to create duplicates edge for %s: %w", issueID, err) + } + } + if err := rows.Err(); err != nil { + return fmt.Errorf("error iterating duplicate_of rows: %w", err) } - } - if err := rows.Err(); err != nil { - return fmt.Errorf("error iterating duplicate_of rows: %w", err) } // Migrate superseded_by fields to supersedes edges - rows, err = db.Query(` - SELECT id, superseded_by - FROM issues - WHERE superseded_by != '' AND superseded_by IS NOT NULL - `) - if err != nil { - return fmt.Errorf("failed to query superseded_by fields: %w", err) - } - defer rows.Close() - - for rows.Next() { - var issueID, supersededBy string - if err := rows.Scan(&issueID, &supersededBy); err != nil { - return fmt.Errorf("failed to scan superseded_by row: %w", err) - } - - _, err := db.Exec(` - INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id) - VALUES (?, ?, 'supersedes', ?, 'migration', '{}', '') - `, issueID, supersededBy, now) + if hasSupersededBy { + rows, err := db.Query(` + SELECT id, superseded_by + FROM issues + WHERE superseded_by != '' AND superseded_by IS NOT NULL + `) if err != nil { - return fmt.Errorf("failed to create supersedes edge for %s: %w", issueID, err) + return fmt.Errorf("failed to query superseded_by fields: %w", err) + } + defer rows.Close() + + for rows.Next() { + var issueID, supersededBy string + if err := rows.Scan(&issueID, &supersededBy); err != nil { + return fmt.Errorf("failed to scan superseded_by row: %w", err) + } + + _, err := db.Exec(` + INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id) + VALUES (?, ?, 'supersedes', ?, 'migration', '{}', '') + `, issueID, supersededBy, now) + if err != nil { + return fmt.Errorf("failed to create supersedes edge for %s: %w", issueID, err) + } + } + if err := rows.Err(); err != nil { + return fmt.Errorf("error iterating superseded_by rows: %w", err) } - } - if err := rows.Err(); err != nil { - return fmt.Errorf("error iterating superseded_by rows: %w", err) } return nil diff --git a/internal/storage/sqlite/migrations/022_drop_edge_columns.go b/internal/storage/sqlite/migrations/022_drop_edge_columns.go index 944bddb5..b2cee13e 100644 --- a/internal/storage/sqlite/migrations/022_drop_edge_columns.go +++ b/internal/storage/sqlite/migrations/022_drop_edge_columns.go @@ -57,6 +57,57 @@ func MigrateDropEdgeColumns(db *sql.DB) error { return nil } + // Preserve newer columns if they already exist (migration may run on partially-migrated DBs). + hasPinned, err := checkCol("pinned") + if err != nil { + return fmt.Errorf("failed to check pinned column: %w", err) + } + hasIsTemplate, err := checkCol("is_template") + if err != nil { + return fmt.Errorf("failed to check is_template column: %w", err) + } + hasAwaitType, err := checkCol("await_type") + if err != nil { + return fmt.Errorf("failed to check await_type column: %w", err) + } + hasAwaitID, err := checkCol("await_id") + if err != nil { + return fmt.Errorf("failed to check await_id column: %w", err) + } + hasTimeoutNs, err := checkCol("timeout_ns") + if err != nil { + return fmt.Errorf("failed to check timeout_ns column: %w", err) + } + hasWaiters, err := checkCol("waiters") + if err != nil { + return fmt.Errorf("failed to check waiters column: %w", err) + } + + pinnedExpr := "0" + if hasPinned { + pinnedExpr = "pinned" + } + isTemplateExpr := "0" + if hasIsTemplate { + isTemplateExpr = "is_template" + } + awaitTypeExpr := "''" + if hasAwaitType { + awaitTypeExpr = "await_type" + } + awaitIDExpr := "''" + if hasAwaitID { + awaitIDExpr = "await_id" + } + timeoutNsExpr := "0" + if hasTimeoutNs { + timeoutNsExpr = "timeout_ns" + } + waitersExpr := "''" + if hasWaiters { + waitersExpr = "waiters" + } + // SQLite 3.35.0+ supports DROP COLUMN, but we use table recreation for compatibility // This is idempotent - we recreate the table without the deprecated columns @@ -117,6 +168,12 @@ func MigrateDropEdgeColumns(db *sql.DB) error { original_type TEXT DEFAULT '', sender TEXT DEFAULT '', ephemeral INTEGER DEFAULT 0, + pinned INTEGER DEFAULT 0, + is_template INTEGER DEFAULT 0, + await_type TEXT, + await_id TEXT, + timeout_ns INTEGER, + waiters TEXT, close_reason TEXT DEFAULT '', CHECK ((status = 'closed') = (closed_at IS NOT NULL)) ) @@ -132,7 +189,8 @@ func MigrateDropEdgeColumns(db *sql.DB) error { notes, status, priority, issue_type, assignee, estimated_minutes, created_at, updated_at, closed_at, external_ref, source_repo, compaction_level, compacted_at, compacted_at_commit, original_size, deleted_at, - deleted_by, delete_reason, original_type, sender, ephemeral, close_reason + deleted_by, delete_reason, original_type, sender, ephemeral, pinned, is_template, + await_type, await_id, timeout_ns, waiters, close_reason ) SELECT id, content_hash, title, description, design, acceptance_criteria, @@ -140,9 +198,11 @@ func MigrateDropEdgeColumns(db *sql.DB) error { created_at, updated_at, closed_at, external_ref, COALESCE(source_repo, ''), compaction_level, compacted_at, compacted_at_commit, original_size, deleted_at, deleted_by, delete_reason, original_type, sender, ephemeral, + %s, %s, + %s, %s, %s, %s, COALESCE(close_reason, '') FROM issues - `) + `, pinnedExpr, isTemplateExpr, awaitTypeExpr, awaitIDExpr, timeoutNsExpr, waitersExpr) if err != nil { return fmt.Errorf("failed to copy issues data: %w", err) } diff --git a/internal/storage/sqlite/migrations/023_pinned_column.go b/internal/storage/sqlite/migrations/023_pinned_column.go index 9854f8e0..73c238dc 100644 --- a/internal/storage/sqlite/migrations/023_pinned_column.go +++ b/internal/storage/sqlite/migrations/023_pinned_column.go @@ -20,6 +20,11 @@ func MigratePinnedColumn(db *sql.DB) error { } if columnExists { + // Column exists (e.g. created by new schema); ensure index exists. + _, err = db.Exec(`CREATE INDEX IF NOT EXISTS idx_issues_pinned ON issues(pinned) WHERE pinned = 1`) + if err != nil { + return fmt.Errorf("failed to create pinned index: %w", err) + } return nil } diff --git a/internal/storage/sqlite/migrations/024_is_template_column.go b/internal/storage/sqlite/migrations/024_is_template_column.go index 07f9462c..fee0316d 100644 --- a/internal/storage/sqlite/migrations/024_is_template_column.go +++ b/internal/storage/sqlite/migrations/024_is_template_column.go @@ -21,6 +21,11 @@ func MigrateIsTemplateColumn(db *sql.DB) error { } if columnExists { + // Column exists (e.g. created by new schema); ensure index exists. + _, err = db.Exec(`CREATE INDEX IF NOT EXISTS idx_issues_is_template ON issues(is_template) WHERE is_template = 1`) + if err != nil { + return fmt.Errorf("failed to create is_template index: %w", err) + } return nil } diff --git a/internal/storage/sqlite/migrations_template_pinned_regression_test.go b/internal/storage/sqlite/migrations_template_pinned_regression_test.go new file mode 100644 index 00000000..818596bb --- /dev/null +++ b/internal/storage/sqlite/migrations_template_pinned_regression_test.go @@ -0,0 +1,59 @@ +package sqlite + +import ( + "context" + "path/filepath" + "testing" + + "github.com/steveyegge/beads/internal/types" +) + +func TestRunMigrations_DoesNotResetPinnedOrTemplate(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + dbPath := filepath.Join(dir, "beads.db") + + s, err := New(ctx, dbPath) + if err != nil { + t.Fatalf("New: %v", err) + } + t.Cleanup(func() { _ = s.Close() }) + + if err := s.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("SetConfig(issue_prefix): %v", err) + } + + issue := &types.Issue{ + Title: "Pinned template", + Status: types.StatusOpen, + Priority: 2, + IssueType: types.TypeTask, + Pinned: true, + IsTemplate: true, + } + if err := s.CreateIssue(ctx, issue, "test-user"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + + _ = s.Close() + + s2, err := New(ctx, dbPath) + if err != nil { + t.Fatalf("New(reopen): %v", err) + } + defer func() { _ = s2.Close() }() + + got, err := s2.GetIssue(ctx, issue.ID) + if err != nil { + t.Fatalf("GetIssue: %v", err) + } + if got == nil { + t.Fatalf("expected issue to exist") + } + if !got.Pinned { + t.Fatalf("expected issue to remain pinned") + } + if !got.IsTemplate { + t.Fatalf("expected issue to remain template") + } +} From 91285f87b9cee6f1f2a4e3d63b436013954195bb Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 12:04:45 -0400 Subject: [PATCH 11/13] test: cover show paths and stabilize debouncer Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- cmd/bd/cli_coverage_show_test.go | 424 +++++++++++++++++++++++++++++++ cmd/bd/daemon_debouncer_test.go | 27 +- 2 files changed, 439 insertions(+), 12 deletions(-) create mode 100644 cmd/bd/cli_coverage_show_test.go diff --git a/cmd/bd/cli_coverage_show_test.go b/cmd/bd/cli_coverage_show_test.go new file mode 100644 index 00000000..688df80f --- /dev/null +++ b/cmd/bd/cli_coverage_show_test.go @@ -0,0 +1,424 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "io" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/steveyegge/beads/internal/storage/sqlite" + "github.com/steveyegge/beads/internal/types" +) + +var cliCoverageMutex sync.Mutex + +func runBDForCoverage(t *testing.T, dir string, args ...string) (stdout string, stderr string) { + t.Helper() + + cliCoverageMutex.Lock() + defer cliCoverageMutex.Unlock() + + // Add --no-daemon to all commands except init. + if len(args) > 0 && args[0] != "init" { + args = append([]string{"--no-daemon"}, args...) + } + + oldStdout := os.Stdout + oldStderr := os.Stderr + oldDir, _ := os.Getwd() + oldArgs := os.Args + + if err := os.Chdir(dir); err != nil { + t.Fatalf("chdir %s: %v", dir, err) + } + + rOut, wOut, _ := os.Pipe() + rErr, wErr, _ := os.Pipe() + os.Stdout = wOut + os.Stderr = wErr + + // Ensure direct mode. + oldNoDaemon, noDaemonWasSet := os.LookupEnv("BEADS_NO_DAEMON") + os.Setenv("BEADS_NO_DAEMON", "1") + defer func() { + if noDaemonWasSet { + _ = os.Setenv("BEADS_NO_DAEMON", oldNoDaemon) + } else { + os.Unsetenv("BEADS_NO_DAEMON") + } + }() + + // Mark tests explicitly. + oldTestMode, testModeWasSet := os.LookupEnv("BEADS_TEST_MODE") + os.Setenv("BEADS_TEST_MODE", "1") + defer func() { + if testModeWasSet { + _ = os.Setenv("BEADS_TEST_MODE", oldTestMode) + } else { + os.Unsetenv("BEADS_TEST_MODE") + } + }() + + // Ensure all commands (including init) operate on the temp workspace DB. + db := filepath.Join(dir, ".beads", "beads.db") + beadsDir := filepath.Join(dir, ".beads") + oldBeadsDir, beadsDirWasSet := os.LookupEnv("BEADS_DIR") + os.Setenv("BEADS_DIR", beadsDir) + defer func() { + if beadsDirWasSet { + _ = os.Setenv("BEADS_DIR", oldBeadsDir) + } else { + os.Unsetenv("BEADS_DIR") + } + }() + + oldDB, dbWasSet := os.LookupEnv("BEADS_DB") + os.Setenv("BEADS_DB", db) + defer func() { + if dbWasSet { + _ = os.Setenv("BEADS_DB", oldDB) + } else { + os.Unsetenv("BEADS_DB") + } + }() + oldBDDB, bdDBWasSet := os.LookupEnv("BD_DB") + os.Setenv("BD_DB", db) + defer func() { + if bdDBWasSet { + _ = os.Setenv("BD_DB", oldBDDB) + } else { + os.Unsetenv("BD_DB") + } + }() + + // Ensure actor is set so label operations record audit fields. + oldActor, actorWasSet := os.LookupEnv("BD_ACTOR") + os.Setenv("BD_ACTOR", "test-user") + defer func() { + if actorWasSet { + _ = os.Setenv("BD_ACTOR", oldActor) + } else { + os.Unsetenv("BD_ACTOR") + } + }() + oldBeadsActor, beadsActorWasSet := os.LookupEnv("BEADS_ACTOR") + os.Setenv("BEADS_ACTOR", "test-user") + defer func() { + if beadsActorWasSet { + _ = os.Setenv("BEADS_ACTOR", oldBeadsActor) + } else { + os.Unsetenv("BEADS_ACTOR") + } + }() + + rootCmd.SetArgs(args) + os.Args = append([]string{"bd"}, args...) + + err := rootCmd.Execute() + + // Close and clean up all global state to prevent contamination between tests. + if store != nil { + store.Close() + store = nil + } + if daemonClient != nil { + daemonClient.Close() + daemonClient = nil + } + + // Reset all global flags and state (keep aligned with integration cli_fast_test). + dbPath = "" + actor = "" + jsonOutput = false + noDaemon = false + noAutoFlush = false + noAutoImport = false + sandboxMode = false + noDb = false + autoFlushEnabled = true + storeActive = false + flushFailureCount = 0 + lastFlushError = nil + if flushManager != nil { + _ = flushManager.Shutdown() + flushManager = nil + } + rootCtx = nil + rootCancel = nil + + // Give SQLite time to release file locks. + time.Sleep(10 * time.Millisecond) + + _ = wOut.Close() + _ = wErr.Close() + os.Stdout = oldStdout + os.Stderr = oldStderr + _ = os.Chdir(oldDir) + os.Args = oldArgs + rootCmd.SetArgs(nil) + + var outBuf, errBuf bytes.Buffer + _, _ = io.Copy(&outBuf, rOut) + _, _ = io.Copy(&errBuf, rErr) + _ = rOut.Close() + _ = rErr.Close() + + stdout = outBuf.String() + stderr = errBuf.String() + + if err != nil { + t.Fatalf("bd %v failed: %v\nStdout: %s\nStderr: %s", args, err, stdout, stderr) + } + + return stdout, stderr +} + +func extractJSONPayload(s string) string { + if i := strings.IndexAny(s, "[{"); i >= 0 { + return s[i:] + } + return s +} + +func parseCreatedIssueID(t *testing.T, out string) string { + t.Helper() + + p := extractJSONPayload(out) + var m map[string]interface{} + if err := json.Unmarshal([]byte(p), &m); err != nil { + t.Fatalf("parse create JSON: %v\n%s", err, out) + } + id, _ := m["id"].(string) + if id == "" { + t.Fatalf("missing id in create output: %s", out) + } + return id +} + +func TestCoverage_ShowUpdateClose(t *testing.T) { + if testing.Short() { + t.Skip("skipping CLI coverage test in short mode") + } + + dir := t.TempDir() + runBDForCoverage(t, dir, "init", "--prefix", "test", "--quiet") + + out, _ := runBDForCoverage(t, dir, "create", "Show coverage issue", "-p", "1", "--json") + id := parseCreatedIssueID(t, out) + + // Exercise update label flows (add -> set -> add/remove). + runBDForCoverage(t, dir, "update", id, "--add-label", "old", "--json") + runBDForCoverage(t, dir, "update", id, "--set-labels", "a,b", "--add-label", "c", "--remove-label", "a", "--json") + runBDForCoverage(t, dir, "update", id, "--remove-label", "old", "--json") + + // Show JSON output and verify labels were applied. + showOut, _ := runBDForCoverage(t, dir, "show", "--allow-stale", id, "--json") + showPayload := extractJSONPayload(showOut) + + var details []map[string]interface{} + if err := json.Unmarshal([]byte(showPayload), &details); err != nil { + // Some commands may emit a single object; fall back to object parse. + var single map[string]interface{} + if err2 := json.Unmarshal([]byte(showPayload), &single); err2 != nil { + t.Fatalf("parse show JSON: %v / %v\n%s", err, err2, showOut) + } + details = []map[string]interface{}{single} + } + if len(details) != 1 { + t.Fatalf("expected 1 issue, got %d", len(details)) + } + labelsAny, ok := details[0]["labels"] + if !ok { + t.Fatalf("expected labels in show output: %s", showOut) + } + labelsBytes, _ := json.Marshal(labelsAny) + labelsStr := string(labelsBytes) + if !strings.Contains(labelsStr, "b") || !strings.Contains(labelsStr, "c") { + t.Fatalf("expected labels b and c, got %s", labelsStr) + } + if strings.Contains(labelsStr, "a") || strings.Contains(labelsStr, "old") { + t.Fatalf("expected labels a and old to be absent, got %s", labelsStr) + } + + // Show text output. + showText, _ := runBDForCoverage(t, dir, "show", "--allow-stale", id) + if !strings.Contains(showText, "Show coverage issue") { + t.Fatalf("expected show output to contain title, got: %s", showText) + } + + // Multi-ID show should print both issues. + out2, _ := runBDForCoverage(t, dir, "create", "Second issue", "-p", "2", "--json") + id2 := parseCreatedIssueID(t, out2) + multi, _ := runBDForCoverage(t, dir, "show", "--allow-stale", id, id2) + if !strings.Contains(multi, "Show coverage issue") || !strings.Contains(multi, "Second issue") { + t.Fatalf("expected multi-show output to include both titles, got: %s", multi) + } + if !strings.Contains(multi, "─") { + t.Fatalf("expected multi-show output to include a separator line, got: %s", multi) + } + + // Close and verify JSON output. + closeOut, _ := runBDForCoverage(t, dir, "close", id, "--reason", "Done", "--json") + closePayload := extractJSONPayload(closeOut) + var closed []map[string]interface{} + if err := json.Unmarshal([]byte(closePayload), &closed); err != nil { + t.Fatalf("parse close JSON: %v\n%s", err, closeOut) + } + if len(closed) != 1 { + t.Fatalf("expected 1 closed issue, got %d", len(closed)) + } + if status, _ := closed[0]["status"].(string); status != string(types.StatusClosed) { + t.Fatalf("expected status closed, got %q", status) + } +} + +func TestCoverage_TemplateAndPinnedProtections(t *testing.T) { + if testing.Short() { + t.Skip("skipping CLI coverage test in short mode") + } + + dir := t.TempDir() + runBDForCoverage(t, dir, "init", "--prefix", "test", "--quiet") + + // Create a pinned issue and verify close requires --force. + out, _ := runBDForCoverage(t, dir, "create", "Pinned issue", "-p", "1", "--json") + pinnedID := parseCreatedIssueID(t, out) + runBDForCoverage(t, dir, "update", pinnedID, "--status", string(types.StatusPinned), "--json") + _, closeErr := runBDForCoverage(t, dir, "close", pinnedID, "--reason", "Done") + if !strings.Contains(closeErr, "cannot close pinned issue") { + t.Fatalf("expected pinned close to be rejected, stderr: %s", closeErr) + } + + forceOut, _ := runBDForCoverage(t, dir, "close", pinnedID, "--force", "--reason", "Done", "--json") + forcePayload := extractJSONPayload(forceOut) + var closed []map[string]interface{} + if err := json.Unmarshal([]byte(forcePayload), &closed); err != nil { + t.Fatalf("parse close JSON: %v\n%s", err, forceOut) + } + if len(closed) != 1 { + t.Fatalf("expected 1 closed issue, got %d", len(closed)) + } + + // Insert a template issue directly and verify update/close protect it. + dbFile := filepath.Join(dir, ".beads", "beads.db") + s, err := sqlite.New(context.Background(), dbFile) + if err != nil { + t.Fatalf("sqlite.New: %v", err) + } + ctx := context.Background() + template := &types.Issue{ + Title: "Template issue", + Status: types.StatusOpen, + Priority: 2, + IssueType: types.TypeTask, + IsTemplate: true, + } + if err := s.CreateIssue(ctx, template, "test-user"); err != nil { + s.Close() + t.Fatalf("CreateIssue: %v", err) + } + created, err := s.GetIssue(ctx, template.ID) + if err != nil { + s.Close() + t.Fatalf("GetIssue(template): %v", err) + } + if created == nil || !created.IsTemplate { + s.Close() + t.Fatalf("expected inserted issue to be IsTemplate=true, got %+v", created) + } + _ = s.Close() + + showOut, _ := runBDForCoverage(t, dir, "show", "--allow-stale", template.ID, "--json") + showPayload := extractJSONPayload(showOut) + var showDetails []map[string]interface{} + if err := json.Unmarshal([]byte(showPayload), &showDetails); err != nil { + t.Fatalf("parse show JSON: %v\n%s", err, showOut) + } + if len(showDetails) != 1 { + t.Fatalf("expected 1 issue from show, got %d", len(showDetails)) + } + // Re-open the DB after running the CLI to confirm is_template persisted. + s2, err := sqlite.New(context.Background(), dbFile) + if err != nil { + t.Fatalf("sqlite.New (reopen): %v", err) + } + postShow, err := s2.GetIssue(context.Background(), template.ID) + _ = s2.Close() + if err != nil { + t.Fatalf("GetIssue(template, post-show): %v", err) + } + if postShow == nil || !postShow.IsTemplate { + t.Fatalf("expected template to remain IsTemplate=true post-show, got %+v", postShow) + } + if v, ok := showDetails[0]["is_template"]; ok { + if b, ok := v.(bool); !ok || !b { + t.Fatalf("expected show JSON is_template=true, got %v", v) + } + } else { + t.Fatalf("expected show JSON to include is_template=true, got: %s", showOut) + } + + _, updErr := runBDForCoverage(t, dir, "update", template.ID, "--title", "New title") + if !strings.Contains(updErr, "cannot update template") { + t.Fatalf("expected template update to be rejected, stderr: %s", updErr) + } + _, closeTemplateErr := runBDForCoverage(t, dir, "close", template.ID, "--reason", "Done") + if !strings.Contains(closeTemplateErr, "cannot close template") { + t.Fatalf("expected template close to be rejected, stderr: %s", closeTemplateErr) + } +} + +func TestCoverage_ShowThread(t *testing.T) { + if testing.Short() { + t.Skip("skipping CLI coverage test in short mode") + } + + dir := t.TempDir() + runBDForCoverage(t, dir, "init", "--prefix", "test", "--quiet") + + dbFile := filepath.Join(dir, ".beads", "beads.db") + s, err := sqlite.New(context.Background(), dbFile) + if err != nil { + t.Fatalf("sqlite.New: %v", err) + } + ctx := context.Background() + + root := &types.Issue{Title: "Root message", IssueType: types.TypeMessage, Status: types.StatusOpen, Sender: "alice", Assignee: "bob"} + reply1 := &types.Issue{Title: "Re: Root", IssueType: types.TypeMessage, Status: types.StatusOpen, Sender: "bob", Assignee: "alice"} + reply2 := &types.Issue{Title: "Re: Re: Root", IssueType: types.TypeMessage, Status: types.StatusOpen, Sender: "alice", Assignee: "bob"} + if err := s.CreateIssue(ctx, root, "test-user"); err != nil { + s.Close() + t.Fatalf("CreateIssue root: %v", err) + } + if err := s.CreateIssue(ctx, reply1, "test-user"); err != nil { + s.Close() + t.Fatalf("CreateIssue reply1: %v", err) + } + if err := s.CreateIssue(ctx, reply2, "test-user"); err != nil { + s.Close() + t.Fatalf("CreateIssue reply2: %v", err) + } + if err := s.AddDependency(ctx, &types.Dependency{IssueID: reply1.ID, DependsOnID: root.ID, Type: types.DepRepliesTo, ThreadID: root.ID}, "test-user"); err != nil { + s.Close() + t.Fatalf("AddDependency reply1->root: %v", err) + } + if err := s.AddDependency(ctx, &types.Dependency{IssueID: reply2.ID, DependsOnID: reply1.ID, Type: types.DepRepliesTo, ThreadID: root.ID}, "test-user"); err != nil { + s.Close() + t.Fatalf("AddDependency reply2->reply1: %v", err) + } + _ = s.Close() + + out, _ := runBDForCoverage(t, dir, "show", "--allow-stale", reply2.ID, "--thread") + if !strings.Contains(out, "Thread") || !strings.Contains(out, "Total: 3 messages") { + t.Fatalf("expected thread output, got: %s", out) + } + if !strings.Contains(out, root.ID) || !strings.Contains(out, reply1.ID) || !strings.Contains(out, reply2.ID) { + t.Fatalf("expected thread output to include message IDs, got: %s", out) + } +} diff --git a/cmd/bd/daemon_debouncer_test.go b/cmd/bd/daemon_debouncer_test.go index 33658277..69e36a7d 100644 --- a/cmd/bd/daemon_debouncer_test.go +++ b/cmd/bd/daemon_debouncer_test.go @@ -157,23 +157,26 @@ func TestDebouncer_MultipleSequentialTriggerCycles(t *testing.T) { }) t.Cleanup(debouncer.Cancel) - debouncer.Trigger() - time.Sleep(40 * time.Millisecond) - if got := atomic.LoadInt32(&count); got != 1 { - t.Errorf("first cycle: got %d, want 1", got) + awaitCount := func(want int32) { + deadline := time.Now().Add(500 * time.Millisecond) + for time.Now().Before(deadline) { + if got := atomic.LoadInt32(&count); got >= want { + return + } + time.Sleep(5 * time.Millisecond) + } + got := atomic.LoadInt32(&count) + t.Fatalf("timeout waiting for count=%d (got %d)", want, got) } debouncer.Trigger() - time.Sleep(40 * time.Millisecond) - if got := atomic.LoadInt32(&count); got != 2 { - t.Errorf("second cycle: got %d, want 2", got) - } + awaitCount(1) debouncer.Trigger() - time.Sleep(40 * time.Millisecond) - if got := atomic.LoadInt32(&count); got != 3 { - t.Errorf("third cycle: got %d, want 3", got) - } + awaitCount(2) + + debouncer.Trigger() + awaitCount(3) } func TestDebouncer_CancelImmediatelyAfterTrigger(t *testing.T) { From f5ef444d15aa96fc8e132af7090c3882b62d10d3 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 12:33:59 -0400 Subject: [PATCH 12/13] cmd/bd: add unit coverage for show/update/close Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- cmd/bd/cli_coverage_show_test.go | 2 + cmd/bd/show.go | 116 ++++++++++---------------- cmd/bd/show_unit_helpers.go | 68 +++++++++++++++ cmd/bd/show_unit_helpers_test.go | 139 +++++++++++++++++++++++++++++++ 4 files changed, 254 insertions(+), 71 deletions(-) create mode 100644 cmd/bd/show_unit_helpers.go create mode 100644 cmd/bd/show_unit_helpers_test.go diff --git a/cmd/bd/cli_coverage_show_test.go b/cmd/bd/cli_coverage_show_test.go index 688df80f..6eb4d661 100644 --- a/cmd/bd/cli_coverage_show_test.go +++ b/cmd/bd/cli_coverage_show_test.go @@ -1,3 +1,5 @@ +//go:build e2e + package main import ( diff --git a/cmd/bd/show.go b/cmd/bd/show.go index cec5ae93..856db09c 100644 --- a/cmd/bd/show.go +++ b/cmd/bd/show.go @@ -663,8 +663,8 @@ var updateCmd = &cobra.Command{ fmt.Fprintf(os.Stderr, "Error getting %s: %v\n", id, err) continue } - if issue != nil && issue.IsTemplate { - fmt.Fprintf(os.Stderr, "Error: cannot update template %s: templates are read-only; use 'bd molecule instantiate' to create a work item\n", id) + if err := validateIssueUpdatable(id, issue); err != nil { + fmt.Fprintf(os.Stderr, "%s\n", err) continue } @@ -683,48 +683,21 @@ var updateCmd = &cobra.Command{ } // Handle label operations - // Set labels (replaces all existing labels) - if setLabels, ok := updates["set_labels"].([]string); ok && len(setLabels) > 0 { - // Get current labels - currentLabels, err := store.GetLabels(ctx, id) - if err != nil { - fmt.Fprintf(os.Stderr, "Error getting labels for %s: %v\n", id, err) + var setLabels, addLabels, removeLabels []string + if v, ok := updates["set_labels"].([]string); ok { + setLabels = v + } + if v, ok := updates["add_labels"].([]string); ok { + addLabels = v + } + if v, ok := updates["remove_labels"].([]string); ok { + removeLabels = v + } + if len(setLabels) > 0 || len(addLabels) > 0 || len(removeLabels) > 0 { + if err := applyLabelUpdates(ctx, store, id, actor, setLabels, addLabels, removeLabels); err != nil { + fmt.Fprintf(os.Stderr, "Error updating labels for %s: %v\n", id, err) continue } - // Remove all current labels - for _, label := range currentLabels { - if err := store.RemoveLabel(ctx, id, label, actor); err != nil { - fmt.Fprintf(os.Stderr, "Error removing label %s from %s: %v\n", label, id, err) - continue - } - } - // Add new labels - for _, label := range setLabels { - if err := store.AddLabel(ctx, id, label, actor); err != nil { - fmt.Fprintf(os.Stderr, "Error setting label %s on %s: %v\n", label, id, err) - continue - } - } - } - - // Add labels - if addLabels, ok := updates["add_labels"].([]string); ok { - for _, label := range addLabels { - if err := store.AddLabel(ctx, id, label, actor); err != nil { - fmt.Fprintf(os.Stderr, "Error adding label %s to %s: %v\n", label, id, err) - continue - } - } - } - - // Remove labels - if removeLabels, ok := updates["remove_labels"].([]string); ok { - for _, label := range removeLabels { - if err := store.RemoveLabel(ctx, id, label, actor); err != nil { - fmt.Fprintf(os.Stderr, "Error removing label %s from %s: %v\n", label, id, err) - continue - } - } } // Run update hook (bd-kwro.8) @@ -993,14 +966,8 @@ var closeCmd = &cobra.Command{ if showErr == nil { var issue types.Issue if json.Unmarshal(showResp.Data, &issue) == nil { - // Check if issue is a template (beads-1ra): templates are read-only - if issue.IsTemplate { - fmt.Fprintf(os.Stderr, "Error: cannot close template %s: templates are read-only\n", id) - continue - } - // Check if issue is pinned (bd-6v2) - if !force && issue.Status == types.StatusPinned { - fmt.Fprintf(os.Stderr, "Error: cannot close pinned issue %s (use --force to override)\n", id) + if err := validateIssueClosable(id, &issue, force); err != nil { + fmt.Fprintf(os.Stderr, "%s\n", err) continue } } @@ -1051,20 +1018,11 @@ var closeCmd = &cobra.Command{ // Get issue for checks issue, _ := store.GetIssue(ctx, id) - // Check if issue is a template (beads-1ra): templates are read-only - if issue != nil && issue.IsTemplate { - fmt.Fprintf(os.Stderr, "Error: cannot close template %s: templates are read-only\n", id) + if err := validateIssueClosable(id, issue, force); err != nil { + fmt.Fprintf(os.Stderr, "%s\n", err) continue } - // Check if issue is pinned (bd-6v2) - if !force { - if issue != nil && issue.Status == types.StatusPinned { - fmt.Fprintf(os.Stderr, "Error: cannot close pinned issue %s (use --force to override)\n", id) - continue - } - } - if err := store.CloseIssue(ctx, id, reason, actor); err != nil { fmt.Fprintf(os.Stderr, "Error closing %s: %v\n", id, err) continue @@ -1291,15 +1249,13 @@ func findRepliesTo(ctx context.Context, issueID string, daemonClient *rpc.Client return "" } // Direct mode - query storage - if sqliteStore, ok := store.(*sqlite.SQLiteStorage); ok { - deps, err := sqliteStore.GetDependenciesWithMetadata(ctx, issueID) - if err != nil { - return "" - } - for _, dep := range deps { - if dep.DependencyType == types.DepRepliesTo { - return dep.ID - } + deps, err := store.GetDependencyRecords(ctx, issueID) + if err != nil { + return "" + } + for _, dep := range deps { + if dep.Type == types.DepRepliesTo { + return dep.DependsOnID } } return "" @@ -1348,7 +1304,25 @@ func findReplies(ctx context.Context, issueID string, daemonClient *rpc.Client, } return replies } - return nil + + allDeps, err := store.GetAllDependencyRecords(ctx) + if err != nil { + return nil + } + + var replies []*types.Issue + for childID, deps := range allDeps { + for _, dep := range deps { + if dep.Type == types.DepRepliesTo && dep.DependsOnID == issueID { + issue, _ := store.GetIssue(ctx, childID) + if issue != nil { + replies = append(replies, issue) + } + } + } + } + + return replies } func init() { diff --git a/cmd/bd/show_unit_helpers.go b/cmd/bd/show_unit_helpers.go new file mode 100644 index 00000000..23c80a1e --- /dev/null +++ b/cmd/bd/show_unit_helpers.go @@ -0,0 +1,68 @@ +package main + +import ( + "context" + "fmt" + + "github.com/steveyegge/beads/internal/storage" + "github.com/steveyegge/beads/internal/types" +) + +func validateIssueUpdatable(id string, issue *types.Issue) error { + if issue == nil { + return nil + } + if issue.IsTemplate { + return fmt.Errorf("Error: cannot update template %s: templates are read-only; use 'bd molecule instantiate' to create a work item", id) + } + return nil +} + +func validateIssueClosable(id string, issue *types.Issue, force bool) error { + if issue == nil { + return nil + } + if issue.IsTemplate { + return fmt.Errorf("Error: cannot close template %s: templates are read-only", id) + } + if !force && issue.Status == types.StatusPinned { + return fmt.Errorf("Error: cannot close pinned issue %s (use --force to override)", id) + } + return nil +} + +func applyLabelUpdates(ctx context.Context, st storage.Storage, issueID, actor string, setLabels, addLabels, removeLabels []string) error { + // Set labels (replaces all existing labels) + if len(setLabels) > 0 { + currentLabels, err := st.GetLabels(ctx, issueID) + if err != nil { + return err + } + for _, label := range currentLabels { + if err := st.RemoveLabel(ctx, issueID, label, actor); err != nil { + return err + } + } + for _, label := range setLabels { + if err := st.AddLabel(ctx, issueID, label, actor); err != nil { + return err + } + } + } + + // Add labels + for _, label := range addLabels { + if err := st.AddLabel(ctx, issueID, label, actor); err != nil { + return err + } + } + + // Remove labels + for _, label := range removeLabels { + if err := st.RemoveLabel(ctx, issueID, label, actor); err != nil { + return err + } + } + + return nil +} diff --git a/cmd/bd/show_unit_helpers_test.go b/cmd/bd/show_unit_helpers_test.go new file mode 100644 index 00000000..e1bef58d --- /dev/null +++ b/cmd/bd/show_unit_helpers_test.go @@ -0,0 +1,139 @@ +package main + +import ( + "context" + "testing" + + "github.com/steveyegge/beads/internal/storage/memory" + "github.com/steveyegge/beads/internal/types" +) + +func TestValidateIssueUpdatable(t *testing.T) { + if err := validateIssueUpdatable("x", nil); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if err := validateIssueUpdatable("x", &types.Issue{IsTemplate: false}); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if err := validateIssueUpdatable("bd-1", &types.Issue{IsTemplate: true}); err == nil { + t.Fatalf("expected error") + } +} + +func TestValidateIssueClosable(t *testing.T) { + if err := validateIssueClosable("x", nil, false); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if err := validateIssueClosable("bd-1", &types.Issue{IsTemplate: true}, false); err == nil { + t.Fatalf("expected template close error") + } + if err := validateIssueClosable("bd-2", &types.Issue{Status: types.StatusPinned}, false); err == nil { + t.Fatalf("expected pinned close error") + } + if err := validateIssueClosable("bd-2", &types.Issue{Status: types.StatusPinned}, true); err != nil { + t.Fatalf("expected pinned close to succeed with force, got %v", err) + } +} + +func TestApplyLabelUpdates_SetAddRemove(t *testing.T) { + ctx := context.Background() + st := memory.New("") + if err := st.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("SetConfig: %v", err) + } + + issue := &types.Issue{Title: "x", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + if err := st.CreateIssue(ctx, issue, "tester"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + + _ = st.AddLabel(ctx, issue.ID, "old1", "tester") + _ = st.AddLabel(ctx, issue.ID, "old2", "tester") + + if err := applyLabelUpdates(ctx, st, issue.ID, "tester", []string{"a", "b"}, []string{"b", "c"}, []string{"a"}); err != nil { + t.Fatalf("applyLabelUpdates: %v", err) + } + labels, _ := st.GetLabels(ctx, issue.ID) + if len(labels) != 2 { + t.Fatalf("expected 2 labels, got %v", labels) + } + // Order is not guaranteed. + foundB := false + foundC := false + for _, l := range labels { + if l == "b" { + foundB = true + } + if l == "c" { + foundC = true + } + if l == "old1" || l == "old2" || l == "a" { + t.Fatalf("unexpected label %q in %v", l, labels) + } + } + if !foundB || !foundC { + t.Fatalf("expected labels b and c, got %v", labels) + } +} + +func TestApplyLabelUpdates_AddRemoveOnly(t *testing.T) { + ctx := context.Background() + st := memory.New("") + issue := &types.Issue{Title: "x", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + if err := st.CreateIssue(ctx, issue, "tester"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + + _ = st.AddLabel(ctx, issue.ID, "a", "tester") + if err := applyLabelUpdates(ctx, st, issue.ID, "tester", nil, []string{"b"}, []string{"a"}); err != nil { + t.Fatalf("applyLabelUpdates: %v", err) + } + labels, _ := st.GetLabels(ctx, issue.ID) + if len(labels) != 1 || labels[0] != "b" { + t.Fatalf("expected [b], got %v", labels) + } +} + +func TestFindRepliesToAndReplies_WorksWithMemoryStorage(t *testing.T) { + ctx := context.Background() + st := memory.New("") + if err := st.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("SetConfig: %v", err) + } + + root := &types.Issue{Title: "root", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeMessage, Sender: "a", Assignee: "b"} + reply1 := &types.Issue{Title: "r1", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeMessage, Sender: "b", Assignee: "a"} + reply2 := &types.Issue{Title: "r2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeMessage, Sender: "a", Assignee: "b"} + if err := st.CreateIssue(ctx, root, "tester"); err != nil { + t.Fatalf("CreateIssue(root): %v", err) + } + if err := st.CreateIssue(ctx, reply1, "tester"); err != nil { + t.Fatalf("CreateIssue(reply1): %v", err) + } + if err := st.CreateIssue(ctx, reply2, "tester"); err != nil { + t.Fatalf("CreateIssue(reply2): %v", err) + } + + if err := st.AddDependency(ctx, &types.Dependency{IssueID: reply1.ID, DependsOnID: root.ID, Type: types.DepRepliesTo}, "tester"); err != nil { + t.Fatalf("AddDependency(reply1->root): %v", err) + } + if err := st.AddDependency(ctx, &types.Dependency{IssueID: reply2.ID, DependsOnID: reply1.ID, Type: types.DepRepliesTo}, "tester"); err != nil { + t.Fatalf("AddDependency(reply2->reply1): %v", err) + } + + if got := findRepliesTo(ctx, root.ID, nil, st); got != "" { + t.Fatalf("expected root replies-to to be empty, got %q", got) + } + if got := findRepliesTo(ctx, reply2.ID, nil, st); got != reply1.ID { + t.Fatalf("expected reply2 parent %q, got %q", reply1.ID, got) + } + + rootReplies := findReplies(ctx, root.ID, nil, st) + if len(rootReplies) != 1 || rootReplies[0].ID != reply1.ID { + t.Fatalf("expected root replies [%s], got %+v", reply1.ID, rootReplies) + } + r1Replies := findReplies(ctx, reply1.ID, nil, st) + if len(r1Replies) != 1 || r1Replies[0].ID != reply2.ID { + t.Fatalf("expected reply1 replies [%s], got %+v", reply2.ID, r1Replies) + } +} From 0b6ec57928112cd82404ed852f25f884cd7d0db7 Mon Sep 17 00:00:00 2001 From: Jordan Hubbard Date: Fri, 26 Dec 2025 17:55:51 -0400 Subject: [PATCH 13/13] test: add cmd/bd helper coverage and stabilize test runner Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- cmd/bd/import_helpers_test.go | 107 ++++++++++++++++ cmd/bd/list_helpers_test.go | 116 ++++++++++++++++++ cmd/bd/reinit_test.go | 6 + cmd/bd/sync_helpers_more_test.go | 71 +++++++++++ cmd/bd/test_wait_helper.go | 4 + cmd/bd/worktree_daemon_test.go | 7 ++ .../memory/memory_more_coverage_test.go | 4 +- scripts/test.sh | 7 +- 8 files changed, 318 insertions(+), 4 deletions(-) create mode 100644 cmd/bd/import_helpers_test.go create mode 100644 cmd/bd/list_helpers_test.go create mode 100644 cmd/bd/sync_helpers_more_test.go diff --git a/cmd/bd/import_helpers_test.go b/cmd/bd/import_helpers_test.go new file mode 100644 index 00000000..5d0c23da --- /dev/null +++ b/cmd/bd/import_helpers_test.go @@ -0,0 +1,107 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/steveyegge/beads/internal/types" +) + +func TestTouchDatabaseFile_UsesJSONLMtime(t *testing.T) { + tmp := t.TempDir() + dbPath := filepath.Join(tmp, "beads.db") + jsonlPath := filepath.Join(tmp, "issues.jsonl") + + if err := os.WriteFile(dbPath, []byte(""), 0o600); err != nil { + t.Fatalf("WriteFile db: %v", err) + } + if err := os.WriteFile(jsonlPath, []byte("{}\n"), 0o600); err != nil { + t.Fatalf("WriteFile jsonl: %v", err) + } + + jsonlTime := time.Now().Add(2 * time.Second) + if err := os.Chtimes(jsonlPath, jsonlTime, jsonlTime); err != nil { + t.Fatalf("Chtimes jsonl: %v", err) + } + + if err := TouchDatabaseFile(dbPath, jsonlPath); err != nil { + t.Fatalf("TouchDatabaseFile: %v", err) + } + + info, err := os.Stat(dbPath) + if err != nil { + t.Fatalf("Stat db: %v", err) + } + if info.ModTime().Before(jsonlTime) { + t.Fatalf("db mtime %v should be >= jsonl mtime %v", info.ModTime(), jsonlTime) + } +} + +func TestImportDetectPrefixFromIssues(t *testing.T) { + if detectPrefixFromIssues(nil) != "" { + t.Fatalf("expected empty") + } + + issues := []*types.Issue{ + {ID: "test-1"}, + {ID: "test-2"}, + {ID: "other-1"}, + } + if got := detectPrefixFromIssues(issues); got != "test" { + t.Fatalf("got %q, want %q", got, "test") + } +} + +func TestCountLines(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "f.txt") + if err := os.WriteFile(p, []byte("a\n\nb\n"), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + if got := countLines(p); got != 3 { + t.Fatalf("countLines=%d, want 3", got) + } +} + +func TestCheckUncommittedChanges_Warns(t *testing.T) { + _, cleanup := setupGitRepo(t) + defer cleanup() + + if err := os.WriteFile("issues.jsonl", []byte("{\"id\":\"test-1\"}\n"), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + _ = execCmd(t, "git", "add", "issues.jsonl") + _ = execCmd(t, "git", "commit", "-m", "add issues") + + // Modify without committing. + if err := os.WriteFile("issues.jsonl", []byte("{\"id\":\"test-1\"}\n{\"id\":\"test-2\"}\n"), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + warn := captureStderr(t, func() { + checkUncommittedChanges("issues.jsonl", &ImportResult{}) + }) + if !strings.Contains(warn, "uncommitted changes") { + t.Fatalf("expected warning, got: %q", warn) + } + + noWarn := captureStderr(t, func() { + checkUncommittedChanges("issues.jsonl", &ImportResult{Created: 1}) + }) + if noWarn != "" { + t.Fatalf("expected no warning, got: %q", noWarn) + } +} + +func execCmd(t *testing.T, name string, args ...string) string { + t.Helper() + out, err := exec.Command(name, args...).CombinedOutput() + if err != nil { + t.Fatalf("%s %v failed: %v\n%s", name, args, err, out) + } + return string(out) +} diff --git a/cmd/bd/list_helpers_test.go b/cmd/bd/list_helpers_test.go new file mode 100644 index 00000000..d2725391 --- /dev/null +++ b/cmd/bd/list_helpers_test.go @@ -0,0 +1,116 @@ +package main + +import ( + "strings" + "testing" + "time" + + "github.com/steveyegge/beads/internal/types" +) + +func TestListParseTimeFlag(t *testing.T) { + cases := []string{ + "2025-12-26", + "2025-12-26T12:34:56", + "2025-12-26 12:34:56", + time.DateOnly, + time.RFC3339, + } + + for _, c := range cases { + // Just make sure we accept the expected formats. + var s string + switch c { + case time.DateOnly: + s = "2025-12-26" + case time.RFC3339: + s = "2025-12-26T12:34:56Z" + default: + s = c + } + got, err := parseTimeFlag(s) + if err != nil { + t.Fatalf("parseTimeFlag(%q) error: %v", s, err) + } + if got.Year() != 2025 { + t.Fatalf("parseTimeFlag(%q) year=%d, want 2025", s, got.Year()) + } + } + + if _, err := parseTimeFlag("not-a-date"); err == nil { + t.Fatalf("expected error") + } +} + +func TestListPinIndicator(t *testing.T) { + if pinIndicator(&types.Issue{Pinned: true}) == "" { + t.Fatalf("expected pin indicator") + } + if pinIndicator(&types.Issue{Pinned: false}) != "" { + t.Fatalf("expected empty pin indicator") + } +} + +func TestListFormatPrettyIssue_BadgesAndDefaults(t *testing.T) { + iss := &types.Issue{ID: "bd-1", Title: "Hello", Status: "wat", Priority: 99, IssueType: "bug"} + out := formatPrettyIssue(iss) + if !strings.Contains(out, "bd-1") || !strings.Contains(out, "Hello") { + t.Fatalf("unexpected output: %q", out) + } + if !strings.Contains(out, "[BUG]") { + t.Fatalf("expected BUG badge: %q", out) + } +} + +func TestListBuildIssueTree_ParentChildByDotID(t *testing.T) { + parent := &types.Issue{ID: "bd-1", Title: "Parent", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + child := &types.Issue{ID: "bd-1.1", Title: "Child", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + orphan := &types.Issue{ID: "bd-2.1", Title: "Orphan", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + + roots, children := buildIssueTree([]*types.Issue{child, parent, orphan}) + if len(children["bd-1"]) != 1 || children["bd-1"][0].ID != "bd-1.1" { + t.Fatalf("expected bd-1 to have bd-1.1 child: %+v", children) + } + if len(roots) != 2 { + t.Fatalf("expected 2 roots (parent + orphan), got %d", len(roots)) + } +} + +func TestListSortIssues_ClosedNilLast(t *testing.T) { + t1 := time.Now().Add(-2 * time.Hour) + t2 := time.Now().Add(-1 * time.Hour) + + closedOld := &types.Issue{ID: "bd-1", ClosedAt: &t1} + closedNew := &types.Issue{ID: "bd-2", ClosedAt: &t2} + open := &types.Issue{ID: "bd-3", ClosedAt: nil} + + issues := []*types.Issue{open, closedOld, closedNew} + sortIssues(issues, "closed", false) + if issues[0].ID != "bd-2" || issues[1].ID != "bd-1" || issues[2].ID != "bd-3" { + t.Fatalf("unexpected order: %s, %s, %s", issues[0].ID, issues[1].ID, issues[2].ID) + } +} + +func TestListDisplayPrettyList(t *testing.T) { + out := captureStdout(t, func() error { + displayPrettyList(nil, false) + return nil + }) + if !strings.Contains(out, "No issues found") { + t.Fatalf("unexpected output: %q", out) + } + + issues := []*types.Issue{ + {ID: "bd-1", Title: "A", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}, + {ID: "bd-2", Title: "B", Status: types.StatusInProgress, Priority: 1, IssueType: types.TypeFeature}, + {ID: "bd-1.1", Title: "C", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}, + } + + out = captureStdout(t, func() error { + displayPrettyList(issues, false) + return nil + }) + if !strings.Contains(out, "bd-1") || !strings.Contains(out, "bd-1.1") || !strings.Contains(out, "Total:") { + t.Fatalf("unexpected output: %q", out) + } +} diff --git a/cmd/bd/reinit_test.go b/cmd/bd/reinit_test.go index f226dd2b..7e370420 100644 --- a/cmd/bd/reinit_test.go +++ b/cmd/bd/reinit_test.go @@ -9,6 +9,7 @@ import ( "runtime" "testing" + "github.com/steveyegge/beads/internal/git" "github.com/steveyegge/beads/internal/storage/sqlite" "github.com/steveyegge/beads/internal/types" ) @@ -90,6 +91,7 @@ func testFreshCloneAutoImport(t *testing.T) { // Test checkGitForIssues detects issues.jsonl t.Chdir(dir) + git.ResetCaches() count, path, gitRef := checkGitForIssues() if count != 1 { @@ -169,6 +171,7 @@ func testDatabaseRemovalScenario(t *testing.T) { // Change to test directory t.Chdir(dir) + git.ResetCaches() // Test checkGitForIssues finds issues.jsonl (canonical name) count, path, gitRef := checkGitForIssues() @@ -247,6 +250,7 @@ func testLegacyFilenameSupport(t *testing.T) { // Change to test directory t.Chdir(dir) + git.ResetCaches() // Test checkGitForIssues finds issues.jsonl count, path, gitRef := checkGitForIssues() @@ -323,6 +327,7 @@ func testPrecedenceTest(t *testing.T) { // Change to test directory t.Chdir(dir) + git.ResetCaches() // Test checkGitForIssues prefers issues.jsonl count, path, _ := checkGitForIssues() @@ -369,6 +374,7 @@ func testInitSafetyCheck(t *testing.T) { // Change to test directory t.Chdir(dir) + git.ResetCaches() // Create empty database (simulating failed import) dbPath := filepath.Join(beadsDir, "test.db") diff --git a/cmd/bd/sync_helpers_more_test.go b/cmd/bd/sync_helpers_more_test.go new file mode 100644 index 00000000..ae8479e4 --- /dev/null +++ b/cmd/bd/sync_helpers_more_test.go @@ -0,0 +1,71 @@ +package main + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/steveyegge/beads/internal/config" +) + +func TestBuildGitCommitArgs_ConfigOptions(t *testing.T) { + if err := config.Initialize(); err != nil { + t.Fatalf("config.Initialize: %v", err) + } + config.Set("git.author", "Test User ") + config.Set("git.no-gpg-sign", true) + + args := buildGitCommitArgs("/repo", "hello", "--", ".beads") + joined := strings.Join(args, " ") + if !strings.Contains(joined, "--author") { + t.Fatalf("expected --author in args: %v", args) + } + if !strings.Contains(joined, "--no-gpg-sign") { + t.Fatalf("expected --no-gpg-sign in args: %v", args) + } + if !strings.Contains(joined, "-m hello") { + t.Fatalf("expected message in args: %v", args) + } +} + +func TestGitCommitBeadsDir_PathspecDoesNotCommitOtherStagedFiles(t *testing.T) { + _, cleanup := setupGitRepo(t) + defer cleanup() + + if err := config.Initialize(); err != nil { + t.Fatalf("config.Initialize: %v", err) + } + + if err := os.MkdirAll(".beads", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + // Stage an unrelated file before running gitCommitBeadsDir. + if err := os.WriteFile("other.txt", []byte("x\n"), 0o600); err != nil { + t.Fatalf("WriteFile other: %v", err) + } + _ = exec.Command("git", "add", "other.txt").Run() + + // Create a beads sync file to commit. + issuesPath := filepath.Join(".beads", "issues.jsonl") + if err := os.WriteFile(issuesPath, []byte("{\"id\":\"test-1\"}\n"), 0o600); err != nil { + t.Fatalf("WriteFile issues: %v", err) + } + + ctx := context.Background() + if err := gitCommitBeadsDir(ctx, "beads commit"); err != nil { + t.Fatalf("gitCommitBeadsDir: %v", err) + } + + // other.txt should still be staged after the beads-only commit. + out, err := exec.Command("git", "diff", "--cached", "--name-only").CombinedOutput() + if err != nil { + t.Fatalf("git diff --cached: %v\n%s", err, out) + } + if strings.TrimSpace(string(out)) != "other.txt" { + t.Fatalf("expected other.txt still staged, got: %q", out) + } +} diff --git a/cmd/bd/test_wait_helper.go b/cmd/bd/test_wait_helper.go index 78d965c5..d67a9083 100644 --- a/cmd/bd/test_wait_helper.go +++ b/cmd/bd/test_wait_helper.go @@ -5,6 +5,8 @@ import ( "os/exec" "testing" "time" + + "github.com/steveyegge/beads/internal/git" ) // waitFor repeatedly evaluates pred until it returns true or timeout expires. @@ -42,6 +44,7 @@ func setupGitRepo(t *testing.T) (repoPath string, cleanup func()) { _ = os.Chdir(originalWd) t.Fatalf("failed to init git repo: %v", err) } + git.ResetCaches() // Configure git _ = exec.Command("git", "config", "user.email", "test@test.com").Run() @@ -85,6 +88,7 @@ func setupGitRepoWithBranch(t *testing.T, branch string) (repoPath string, clean _ = os.Chdir(originalWd) t.Fatalf("failed to init git repo: %v", err) } + git.ResetCaches() // Configure git _ = exec.Command("git", "config", "user.email", "test@test.com").Run() diff --git a/cmd/bd/worktree_daemon_test.go b/cmd/bd/worktree_daemon_test.go index 6e793499..28228f66 100644 --- a/cmd/bd/worktree_daemon_test.go +++ b/cmd/bd/worktree_daemon_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/steveyegge/beads/internal/config" + "github.com/steveyegge/beads/internal/git" // Import SQLite driver for test database creation _ "github.com/ncruces/go-sqlite3/driver" @@ -78,6 +79,7 @@ func TestShouldDisableDaemonForWorktree(t *testing.T) { if err := os.Chdir(worktreeDir); err != nil { t.Fatalf("Failed to change to worktree dir: %v", err) } + git.ResetCaches() // No sync-branch configured os.Unsetenv("BEADS_SYNC_BRANCH") @@ -113,6 +115,7 @@ func TestShouldDisableDaemonForWorktree(t *testing.T) { if err := os.Chdir(worktreeDir); err != nil { t.Fatalf("Failed to change to worktree dir: %v", err) } + git.ResetCaches() // Reinitialize config to pick up the new directory's config.yaml if err := config.Initialize(); err != nil { @@ -144,6 +147,7 @@ func TestShouldDisableDaemonForWorktree(t *testing.T) { if err := os.Chdir(worktreeDir); err != nil { t.Fatalf("Failed to change to worktree dir: %v", err) } + git.ResetCaches() // Reinitialize config to pick up the new directory's config.yaml if err := config.Initialize(); err != nil { @@ -194,6 +198,7 @@ func TestShouldAutoStartDaemonWorktreeIntegration(t *testing.T) { if err := os.Chdir(worktreeDir); err != nil { t.Fatalf("Failed to change to worktree dir: %v", err) } + git.ResetCaches() // Clear all daemon-related env vars os.Unsetenv("BEADS_NO_DAEMON") @@ -227,6 +232,7 @@ func TestShouldAutoStartDaemonWorktreeIntegration(t *testing.T) { if err := os.Chdir(worktreeDir); err != nil { t.Fatalf("Failed to change to worktree dir: %v", err) } + git.ResetCaches() // Reinitialize config to pick up the new directory's config.yaml if err := config.Initialize(); err != nil { @@ -260,6 +266,7 @@ func TestShouldAutoStartDaemonWorktreeIntegration(t *testing.T) { if err := os.Chdir(worktreeDir); err != nil { t.Fatalf("Failed to change to worktree dir: %v", err) } + git.ResetCaches() // Reinitialize config to pick up the new directory's config.yaml if err := config.Initialize(); err != nil { diff --git a/internal/storage/memory/memory_more_coverage_test.go b/internal/storage/memory/memory_more_coverage_test.go index 401be669..82651647 100644 --- a/internal/storage/memory/memory_more_coverage_test.go +++ b/internal/storage/memory/memory_more_coverage_test.go @@ -793,7 +793,7 @@ func TestMemoryStorage_UpdateIssue_SearchIssues_ReadyWork_BlockedIssues(t *testi } // Blocked issues: child is blocked by an open blocker. - blocked, err := store.GetBlockedIssues(ctx) + blocked, err := store.GetBlockedIssues(ctx, types.WorkFilter{}) if err != nil { t.Fatalf("GetBlockedIssues: %v", err) } @@ -810,7 +810,7 @@ func TestMemoryStorage_UpdateIssue_SearchIssues_ReadyWork_BlockedIssues(t *testi store.mu.Lock() store.dependencies[missing.ID] = append(store.dependencies[missing.ID], &types.Dependency{IssueID: missing.ID, DependsOnID: "bd-does-not-exist", Type: types.DepBlocks}) store.mu.Unlock() - blocked, err = store.GetBlockedIssues(ctx) + blocked, err = store.GetBlockedIssues(ctx, types.WorkFilter{}) if err != nil { t.Fatalf("GetBlockedIssues: %v", err) } diff --git a/scripts/test.sh b/scripts/test.sh index ac5729bf..d5f2c3ba 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -26,7 +26,7 @@ VERBOSE="${TEST_VERBOSE:-}" RUN_PATTERN="${TEST_RUN:-}" COVERAGE="${TEST_COVER:-}" COVERPROFILE="${TEST_COVERPROFILE:-/tmp/beads.coverage.out}" -COVERPKG="${TEST_COVERPKG:-./...}" +COVERPKG="${TEST_COVERPKG:-}" # Parse arguments PACKAGES=() @@ -81,7 +81,10 @@ if [[ -n "$RUN_PATTERN" ]]; then fi if [[ -n "$COVERAGE" ]]; then - CMD+=(-covermode=atomic -coverpkg "$COVERPKG" -coverprofile "$COVERPROFILE") + CMD+=(-covermode=atomic -coverprofile "$COVERPROFILE") + if [[ -n "$COVERPKG" ]]; then + CMD+=(-coverpkg "$COVERPKG") + fi fi CMD+=("${PACKAGES[@]}")