From 5a6177b4bc1acbd380017b493cf906458c6d81bd Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Wed, 22 Oct 2025 23:05:00 -0700 Subject: [PATCH] Fix bd-73: Add git worktree detection and warnings - Implement robust worktree detection using git-dir vs git-common-dir comparison - Add prominent warning when daemon mode is active in a worktree - Warn in 3 places: initial connection, auto-start, and daemon start command - Show shared database path and clarify BEADS_AUTO_START_DAEMON behavior - Document limitations and solutions in README.md and AGENTS.md - Add comprehensive tests for detection and path truncation Fixes #55 Amp-Thread-ID: https://ampcode.com/threads/T-254eb9e3-1a42-42d7-afdf-b7ca2d2dcb8b Co-authored-by: Amp --- .beads/beads.jsonl | 2 + AGENTS.md | 38 +++++++++++++ README.md | 34 ++++++++++++ cmd/bd/daemon.go | 13 +++++ cmd/bd/main.go | 4 ++ cmd/bd/worktree.go | 93 +++++++++++++++++++++++++++++++ cmd/bd/worktree_test.go | 118 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 302 insertions(+) create mode 100644 cmd/bd/worktree.go create mode 100644 cmd/bd/worktree_test.go diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index 7c14e774..a358ef7e 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -67,5 +67,7 @@ {"id":"bd-7","title":"Write tests for merge functionality","description":"Unit tests: validation, merge logic, data integrity. Integration tests: end-to-end workflow, export/import. Edge case tests: chains, circular refs, epics.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-21T23:53:44.31362-07:00","updated_at":"2025-10-22T21:23:20.836984-07:00","closed_at":"2025-10-22T01:07:04.72062-07:00"} {"id":"bd-70","title":"Fix bd sync prefix mismatch error message suggesting non-existent flag","description":"GH #103: bd sync suggests using --rename-on-import flag that doesn't exist. Need to either implement the flag or fix the error message to suggest the correct workflow.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-22T17:54:24.473508-07:00","updated_at":"2025-10-22T21:23:20.837249-07:00","closed_at":"2025-10-22T17:57:46.973029-07:00"} {"id":"bd-71","title":"Fix MCP close tool method signature error","description":"GH #107: MCP close() tool fails with \"BdDaemonClient.close() takes 1 positional argument but 2 were given\". Need to fix method signature in beads-mcp server.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-22T19:17:05.429429-07:00","updated_at":"2025-10-22T21:23:20.837473-07:00","closed_at":"2025-10-22T19:19:54.601153-07:00"} +{"id":"bd-72","title":"Update Claude Code marketplace plugin","description":"Update the beads plugin in the Claude Code marketplace to the latest version. This may help resolve some of the open GitHub issues related to marketplace installation and compatibility (#54, #112).\n\nShould include:\n- Latest beads version\n- Updated documentation\n- Any new features or bug fixes","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-22T22:29:11.293161-07:00","updated_at":"2025-10-22T22:29:11.293161-07:00"} +{"id":"bd-73","title":"Git worktree support - ensure --no-daemon works correctly","description":"Users working with git worktrees report that beads MCP commits to the wrong branch (main instead of worktree branch). \n\n**Root cause:** Git worktrees share the same .beads directory, so the daemon/MCP server doesn't know which branch the worktree has checked out.\n\n**Current state:**\n- Daemon mode: Cannot work properly with worktrees (fundamental limitation - shared .beads, unknown branch)\n- CLI with --no-daemon: Should work but needs verification\n\n**Action items:**\n1. Test and verify --no-daemon works correctly in worktrees\n2. Document the limitation in README/AGENTS.md\n3. Add clear error/warning when using daemon with worktrees\n4. Consider if MCP server can detect worktree usage and auto-disable daemon\n\n**Workaround for users:**\nUse --no-daemon flag when working in worktrees, or set BEADS_AUTO_DAEMON=false\n\n**Related:** GH issue #55","notes":"**Implementation completed with oracle review improvements:**\n\n1. **Robust detection** - Changed from substring check to canonical git-dir vs git-common-dir comparison\n2. **Correct warning gate** - Now only checks isGitWorktree() and only called when daemon is actually connected\n3. **Complete coverage** - Added warning to `bd daemon` command start path\n4. **Better UX** - Shows shared database path and clarifies BEADS_AUTO_START_DAEMON behavior\n5. **Improved tests** - Validates canonical detection method and path truncation\n\n**Files changed:**\n- cmd/bd/worktree.go - Improved detection and warning\n- cmd/bd/worktree_test.go - Better test coverage\n- cmd/bd/main.go - Warning after daemon connection (2 places)\n- cmd/bd/daemon.go - Warning when starting daemon\n- README.md \u0026 AGENTS.md - Documented limitations and solutions","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-10-22T22:38:52.438601-07:00","updated_at":"2025-10-22T22:58:00.306114-07:00","closed_at":"2025-10-22T22:51:45.464598-07:00"} {"id":"bd-8","title":"Improve error handling in dependency removal during remapping","description":"In updateDependencyReferences(), RemoveDependency errors are caught and ignored with continue (line 392). Comment says 'if dependency doesn't exist' but this catches ALL errors including real failures. Should check error type with errors.Is(err, ErrDependencyNotFound) and only ignore not-found errors, returning other errors properly.","status":"closed","priority":3,"issue_type":"bug","created_at":"2025-10-21T23:53:44.31362-07:00","updated_at":"2025-10-22T21:23:20.837687-07:00","closed_at":"2025-10-18T09:41:18.209717-07:00"} {"id":"bd-9","title":"Test issue 2","description":"","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-21T23:53:44.31362-07:00","updated_at":"2025-10-22T21:23:20.8379-07:00","closed_at":"2025-10-21T22:06:41.257019-07:00","labels":["test-label"]} diff --git a/AGENTS.md b/AGENTS.md index b825ecef..e5b9596b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -309,6 +309,44 @@ bd ready # Fresh data from git! **Optional**: Use the git hooks in `examples/git-hooks/` for immediate export (no 5-second wait) and guaranteed import after git operations. Not required with auto-sync enabled. +### Git Worktrees + +**⚠️ Important Limitation:** Daemon mode does not work correctly with `git worktree`. + +**The Problem:** +Git worktrees share the same `.git` directory and thus share the same `.beads` database. The daemon doesn't know which branch each worktree has checked out, which can cause it to commit/push to the wrong branch. + +**What you lose without daemon mode:** +- **Auto-sync** - No automatic commit/push of changes (use `bd sync` manually) +- **MCP server** - The beads-mcp server requires daemon mode for multi-repo support +- **Background watching** - No automatic detection of remote changes + +**Solutions for Worktree Users:** + +1. **Use `--no-daemon` flag** (recommended): + ```bash + bd --no-daemon ready + bd --no-daemon create "Fix bug" -p 1 + bd --no-daemon update bd-42 --status in_progress + ``` + +2. **Disable daemon via environment variable** (for entire worktree session): + ```bash + export BEADS_NO_DAEMON=1 + bd ready # All commands use direct mode + ``` + +3. **Disable auto-start** (less safe, still warns): + ```bash + export BEADS_AUTO_START_DAEMON=false + ``` + +**Automatic Detection:** +bd automatically detects when you're in a worktree and shows a prominent warning if daemon mode is active. The `--no-daemon` mode works correctly with worktrees since it operates directly on the database without shared state. + +**Why It Matters:** +The daemon maintains its own view of the current working directory and git state. When multiple worktrees share the same `.beads` database, the daemon may commit changes intended for one branch to a different branch, leading to confusion and incorrect git history. + ### Handling Import Collisions When merging branches or pulling changes, you may encounter ID collisions (same ID, different content). bd detects and safely handles these: diff --git a/README.md b/README.md index a06c7322..fee82a0f 100644 --- a/README.md +++ b/README.md @@ -646,10 +646,44 @@ bd --db ~/otherproject/.beads/other.db list - `BEADS_DB` - Override database path - `BEADS_AUTO_START_DAEMON` - Enable/disable automatic daemon start (default: `true`). Set to `false` or `0` to disable. +- `BEADS_NO_DAEMON` - Disable daemon mode entirely (same as `--no-daemon` flag). Set to `1` or `true` to disable. - `BD_ACTOR` - Set actor name for change tracking (defaults to `$USER`) - `BD_DEBUG` - Enable debug logging (connection attempts, auto-start timing, health checks) - `BD_VERBOSE` - Show warnings when falling back from daemon to direct mode +### Git Worktrees + +**⚠️ Important:** Git worktrees share the same `.beads` directory, which creates issues with daemon mode. + +**The Problem:** +When using `git worktree`, multiple working directories share a single `.git` directory and thus share the same `.beads` database. The daemon doesn't know which branch each worktree has checked out, which can cause it to commit/push to the wrong branch. + +**What you lose without daemon mode:** +- **Auto-sync** - No automatic commit/push of changes (use `bd sync` manually) +- **MCP server** - The beads-mcp server requires daemon mode for multi-repo support +- **Background watching** - No automatic detection of remote changes + +**Solutions:** + +1. **Use `--no-daemon` flag** (recommended): + ```bash + bd --no-daemon ready + bd --no-daemon create "Fix bug" -p 1 + ``` + +2. **Disable daemon via environment variable**: + ```bash + export BEADS_NO_DAEMON=1 + bd ready # Will use direct mode + ``` + +3. **Disable auto-start** (for this worktree only): + ```bash + export BEADS_AUTO_START_DAEMON=false + ``` + +bd will automatically detect when you're in a worktree and show a warning if daemon mode is active. The `--no-daemon` mode works correctly with worktrees since it operates directly on the database without shared state. + ### Version Compatibility The daemon and CLI check version compatibility automatically: diff --git a/cmd/bd/daemon.go b/cmd/bd/daemon.go index 1cba9935..da21490d 100644 --- a/cmd/bd/daemon.go +++ b/cmd/bd/daemon.go @@ -117,6 +117,19 @@ Use --health to check daemon health and metrics.`, os.Exit(1) } + // Warn if starting daemon in a git worktree + if !global { + // Ensure dbPath is set for warning + if dbPath == "" { + if foundDB := beads.FindDatabasePath(); foundDB != "" { + dbPath = foundDB + } + } + if dbPath != "" { + warnWorktreeDaemon(dbPath) + } + } + // Start daemon scope := "local" if global { diff --git a/cmd/bd/main.go b/cmd/bd/main.go index de4f0b93..dd6f3be4 100644 --- a/cmd/bd/main.go +++ b/cmd/bd/main.go @@ -188,6 +188,8 @@ var rootCmd = &cobra.Command{ 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 } else { // Health check failed or daemon unhealthy @@ -248,6 +250,8 @@ var rootCmd = &cobra.Command{ elapsed := time.Since(startTime).Milliseconds() fmt.Fprintf(os.Stderr, "Debug: auto-start succeeded; connected at %s in %dms\n", socketPath, elapsed) } + // Warn if using daemon with git worktrees + warnWorktreeDaemon(dbPath) return // Skip direct storage initialization } else { // Auto-started daemon is unhealthy diff --git a/cmd/bd/worktree.go b/cmd/bd/worktree.go new file mode 100644 index 00000000..ae5c877e --- /dev/null +++ b/cmd/bd/worktree.go @@ -0,0 +1,93 @@ +package main + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" +) + +// isGitWorktree detects if the current directory is in a git worktree +// by comparing --git-dir and --git-common-dir (canonical detection method) +func isGitWorktree() bool { + gitDir := gitRevParse("--git-dir") + if gitDir == "" { + return false + } + + commonDir := gitRevParse("--git-common-dir") + if commonDir == "" { + return false + } + + absGit, err1 := filepath.Abs(gitDir) + absCommon, err2 := filepath.Abs(commonDir) + if err1 != nil || err2 != nil { + return false + } + + return absGit != absCommon +} + +// gitRevParse runs git rev-parse with the given flag and returns the trimmed output +func gitRevParse(flag string) string { + out, err := exec.Command("git", "rev-parse", flag).Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +// getWorktreeGitDir returns the .git directory path for a worktree +// Returns empty string if not in a git repo or not a worktree +func getWorktreeGitDir() string { + cmd := exec.Command("git", "rev-parse", "--git-dir") + out, err := cmd.Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +// warnWorktreeDaemon prints a warning if using daemon with worktrees +// Call this only when daemon mode is actually active (connected) +func warnWorktreeDaemon(dbPathForWarning string) { + if !isGitWorktree() { + return + } + + gitDir := getWorktreeGitDir() + beadsDir := filepath.Dir(dbPathForWarning) + if beadsDir == "." || beadsDir == "" { + beadsDir = dbPathForWarning + } + + fmt.Fprintln(os.Stderr) + fmt.Fprintln(os.Stderr, "╔══════════════════════════════════════════════════════════════════════════╗") + fmt.Fprintln(os.Stderr, "║ WARNING: Git worktree detected with daemon mode ║") + fmt.Fprintln(os.Stderr, "╠══════════════════════════════════════════════════════════════════════════╣") + fmt.Fprintln(os.Stderr, "║ Git worktrees share the same .beads directory, which can cause the ║") + fmt.Fprintln(os.Stderr, "║ daemon to commit/push to the wrong branch. ║") + fmt.Fprintln(os.Stderr, "║ ║") + fmt.Fprintf(os.Stderr, "║ Shared database: %-55s ║\n", truncateForBox(beadsDir, 55)) + fmt.Fprintf(os.Stderr, "║ Worktree git dir: %-54s ║\n", truncateForBox(gitDir, 54)) + fmt.Fprintln(os.Stderr, "║ ║") + fmt.Fprintln(os.Stderr, "║ RECOMMENDED SOLUTIONS: ║") + fmt.Fprintln(os.Stderr, "║ 1. Use --no-daemon flag: bd --no-daemon ║") + fmt.Fprintln(os.Stderr, "║ 2. Disable daemon mode: export BEADS_NO_DAEMON=1 ║") + fmt.Fprintln(os.Stderr, "║ ║") + fmt.Fprintln(os.Stderr, "║ Note: BEADS_AUTO_START_DAEMON=false only prevents auto-start; ║") + fmt.Fprintln(os.Stderr, "║ you can still connect to a running daemon. ║") + fmt.Fprintln(os.Stderr, "╚══════════════════════════════════════════════════════════════════════════╝") + fmt.Fprintln(os.Stderr) +} + +// truncateForBox truncates a path to fit in the warning box +func truncateForBox(path string, maxLen int) string { + if len(path) <= maxLen { + return path + } + // Truncate with ellipsis + return "..." + path[len(path)-(maxLen-3):] +} diff --git a/cmd/bd/worktree_test.go b/cmd/bd/worktree_test.go new file mode 100644 index 00000000..8e7a5808 --- /dev/null +++ b/cmd/bd/worktree_test.go @@ -0,0 +1,118 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +func TestIsGitWorktree(t *testing.T) { + // Save current directory + origDir, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer os.Chdir(origDir) + + // Create a temp directory for our test repo + tmpDir := t.TempDir() + + // Initialize a git repo + mainRepo := filepath.Join(tmpDir, "main") + if err := os.Mkdir(mainRepo, 0755); err != nil { + t.Fatal(err) + } + + // Initialize main git repo + if err := os.Chdir(mainRepo); err != nil { + t.Fatal(err) + } + + if err := exec.Command("git", "init").Run(); err != nil { + t.Skip("git not available") + } + + if err := exec.Command("git", "config", "user.email", "test@example.com").Run(); err != nil { + t.Fatal(err) + } + if err := exec.Command("git", "config", "user.name", "Test User").Run(); err != nil { + t.Fatal(err) + } + + // Create a commit + readmeFile := filepath.Join(mainRepo, "README.md") + if err := os.WriteFile(readmeFile, []byte("# Test\n"), 0644); err != nil { + t.Fatal(err) + } + if err := exec.Command("git", "add", "README.md").Run(); err != nil { + t.Fatal(err) + } + if err := exec.Command("git", "commit", "-m", "Initial commit").Run(); err != nil { + t.Fatal(err) + } + + // Test 1: Main repo should NOT be a worktree + if isGitWorktree() { + t.Error("Main repository should not be detected as a worktree") + } + + // Create a worktree + worktreeDir := filepath.Join(tmpDir, "worktree") + if err := exec.Command("git", "worktree", "add", worktreeDir, "-b", "feature").Run(); err != nil { + t.Skip("git worktree not available") + } + + // Change to worktree directory + if err := os.Chdir(worktreeDir); err != nil { + t.Fatal(err) + } + + // Test 2: Worktree should be detected + if !isGitWorktree() { + t.Error("Worktree should be detected as a worktree") + } + + // Test 3: Verify git-dir != git-common-dir in worktree + wtGitDir := gitRevParse("--git-dir") + wtCommonDir := gitRevParse("--git-common-dir") + if wtGitDir == "" || wtCommonDir == "" { + t.Error("git rev-parse should return valid paths in worktree") + } + if wtGitDir == wtCommonDir { + t.Errorf("In worktree, git-dir (%s) should differ from git-common-dir (%s)", wtGitDir, wtCommonDir) + } + + // Clean up worktree + if err := os.Chdir(mainRepo); err != nil { + t.Fatal(err) + } + if err := exec.Command("git", "worktree", "remove", worktreeDir).Run(); err != nil { + t.Logf("Warning: failed to clean up worktree: %v", err) + } +} + +func TestTruncateForBox(t *testing.T) { + tests := []struct { + name string + path string + maxLen int + want string + }{ + {"short path", "/home/user", 20, "/home/user"}, + {"exact length", "/home/user/test", 15, "/home/user/test"}, + {"long path", "/very/long/path/to/database/file.db", 20, ".../database/file.db"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := truncateForBox(tt.path, tt.maxLen) + if len(got) > tt.maxLen { + t.Errorf("truncateForBox() result too long: got %d chars, want <= %d", len(got), tt.maxLen) + } + if len(tt.path) <= tt.maxLen && got != tt.path { + t.Errorf("truncateForBox() shouldn't truncate short paths: got %q, want %q", got, tt.path) + } + }) + } +}