diff --git a/cmd/bd/daemon.go b/cmd/bd/daemon.go index 09f72d05..357a5ff3 100644 --- a/cmd/bd/daemon.go +++ b/cmd/bd/daemon.go @@ -829,9 +829,9 @@ func setupDaemonLogger(logPath string) (*lumberjack.Logger, daemonLogger) { return logF, logger } -func setupDaemonLock(pidFile string, global bool, log daemonLogger) (io.Closer, error) { +func setupDaemonLock(pidFile string, dbPath string, log daemonLogger) (io.Closer, error) { beadsDir := filepath.Dir(pidFile) - lock, err := acquireDaemonLock(beadsDir, global) + lock, err := acquireDaemonLock(beadsDir, dbPath) if err != nil { if err == ErrDaemonLocked { log.log("Daemon already running (lock held), exiting") @@ -1044,7 +1044,22 @@ func runDaemonLoop(interval time.Duration, autoCommit, autoPush bool, logPath, p logF, log := setupDaemonLogger(logPath) defer func() { _ = logF.Close() }() - lock, err := setupDaemonLock(pidFile, global, log) + // Determine database path first (needed for lock file metadata) + daemonDBPath := "" + if !global { + daemonDBPath = dbPath + if daemonDBPath == "" { + if foundDB := beads.FindDatabasePath(); foundDB != "" { + daemonDBPath = foundDB + } else { + log.log("Error: no beads database found") + log.log("Hint: run 'bd init' to create a database or set BEADS_DB environment variable") + os.Exit(1) + } + } + } + + lock, err := setupDaemonLock(pidFile, daemonDBPath, log) if err != nil { os.Exit(1) } @@ -1058,17 +1073,6 @@ func runDaemonLoop(interval time.Duration, autoCommit, autoPush bool, logPath, p return } - daemonDBPath := dbPath - if daemonDBPath == "" { - if foundDB := beads.FindDatabasePath(); foundDB != "" { - daemonDBPath = foundDB - } else { - log.log("Error: no beads database found") - log.log("Hint: run 'bd init' to create a database or set BEADS_DB environment variable") - os.Exit(1) - } - } - // Check for multiple .db files (ambiguity error) beadsDir := filepath.Dir(daemonDBPath) matches, err := filepath.Glob(filepath.Join(beadsDir, "*.db")) diff --git a/cmd/bd/daemon_lock.go b/cmd/bd/daemon_lock.go index 3071012e..4233a662 100644 --- a/cmd/bd/daemon_lock.go +++ b/cmd/bd/daemon_lock.go @@ -1,16 +1,26 @@ package main import ( + "encoding/json" "errors" "fmt" "os" "path/filepath" "strconv" "strings" + "time" ) var ErrDaemonLocked = errors.New("daemon lock already held by another process") +// DaemonLockInfo represents the metadata stored in the daemon.lock file +type DaemonLockInfo struct { + PID int `json:"pid"` + Database string `json:"database"` + Version string `json:"version"` + StartedAt time.Time `json:"started_at"` +} + // DaemonLock represents a held lock on the daemon.lock file type DaemonLock struct { file *os.File @@ -30,7 +40,8 @@ func (l *DaemonLock) Close() error { // acquireDaemonLock attempts to acquire an exclusive lock on daemon.lock // Returns ErrDaemonLocked if another daemon is already running -func acquireDaemonLock(beadsDir string, _ bool) (*DaemonLock, error) { +// dbPath is the full path to the database file (e.g., /path/to/.beads/beads.db) +func acquireDaemonLock(beadsDir string, dbPath string) (*DaemonLock, error) { lockPath := filepath.Join(beadsDir, "daemon.lock") // Open or create the lock file @@ -48,10 +59,19 @@ func acquireDaemonLock(beadsDir string, _ bool) (*DaemonLock, error) { return nil, fmt.Errorf("cannot lock file: %w", err) } - // Write our PID to the lock file for debugging (optional) + // Write JSON metadata to the lock file + lockInfo := DaemonLockInfo{ + PID: os.Getpid(), + Database: dbPath, + Version: Version, + StartedAt: time.Now().UTC(), + } + _ = f.Truncate(0) _, _ = f.Seek(0, 0) - _, _ = fmt.Fprintf(f, "%d\n", os.Getpid()) + encoder := json.NewEncoder(f) + encoder.SetIndent("", " ") + _ = encoder.Encode(lockInfo) _ = f.Sync() // Also write PID file for Windows compatibility (can't read locked files on Windows) @@ -80,16 +100,23 @@ func tryDaemonLock(beadsDir string) (running bool, pid int) { if err := flockExclusive(f); err != nil { if err == ErrDaemonLocked { // Lock is held - daemon is running - // Try to read PID for display (best effort) - _, _ = f.Seek(0, 0) // Seek to beginning before reading - data := make([]byte, 32) - n, _ := f.Read(data) - if n > 0 { - _, _ = fmt.Sscanf(string(data[:n]), "%d", &pid) - } - // Fallback to PID file if we couldn't read PID from lock file - if pid == 0 { - _, pid = checkPIDFile(beadsDir) + // Try to read PID from JSON format (best effort) + _, _ = f.Seek(0, 0) + var lockInfo DaemonLockInfo + if err := json.NewDecoder(f).Decode(&lockInfo); err == nil { + pid = lockInfo.PID + } else { + // Fallback: try reading as plain integer (old format) + _, _ = f.Seek(0, 0) + data := make([]byte, 32) + n, _ := f.Read(data) + if n > 0 { + _, _ = fmt.Sscanf(string(data[:n]), "%d", &pid) + } + // Fallback to PID file if we couldn't read PID from lock file + if pid == 0 { + _, pid = checkPIDFile(beadsDir) + } } return true, pid } @@ -102,6 +129,55 @@ func tryDaemonLock(beadsDir string) (running bool, pid int) { return false, 0 } +// readDaemonLockInfo reads and parses the daemon lock file +// Returns lock info if available, or error if file doesn't exist or can't be parsed +func readDaemonLockInfo(beadsDir string) (*DaemonLockInfo, error) { + lockPath := filepath.Join(beadsDir, "daemon.lock") + + data, err := os.ReadFile(lockPath) + if err != nil { + return nil, err + } + + var lockInfo DaemonLockInfo + if err := json.Unmarshal(data, &lockInfo); err != nil { + // Try parsing as old format (plain PID) + var pid int + if _, err := fmt.Sscanf(string(data), "%d", &pid); err == nil { + return &DaemonLockInfo{PID: pid}, nil + } + return nil, fmt.Errorf("cannot parse lock file: %w", err) + } + + return &lockInfo, nil +} + +// validateDaemonLock validates that the running daemon matches expected parameters +// Returns error if validation fails (mismatch detected) +func validateDaemonLock(beadsDir string, expectedDB string) error { + lockInfo, err := readDaemonLockInfo(beadsDir) + if err != nil { + // No lock file or can't read - not an error for validation + return nil + } + + // Validate database path if specified in lock + if lockInfo.Database != "" && expectedDB != "" { + if lockInfo.Database != expectedDB { + return fmt.Errorf("daemon database mismatch: daemon uses %s but expected %s", lockInfo.Database, expectedDB) + } + } + + // Version mismatch is a warning, not a hard error (handled elsewhere) + // But we return the info for caller to decide + if lockInfo.Version != "" && lockInfo.Version != Version { + // Not a hard error - version compatibility check happens via RPC + // This is just informational + } + + return nil +} + // checkPIDFile checks if a daemon is running by reading the PID file. // This is used for backward compatibility with pre-lock daemons. func checkPIDFile(beadsDir string) (running bool, pid int) { diff --git a/cmd/bd/daemon_lock_test.go b/cmd/bd/daemon_lock_test.go index 181a85c9..37587c03 100644 --- a/cmd/bd/daemon_lock_test.go +++ b/cmd/bd/daemon_lock_test.go @@ -17,15 +17,17 @@ func TestDaemonLockPreventsMultipleInstances(t *testing.T) { t.Fatal(err) } + dbPath := filepath.Join(beadsDir, "beads.db") + // Acquire lock - lock1, err := acquireDaemonLock(beadsDir, false) + lock1, err := acquireDaemonLock(beadsDir, dbPath) if err != nil { t.Fatalf("Failed to acquire first lock: %v", err) } defer lock1.Close() // Try to acquire lock again - should fail - lock2, err := acquireDaemonLock(beadsDir, false) + lock2, err := acquireDaemonLock(beadsDir, dbPath) if err != ErrDaemonLocked { if lock2 != nil { lock2.Close() @@ -37,7 +39,7 @@ func TestDaemonLockPreventsMultipleInstances(t *testing.T) { lock1.Close() // Now should be able to acquire lock - lock3, err := acquireDaemonLock(beadsDir, false) + lock3, err := acquireDaemonLock(beadsDir, dbPath) if err != nil { t.Fatalf("Failed to acquire lock after release: %v", err) } @@ -51,6 +53,8 @@ func TestTryDaemonLockDetectsRunning(t *testing.T) { t.Fatal(err) } + dbPath := filepath.Join(beadsDir, "beads.db") + // Initially no daemon running running, _ := tryDaemonLock(beadsDir) if running { @@ -58,7 +62,7 @@ func TestTryDaemonLockDetectsRunning(t *testing.T) { } // Acquire lock - lock, err := acquireDaemonLock(beadsDir, false) + lock, err := acquireDaemonLock(beadsDir, dbPath) if err != nil { t.Fatalf("Failed to acquire lock: %v", err) } @@ -107,6 +111,78 @@ func TestBackwardCompatibilityWithOldDaemon(t *testing.T) { } } +func TestDaemonLockJSONFormat(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0700); err != nil { + t.Fatal(err) + } + + dbPath := filepath.Join(beadsDir, "beads.db") + + // Acquire lock + lock, err := acquireDaemonLock(beadsDir, dbPath) + if err != nil { + t.Fatalf("Failed to acquire lock: %v", err) + } + defer lock.Close() + + // Read the lock file and verify JSON format + lockInfo, err := readDaemonLockInfo(beadsDir) + if err != nil { + t.Fatalf("Failed to read lock info: %v", err) + } + + if lockInfo.PID != os.Getpid() { + t.Errorf("Expected PID %d, got %d", os.Getpid(), lockInfo.PID) + } + + if lockInfo.Database != dbPath { + t.Errorf("Expected database %s, got %s", dbPath, lockInfo.Database) + } + + if lockInfo.Version != Version { + t.Errorf("Expected version %s, got %s", Version, lockInfo.Version) + } + + if lockInfo.StartedAt.IsZero() { + t.Error("Expected non-zero StartedAt timestamp") + } +} + +func TestValidateDaemonLock(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0700); err != nil { + t.Fatal(err) + } + + dbPath := filepath.Join(beadsDir, "beads.db") + + // No lock file - validation should pass + if err := validateDaemonLock(beadsDir, dbPath); err != nil { + t.Errorf("Expected no error when no lock file exists, got: %v", err) + } + + // Acquire lock with correct database + lock, err := acquireDaemonLock(beadsDir, dbPath) + if err != nil { + t.Fatalf("Failed to acquire lock: %v", err) + } + defer lock.Close() + + // Validation should pass with matching database + if err := validateDaemonLock(beadsDir, dbPath); err != nil { + t.Errorf("Expected no error with matching database, got: %v", err) + } + + // Validation should fail with different database + wrongDB := filepath.Join(beadsDir, "wrong.db") + if err := validateDaemonLock(beadsDir, wrongDB); err == nil { + t.Error("Expected error with mismatched database") + } +} + func TestMultipleDaemonProcessesRace(t *testing.T) { if testing.Short() { t.Skip("Skipping race condition test in short mode") diff --git a/cmd/bd/daemon_test.go b/cmd/bd/daemon_test.go index d834c5ab..8b3e48c3 100644 --- a/cmd/bd/daemon_test.go +++ b/cmd/bd/daemon_test.go @@ -132,7 +132,8 @@ func TestIsDaemonRunning_CurrentProcess(t *testing.T) { // Acquire the daemon lock to simulate a running daemon beadsDir := filepath.Dir(pidFile) - lock, err := acquireDaemonLock(beadsDir, false) + dbPath := filepath.Join(beadsDir, "beads.db") + lock, err := acquireDaemonLock(beadsDir, dbPath) if err != nil { t.Fatalf("Failed to acquire daemon lock: %v", err) } diff --git a/cmd/bd/main.go b/cmd/bd/main.go index 6f1b1171..248aeaa1 100644 --- a/cmd/bd/main.go +++ b/cmd/bd/main.go @@ -269,18 +269,30 @@ var rootCmd = &cobra.Command{ daemonStatus.Detail = fmt.Sprintf("version mismatch (daemon: %s, client: %s) and restart failed", health.Version, Version) } else { - // Daemon is healthy and compatible - use it - daemonClient = client - daemonStatus.Mode = cmdDaemon - daemonStatus.Connected = true - daemonStatus.Degraded = false - daemonStatus.Health = health.Status - if os.Getenv("BD_DEBUG") != "" { - fmt.Fprintf(os.Stderr, "Debug: connected to daemon at %s (health: %s)\n", socketPath, health.Status) + // Daemon is healthy and compatible - validate database path + beadsDir := filepath.Dir(dbPath) + if err := validateDaemonLock(beadsDir, dbPath); err != nil { + _ = client.Close() + daemonStatus.FallbackReason = FallbackHealthFailed + daemonStatus.Detail = fmt.Sprintf("daemon lock validation failed: %v", err) + if os.Getenv("BD_DEBUG") != "" { + fmt.Fprintf(os.Stderr, "Debug: daemon lock validation failed: %v\n", err) + } + // Fall through to direct mode + } else { + // Daemon is healthy, compatible, and validated - use it + daemonClient = client + daemonStatus.Mode = cmdDaemon + daemonStatus.Connected = true + daemonStatus.Degraded = false + daemonStatus.Health = health.Status + if os.Getenv("BD_DEBUG") != "" { + fmt.Fprintf(os.Stderr, "Debug: connected to daemon at %s (health: %s)\n", socketPath, health.Status) + } + // Warn if using daemon with git worktrees + warnWorktreeDaemon(dbPath) + return // Skip direct storage initialization } - // Warn if using daemon with git worktrees - warnWorktreeDaemon(dbPath) - return // Skip direct storage initialization } } else { // Health check failed or daemon unhealthy