fix(doctor): UX improvements for diagnostics and daemon (#687)

* fix(doctor): UX improvements for diagnostics and daemon

- Add Repo Fingerprint check to detect when database belongs to a
  different repository (copied .beads dir or git remote URL change)
- Add interactive fix for repo fingerprint with options: update repo ID,
  reinitialize database, or skip
- Add visible warning when daemon takes >5s to start, recommending
  'bd doctor' for diagnosis
- Detect install method (Homebrew vs script) and show only relevant
  upgrade command
- Improve WARNINGS section:
  - Add icons (⚠ or ✖) next to each item
  - Color numbers by severity (yellow for warnings, red for errors)
  - Render entire error lines in red
  - Sort by severity (errors first)
  - Fix alignment with checkmarks above
- Use heavier fail icon (✖) for better visibility
- Add integration and validation tests for doctor fixes

* fix(lint): address errcheck and gosec warnings

- mol_bond.go: explicitly ignore ephStore.Close() error
- beads.go: add nosec for .gitignore file permissions (0644 is standard)
This commit is contained in:
Ryan
2025-12-22 01:25:23 -08:00
committed by GitHub
parent 9d30e80fdf
commit a11b20960a
12 changed files with 563 additions and 10 deletions

View File

@@ -12,6 +12,7 @@ import (
"github.com/steveyegge/beads/internal/config"
"github.com/steveyegge/beads/internal/debug"
"github.com/steveyegge/beads/internal/rpc"
"github.com/steveyegge/beads/internal/ui"
)
// Daemon start failure tracking for exponential backoff
@@ -135,6 +136,8 @@ func restartDaemonForVersionMismatch() bool {
}
debug.Logf("new daemon failed to become ready")
fmt.Fprintf(os.Stderr, "%s Daemon restart timed out (>5s). Running in direct mode.\n", ui.RenderWarn("Warning:"))
fmt.Fprintf(os.Stderr, " %s Run 'bd doctor' to diagnose daemon issues\n", ui.RenderMuted("Hint:"))
return false
}
@@ -286,6 +289,9 @@ func startDaemonProcess(socketPath string) bool {
recordDaemonStartFailure()
debugLog("daemon socket not ready after 5 seconds")
// Emit visible warning so user understands why command was slow
fmt.Fprintf(os.Stderr, "%s Daemon took too long to start (>5s). Running in direct mode.\n", ui.RenderWarn("Warning:"))
fmt.Fprintf(os.Stderr, " %s Run 'bd doctor' to diagnose daemon issues\n", ui.RenderMuted("Hint:"))
return false
}

View File

@@ -7,6 +7,7 @@ import (
"fmt"
"os"
"path/filepath"
"sort"
"strings"
"time"
@@ -371,6 +372,8 @@ func applyFixList(path string, fixes []doctorCheck) {
err = fix.DatabaseVersion(path)
case "Schema Compatibility":
err = fix.SchemaCompatibility(path)
case "Repo Fingerprint":
err = fix.RepoFingerprint(path)
case "Git Merge Driver":
err = fix.MergeDriver(path)
case "Sync Branch Config":
@@ -586,7 +589,14 @@ func runDiagnostics(path string) doctorResult {
result.OverallOK = false
}
// Check 2b: Database integrity (bd-2au)
// Check 2b: Repo fingerprint (detects wrong database or URL change)
fingerprintCheck := convertWithCategory(doctor.CheckRepoFingerprint(path), doctor.CategoryCore)
result.Checks = append(result.Checks, fingerprintCheck)
if fingerprintCheck.Status == statusError {
result.OverallOK = false
}
// Check 2c: Database integrity (bd-2au)
integrityCheck := convertWithCategory(doctor.CheckDatabaseIntegrity(path), doctor.CategoryCore)
result.Checks = append(result.Checks, integrityCheck)
if integrityCheck.Status == statusError {
@@ -883,10 +893,30 @@ func printDiagnostics(result doctorResult) {
if len(warnings) > 0 {
fmt.Println()
fmt.Println(ui.RenderWarn(ui.IconWarn + " WARNINGS"))
for _, check := range warnings {
fmt.Printf(" %s: %s\n", check.Name, check.Message)
// Sort by severity: errors first, then warnings
sort.Slice(warnings, func(i, j int) bool {
// Errors (statusError) come before warnings (statusWarning)
if warnings[i].Status == statusError && warnings[j].Status != statusError {
return true
}
if warnings[i].Status != statusError && warnings[j].Status == statusError {
return false
}
return false // maintain original order within same severity
})
for i, check := range warnings {
// Show numbered items with icon and color based on status
// Errors get entire line in red, warnings just the number in yellow
line := fmt.Sprintf("%s: %s", check.Name, check.Message)
if check.Status == statusError {
fmt.Printf(" %s %s %s\n", ui.RenderFailIcon(), ui.RenderFail(fmt.Sprintf("%d.", i+1)), ui.RenderFail(line))
} else {
fmt.Printf(" %s %s %s\n", ui.RenderWarnIcon(), ui.RenderWarn(fmt.Sprintf("%d.", i+1)), line)
}
if check.Fix != "" {
fmt.Printf(" %s%s\n", ui.MutedStyle.Render(ui.TreeLast), check.Fix)
fmt.Printf(" %s%s\n", ui.MutedStyle.Render(ui.TreeLast), check.Fix)
}
}
} else {

View File

@@ -0,0 +1,225 @@
//go:build integration
package fix
import (
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
)
// setupTestGitRepoIntegration creates a temporary git repository with a .beads directory
func setupTestGitRepoIntegration(t *testing.T) string {
t.Helper()
dir := t.TempDir()
beadsDir := filepath.Join(dir, ".beads")
if err := os.MkdirAll(beadsDir, 0755); err != nil {
t.Fatalf("failed to create .beads directory: %v", err)
}
// Initialize git repo
cmd := exec.Command("git", "init")
cmd.Dir = dir
if err := cmd.Run(); err != nil {
t.Fatalf("failed to init git repo: %v", err)
}
// Configure git user for commits
cmd = exec.Command("git", "config", "user.email", "test@test.com")
cmd.Dir = dir
_ = cmd.Run()
cmd = exec.Command("git", "config", "user.name", "Test User")
cmd.Dir = dir
_ = cmd.Run()
return dir
}
// runGitIntegration runs a git command and returns output
func runGitIntegration(t *testing.T, dir string, args ...string) string {
t.Helper()
cmd := exec.Command("git", args...)
cmd.Dir = dir
output, err := cmd.CombinedOutput()
if err != nil {
t.Logf("git %v: %s", args, output)
}
return string(output)
}
// TestSyncBranchHealth_LocalAndRemoteDiverged tests fix when branches diverged
func TestSyncBranchHealth_LocalAndRemoteDiverged(t *testing.T) {
// Setup bare remote repo
remoteDir := t.TempDir()
runGitIntegration(t, remoteDir, "init", "--bare")
// Setup local repo
dir := setupTestGitRepoIntegration(t)
runGitIntegration(t, dir, "remote", "add", "origin", remoteDir)
// Create main branch with initial commit
testFile := filepath.Join(dir, "test.txt")
if err := os.WriteFile(testFile, []byte("main content"), 0600); err != nil {
t.Fatalf("failed to create test file: %v", err)
}
runGitIntegration(t, dir, "add", "test.txt")
runGitIntegration(t, dir, "commit", "-m", "initial commit")
runGitIntegration(t, dir, "branch", "-M", "main")
runGitIntegration(t, dir, "push", "-u", "origin", "main")
// Create sync branch
runGitIntegration(t, dir, "checkout", "-b", "beads-sync")
syncFile := filepath.Join(dir, "sync.txt")
if err := os.WriteFile(syncFile, []byte("sync content"), 0600); err != nil {
t.Fatalf("failed to create sync file: %v", err)
}
runGitIntegration(t, dir, "add", "sync.txt")
runGitIntegration(t, dir, "commit", "-m", "sync commit")
runGitIntegration(t, dir, "push", "-u", "origin", "beads-sync")
// Simulate divergence: update main
runGitIntegration(t, dir, "checkout", "main")
if err := os.WriteFile(testFile, []byte("updated main content"), 0600); err != nil {
t.Fatalf("failed to update test file: %v", err)
}
runGitIntegration(t, dir, "add", "test.txt")
runGitIntegration(t, dir, "commit", "-m", "update main")
runGitIntegration(t, dir, "push", "origin", "main")
// Now beads-sync is behind main - fix it
err := SyncBranchHealth(dir, "beads-sync")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// Verify beads-sync was reset to main
runGitIntegration(t, dir, "checkout", "beads-sync")
runGitIntegration(t, dir, "pull", "origin", "beads-sync")
// Check that beads-sync now has main's content
content, err := os.ReadFile(testFile)
if err != nil {
t.Fatalf("failed to read test file: %v", err)
}
if string(content) != "updated main content" {
t.Errorf("expected beads-sync to have main's content, got: %s", content)
}
// Check that sync.txt no longer exists (branch was reset)
if _, err := os.Stat(syncFile); !os.IsNotExist(err) {
t.Error("sync.txt should not exist after reset to main")
}
}
// TestSyncBranchHealth_UncommittedChanges tests fix with uncommitted changes
func TestSyncBranchHealth_UncommittedChanges(t *testing.T) {
// Setup bare remote repo
remoteDir := t.TempDir()
runGitIntegration(t, remoteDir, "init", "--bare")
// Setup local repo
dir := setupTestGitRepoIntegration(t)
runGitIntegration(t, dir, "remote", "add", "origin", remoteDir)
// Create main branch with initial commit
testFile := filepath.Join(dir, "test.txt")
if err := os.WriteFile(testFile, []byte("main content"), 0600); err != nil {
t.Fatalf("failed to create test file: %v", err)
}
runGitIntegration(t, dir, "add", "test.txt")
runGitIntegration(t, dir, "commit", "-m", "initial commit")
runGitIntegration(t, dir, "branch", "-M", "main")
runGitIntegration(t, dir, "push", "-u", "origin", "main")
// Create sync branch and push it
runGitIntegration(t, dir, "checkout", "-b", "beads-sync")
runGitIntegration(t, dir, "push", "-u", "origin", "beads-sync")
// Add uncommitted changes to sync branch
dirtyFile := filepath.Join(dir, "dirty.txt")
if err := os.WriteFile(dirtyFile, []byte("uncommitted"), 0600); err != nil {
t.Fatalf("failed to create dirty file: %v", err)
}
// Checkout main to allow sync branch reset
runGitIntegration(t, dir, "checkout", "main")
// Fix should succeed - it resets the branch, not the working tree
err := SyncBranchHealth(dir, "beads-sync")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// Verify sync branch was reset
output := runGitIntegration(t, dir, "log", "--oneline", "beads-sync")
if !strings.Contains(output, "initial commit") {
t.Errorf("beads-sync should be reset to main, got log: %s", output)
}
}
// TestSyncBranchHealth_RemoteUnreachable tests fix when remote is unreachable
func TestSyncBranchHealth_RemoteUnreachable(t *testing.T) {
dir := setupTestGitRepoIntegration(t)
// Add unreachable remote
runGitIntegration(t, dir, "remote", "add", "origin", "https://nonexistent.example.com/repo.git")
// Create main branch with initial commit
testFile := filepath.Join(dir, "test.txt")
if err := os.WriteFile(testFile, []byte("main content"), 0600); err != nil {
t.Fatalf("failed to create test file: %v", err)
}
runGitIntegration(t, dir, "add", "test.txt")
runGitIntegration(t, dir, "commit", "-m", "initial commit")
runGitIntegration(t, dir, "branch", "-M", "main")
// Create local sync branch
runGitIntegration(t, dir, "checkout", "-b", "beads-sync")
runGitIntegration(t, dir, "checkout", "main")
// Fix should fail when trying to fetch
err := SyncBranchHealth(dir, "beads-sync")
if err == nil {
t.Error("expected error when remote is unreachable")
}
if err != nil && !strings.Contains(err.Error(), "failed to fetch") {
t.Errorf("expected fetch error, got: %v", err)
}
}
// TestSyncBranchHealth_CurrentlyOnSyncBranch tests error when on sync branch
func TestSyncBranchHealth_CurrentlyOnSyncBranch(t *testing.T) {
// Setup bare remote repo
remoteDir := t.TempDir()
runGitIntegration(t, remoteDir, "init", "--bare")
// Setup local repo
dir := setupTestGitRepoIntegration(t)
runGitIntegration(t, dir, "remote", "add", "origin", remoteDir)
// Create main branch with initial commit
testFile := filepath.Join(dir, "test.txt")
if err := os.WriteFile(testFile, []byte("main content"), 0600); err != nil {
t.Fatalf("failed to create test file: %v", err)
}
runGitIntegration(t, dir, "add", "test.txt")
runGitIntegration(t, dir, "commit", "-m", "initial commit")
runGitIntegration(t, dir, "branch", "-M", "main")
runGitIntegration(t, dir, "push", "-u", "origin", "main")
// Create and checkout sync branch
runGitIntegration(t, dir, "checkout", "-b", "beads-sync")
runGitIntegration(t, dir, "push", "-u", "origin", "beads-sync")
// Try to fix while on sync branch
err := SyncBranchHealth(dir, "beads-sync")
if err == nil {
t.Error("expected error when currently on sync branch")
}
if err != nil && !strings.Contains(err.Error(), "currently on beads-sync branch") {
t.Errorf("expected 'currently on branch' error, got: %v", err)
}
}

View File

@@ -0,0 +1,126 @@
package fix
import (
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
)
// readLineUnbuffered reads a line from stdin without buffering.
// This ensures subprocess stdin isn't consumed by our buffered reader.
func readLineUnbuffered() (string, error) {
var result []byte
buf := make([]byte, 1)
for {
n, err := os.Stdin.Read(buf)
if err != nil {
return string(result), err
}
if n == 1 {
c := buf[0] // #nosec G602 -- n==1 guarantees buf has 1 byte
if c == '\n' {
return string(result), nil
}
result = append(result, c)
}
}
}
// 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)
func RepoFingerprint(path string) error {
// Validate workspace
if err := validateBeadsWorkspace(path); err != nil {
return err
}
// Get bd binary path
bdBinary, err := getBdBinary()
if err != nil {
return err
}
// Prompt user for action
fmt.Println("\n Repo fingerprint mismatch detected. Choose an action:")
fmt.Println()
fmt.Println(" [1] Update repo ID (if git remote URL changed or bd was upgraded)")
fmt.Println(" [2] Reinitialize database (if wrong .beads was copied here)")
fmt.Println(" [s] Skip (do nothing)")
fmt.Println()
fmt.Print(" Choice [1/2/s]: ")
// Read single character without buffering to avoid consuming input meant for subprocesses
response, err := readLineUnbuffered()
if err != nil {
return fmt.Errorf("failed to read input: %w", err)
}
response = strings.TrimSpace(strings.ToLower(response))
switch response {
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.Dir = path
cmd.Stdin = os.Stdin // Allow user to respond to migrate's confirmation prompt
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
if err := cmd.Run(); err != nil {
return fmt.Errorf("failed to update repo ID: %w", err)
}
return nil
case "2":
// Confirm before destructive action
fmt.Print(" ⚠️ This will DELETE .beads/beads.db. Continue? [y/N]: ")
confirm, err := readLineUnbuffered()
if err != nil {
return fmt.Errorf("failed to read confirmation: %w", err)
}
confirm = strings.TrimSpace(strings.ToLower(confirm))
if confirm != "y" && confirm != "yes" {
fmt.Println(" → Skipped (canceled)")
return nil
}
// Remove database and reinitialize
beadsDir := filepath.Join(path, ".beads")
dbPath := filepath.Join(beadsDir, "beads.db")
fmt.Printf(" → Removing %s...\n", dbPath)
if err := os.Remove(dbPath); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to remove database: %w", err)
}
// Also remove WAL and SHM files if they exist
_ = os.Remove(dbPath + "-wal")
_ = os.Remove(dbPath + "-shm")
fmt.Println(" → Running 'bd init'...")
cmd := exec.Command(bdBinary, "init", "--quiet") // #nosec G204 -- bdBinary from validated executable path
cmd.Dir = path
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
if err := cmd.Run(); err != nil {
return fmt.Errorf("failed to initialize database: %w", err)
}
return nil
case "s", "":
fmt.Println(" → Skipped")
return nil
default:
fmt.Printf(" → Unrecognized input '%s', skipping\n", response)
return nil
}
}

View File

@@ -0,0 +1,37 @@
package fix
import (
"testing"
)
// TestFixFunctions_RequireBeadsDir verifies all fix functions properly validate
// that a .beads directory exists before attempting fixes.
// This replaces 10+ individual "missing .beads directory" subtests.
func TestFixFunctions_RequireBeadsDir(t *testing.T) {
funcs := []struct {
name string
fn func(string) error
}{
{"GitHooks", GitHooks},
{"MergeDriver", MergeDriver},
{"Daemon", Daemon},
{"DBJSONLSync", DBJSONLSync},
{"DatabaseVersion", DatabaseVersion},
{"SchemaCompatibility", SchemaCompatibility},
{"SyncBranchConfig", SyncBranchConfig},
{"SyncBranchHealth", func(dir string) error { return SyncBranchHealth(dir, "beads-sync") }},
{"UntrackedJSONL", UntrackedJSONL},
{"MigrateTombstones", MigrateTombstones},
}
for _, tc := range funcs {
t.Run(tc.name, func(t *testing.T) {
// Use a temp directory without .beads
dir := t.TempDir()
err := tc.fn(dir)
if err == nil {
t.Errorf("%s should return error for missing .beads directory", tc.name)
}
})
}
}

View File

@@ -396,6 +396,103 @@ func CheckDeletionsManifest(path string) DoctorCheck {
}
}
// CheckRepoFingerprint validates that the database belongs to this repository.
// This detects when a .beads directory was copied from another repo or when
// the git remote URL changed. A mismatch can cause data loss during sync.
func CheckRepoFingerprint(path string) DoctorCheck {
beadsDir := filepath.Join(path, ".beads")
// Get database path
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)
}
// Skip if database doesn't exist
if _, err := os.Stat(dbPath); os.IsNotExist(err) {
return DoctorCheck{
Name: "Repo Fingerprint",
Status: StatusOK,
Message: "N/A (no database)",
}
}
// Open database
db, err := sql.Open("sqlite3", "file:"+dbPath+"?mode=ro")
if err != nil {
return DoctorCheck{
Name: "Repo Fingerprint",
Status: StatusWarning,
Message: "Unable to open database",
Detail: err.Error(),
}
}
defer db.Close()
// Get stored repo ID
var storedRepoID string
err = db.QueryRow("SELECT value FROM metadata WHERE key = 'repo_id'").Scan(&storedRepoID)
if err != nil {
if err == sql.ErrNoRows || strings.Contains(err.Error(), "no such table") {
// Legacy database without repo_id
return DoctorCheck{
Name: "Repo Fingerprint",
Status: StatusWarning,
Message: "Legacy database (no fingerprint)",
Detail: "Database was created before version 0.17.5",
Fix: "Run 'bd migrate --update-repo-id' to add fingerprint",
}
}
return DoctorCheck{
Name: "Repo Fingerprint",
Status: StatusWarning,
Message: "Unable to read repo fingerprint",
Detail: err.Error(),
}
}
// If repo_id is empty, treat as legacy
if storedRepoID == "" {
return DoctorCheck{
Name: "Repo Fingerprint",
Status: StatusWarning,
Message: "Legacy database (empty fingerprint)",
Detail: "Database was created before version 0.17.5",
Fix: "Run 'bd migrate --update-repo-id' to add fingerprint",
}
}
// Compute current repo ID
currentRepoID, err := beads.ComputeRepoID()
if err != nil {
return DoctorCheck{
Name: "Repo Fingerprint",
Status: StatusWarning,
Message: "Unable to compute current repo ID",
Detail: err.Error(),
}
}
// Compare
if storedRepoID != currentRepoID {
return DoctorCheck{
Name: "Repo Fingerprint",
Status: StatusError,
Message: "Database belongs to different repository",
Detail: fmt.Sprintf("stored: %s, current: %s", storedRepoID[:8], currentRepoID[:8]),
Fix: "Run 'bd migrate --update-repo-id' if URL changed, or 'rm -rf .beads && bd init' if wrong database",
}
}
return DoctorCheck{
Name: "Repo Fingerprint",
Status: StatusOK,
Message: fmt.Sprintf("Verified (%s)", currentRepoID[:8]),
}
}
// Fix functions
// FixMigrateTombstones converts legacy deletions.jsonl entries to inline tombstones

View File

@@ -34,14 +34,12 @@ func CheckCLIVersion(cliVersion string) DoctorCheck {
// Compare versions using simple semver-aware comparison
if CompareVersions(latestVersion, cliVersion) > 0 {
upgradeCmds := ` • Homebrew: brew upgrade bd
• Script: curl -fsSL https://raw.githubusercontent.com/steveyegge/beads/main/scripts/install.sh | bash`
upgradeCmd := getUpgradeCommand()
return DoctorCheck{
Name: "CLI Version",
Status: StatusWarning,
Message: fmt.Sprintf("%s (latest: %s)", cliVersion, latestVersion),
Fix: fmt.Sprintf("Upgrade to latest version:\n%s", upgradeCmds),
Fix: fmt.Sprintf("Upgrade: %s", upgradeCmd),
}
}
@@ -52,6 +50,36 @@ func CheckCLIVersion(cliVersion string) DoctorCheck {
}
}
// getUpgradeCommand returns the appropriate upgrade command based on how bd was installed.
// Detects Homebrew on macOS/Linux, and falls back to the install script on all platforms.
func getUpgradeCommand() string {
// Get the executable path
execPath, err := os.Executable()
if err != nil {
return "curl -fsSL https://raw.githubusercontent.com/steveyegge/beads/main/scripts/install.sh | bash"
}
// Resolve symlinks to get the real path
realPath, err := filepath.EvalSymlinks(execPath)
if err != nil {
realPath = execPath
}
// Normalize to lowercase for comparison
lowerPath := strings.ToLower(realPath)
// Check for Homebrew installation (macOS/Linux)
// Homebrew paths: /opt/homebrew/Cellar/bd, /usr/local/Cellar/bd, /home/linuxbrew/.linuxbrew/Cellar/bd
if strings.Contains(lowerPath, "/cellar/bd/") ||
strings.Contains(lowerPath, "/homebrew/") ||
strings.Contains(lowerPath, "/linuxbrew/") {
return "brew upgrade bd"
}
// Default to install script (works on all platforms including Windows via WSL/Git Bash)
return "curl -fsSL https://raw.githubusercontent.com/steveyegge/beads/main/scripts/install.sh | bash"
}
// localVersionFile is the gitignored file that stores the last bd version used locally.
// Must match the constant in version_tracking.go.
const localVersionFile = ".local_version"

View File

@@ -89,7 +89,7 @@ func runMolBond(cmd *cobra.Command, args []string) {
fmt.Fprintf(os.Stderr, "Error: failed to open wisp storage: %v\n", err)
os.Exit(1)
}
defer wispStore.Close()
defer func() { _ = wispStore.Close() }()
targetStore = wispStore
// Ensure wisp directory is gitignored

View File

@@ -186,6 +186,8 @@ func (sm *SnapshotManager) Validate() error {
}
// Cleanup removes all snapshot files and metadata
//
//nolint:unparam // error return kept for API consistency with other methods
func (sm *SnapshotManager) Cleanup() error {
basePath, leftPath := sm.getSnapshotPaths()
baseMetaPath, leftMetaPath := sm.getSnapshotMetadataPaths()