From 665e5b3a87548615404774d74fd442a3079ffdbe Mon Sep 17 00:00:00 2001 From: "Charles P. Cross" Date: Mon, 22 Dec 2025 18:57:43 -0500 Subject: [PATCH] fix(worktree): integrate health check into CreateBeadsWorktree to prevent redundant checks The error message 'path exists but is not a valid git worktree' was appearing in daemon.log when the daemon attempted to use an existing worktree that was in the git worktree list but had other issues (broken sparse checkout, etc.). Root cause: - CreateBeadsWorktree only checked isValidWorktree (is it in git worktree list) - CheckWorktreeHealth was called separately and checked additional things - If the worktree passed isValidWorktree but failed health check, an error was logged and repair was attempted Fix: - CreateBeadsWorktree now performs a full health check when it finds an existing worktree that's in the git worktree list - If the health check fails, it automatically removes and recreates the worktree - Removed redundant CheckWorktreeHealth calls in daemon_sync_branch.go and syncbranch/worktree.go since CreateBeadsWorktree now handles this internally This eliminates the confusing error message and ensures worktrees are always in a healthy state after CreateBeadsWorktree returns successfully. --- cmd/bd/daemon_sync_branch.go | 16 +++------------- internal/git/worktree.go | 19 ++++++++++++++----- internal/syncbranch/worktree.go | 16 +++------------- 3 files changed, 20 insertions(+), 31 deletions(-) diff --git a/cmd/bd/daemon_sync_branch.go b/cmd/bd/daemon_sync_branch.go index 3b5a2ca7..a8ef7fb0 100644 --- a/cmd/bd/daemon_sync_branch.go +++ b/cmd/bd/daemon_sync_branch.go @@ -62,23 +62,13 @@ func syncBranchCommitAndPushWithOptions(ctx context.Context, store storage.Stora // Initialize worktree manager wtMgr := git.NewWorktreeManager(repoRoot) - // Ensure worktree exists + // Ensure worktree exists and is healthy + // CreateBeadsWorktree now performs a full health check internally and + // automatically repairs unhealthy worktrees by removing and recreating them if err := wtMgr.CreateBeadsWorktree(syncBranch, worktreePath); err != nil { return false, fmt.Errorf("failed to create worktree: %w", err) } - // Check worktree health and repair if needed - if err := wtMgr.CheckWorktreeHealth(worktreePath); err != nil { - log.log("Worktree health check failed, attempting repair: %v", err) - // Try to recreate worktree - if err := wtMgr.RemoveBeadsWorktree(worktreePath); err != nil { - log.log("Failed to remove unhealthy worktree: %v", err) - } - if err := wtMgr.CreateBeadsWorktree(syncBranch, worktreePath); err != nil { - return false, fmt.Errorf("failed to recreate worktree after health check: %w", err) - } - } - // Sync JSONL file to worktree // Get the actual JSONL path (could be issues.jsonl, beads.base.jsonl, etc.) jsonlPath := findJSONLPath() diff --git a/internal/git/worktree.go b/internal/git/worktree.go index 52f7b9cd..9d432003 100644 --- a/internal/git/worktree.go +++ b/internal/git/worktree.go @@ -35,11 +35,20 @@ func (wm *WorktreeManager) CreateBeadsWorktree(branch, worktreePath string) erro if _, err := os.Stat(worktreePath); err == nil { // Worktree path exists, check if it's a valid worktree if valid, err := wm.isValidWorktree(worktreePath); err == nil && valid { - return nil // Already exists and is valid - } - // Path exists but isn't a valid worktree, remove it - if err := os.RemoveAll(worktreePath); err != nil { - return fmt.Errorf("failed to remove invalid worktree path: %w", err) + // Worktree exists and is in git worktree list, verify full health + if err := wm.CheckWorktreeHealth(worktreePath); err == nil { + return nil // Already exists and is fully healthy + } + // Health check failed, try to repair by removing and recreating + if err := wm.RemoveBeadsWorktree(worktreePath); err != nil { + // Log but continue - we'll try to recreate anyway + _ = os.RemoveAll(worktreePath) + } + } else { + // Path exists but isn't a valid worktree, remove it + if err := os.RemoveAll(worktreePath); err != nil { + return fmt.Errorf("failed to remove invalid worktree path: %w", err) + } } } diff --git a/internal/syncbranch/worktree.go b/internal/syncbranch/worktree.go index 9a7536aa..b1559c6b 100644 --- a/internal/syncbranch/worktree.go +++ b/internal/syncbranch/worktree.go @@ -83,23 +83,13 @@ func CommitToSyncBranch(ctx context.Context, repoRoot, syncBranch, jsonlPath str // Initialize worktree manager wtMgr := git.NewWorktreeManager(repoRoot) - // Ensure worktree exists + // Ensure worktree exists and is healthy + // CreateBeadsWorktree performs a full health check internally and + // automatically repairs unhealthy worktrees by removing and recreating them if err := wtMgr.CreateBeadsWorktree(syncBranch, worktreePath); err != nil { return nil, fmt.Errorf("failed to create worktree: %w", err) } - // Check worktree health and repair if needed - if err := wtMgr.CheckWorktreeHealth(worktreePath); err != nil { - // Try to recreate worktree - if err := wtMgr.RemoveBeadsWorktree(worktreePath); err != nil { - // Log but continue - removal might fail but recreation might work - _ = err - } - if err := wtMgr.CreateBeadsWorktree(syncBranch, worktreePath); err != nil { - return nil, fmt.Errorf("failed to recreate worktree after health check: %w", err) - } - } - // Get remote name remote := getRemoteForBranch(ctx, worktreePath, syncBranch)