From f724b612d2fb3c82903ca3fb5da84a382cd56a1d Mon Sep 17 00:00:00 2001 From: Marco Del Pin Date: Fri, 21 Nov 2025 21:22:33 +0100 Subject: [PATCH] fix: Deduplicate database paths when symlinks present (#354) Fixes a bug where FindAllDatabases() would return the same database multiple times when symlinks were present in the directory hierarchy. The issue occurred because the function walked up the directory tree without resolving symlinks or checking if a database had already been discovered. This led to confusing warnings about "multiple databases" even when only one physical database existed. Changes: - Added symlink resolution using filepath.EvalSymlinks() - Implemented deduplication using a seen map with canonical paths - Added comprehensive tests for symlink scenarios This ensures that each unique database is reported exactly once, regardless of how many symlinks point to it in the directory tree. Resolves duplicate database warnings when symlinks are present. --- internal/beads/beads.go | 26 +++- internal/beads/beads_symlink_test.go | 174 +++++++++++++++++++++++++++ 2 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 internal/beads/beads_symlink_test.go diff --git a/internal/beads/beads.go b/internal/beads/beads.go index fb03605d..28b1b3a2 100644 --- a/internal/beads/beads.go +++ b/internal/beads/beads.go @@ -346,7 +346,8 @@ func findDatabaseInTree() string { // closest to CWD (most relevant) to the furthest (least relevant). func FindAllDatabases() []DatabaseInfo { var databases []DatabaseInfo - + seen := make(map[string]bool) // Track canonical paths to avoid duplicates + dir, err := os.Getwd() if err != nil { return databases @@ -359,9 +360,28 @@ func FindAllDatabases() []DatabaseInfo { // Found .beads/ directory, look for *.db files matches, err := filepath.Glob(filepath.Join(beadsDir, "*.db")) if err == nil && len(matches) > 0 { + dbPath := matches[0] + + // Resolve symlinks to get canonical path for deduplication + canonicalPath := dbPath + if resolved, err := filepath.EvalSymlinks(dbPath); err == nil { + canonicalPath = resolved + } + + // Skip if we've already seen this database (via symlink or other path) + if seen[canonicalPath] { + // Move up one directory + parent := filepath.Dir(dir) + if parent == dir { + break + } + dir = parent + continue + } + seen[canonicalPath] = true + // Count issues if we can open the database (best-effort) issueCount := -1 - dbPath := matches[0] // Don't fail if we can't open/query the database - it might be locked // or corrupted, but we still want to detect and warn about it ctx := context.Background() @@ -372,7 +392,7 @@ func FindAllDatabases() []DatabaseInfo { } _ = store.Close() } - + databases = append(databases, DatabaseInfo{ Path: dbPath, BeadsDir: beadsDir, diff --git a/internal/beads/beads_symlink_test.go b/internal/beads/beads_symlink_test.go new file mode 100644 index 00000000..e28bfdc8 --- /dev/null +++ b/internal/beads/beads_symlink_test.go @@ -0,0 +1,174 @@ +//go:build integration +// +build integration + +package beads + +import ( + "os" + "path/filepath" + "testing" +) + +// TestFindAllDatabases_SymlinkDeduplication verifies that FindAllDatabases +// properly deduplicates databases when symlinks are present in the path +func TestFindAllDatabases_SymlinkDeduplication(t *testing.T) { + // Create a temporary directory structure + tmpDir, err := os.MkdirTemp("", "beads-symlink-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Resolve symlinks (macOS /var -> /private/var, etc.) + tmpDir, err = filepath.EvalSymlinks(tmpDir) + if err != nil { + t.Fatal(err) + } + + // Create structure: + // tmpDir/ + // real/ + // .beads/test.db + // symlink_to_real -> real/ + // subdir/ + // (working directory here) + + // Create real directory with .beads database + realDir := filepath.Join(tmpDir, "real") + if err := os.MkdirAll(realDir, 0750); err != nil { + t.Fatal(err) + } + + beadsDir := filepath.Join(realDir, ".beads") + if err := os.MkdirAll(beadsDir, 0750); err != nil { + t.Fatal(err) + } + + dbPath := filepath.Join(beadsDir, "test.db") + if err := os.WriteFile(dbPath, []byte("fake db"), 0600); err != nil { + t.Fatal(err) + } + + // Create symlink to real directory + symlinkDir := filepath.Join(tmpDir, "symlink_to_real") + if err := os.Symlink(realDir, symlinkDir); err != nil { + t.Skip("Cannot create symlinks on this system (may require admin on Windows)") + } + + // Create subdirectory as working directory + subdir := filepath.Join(symlinkDir, "subdir") + if err := os.MkdirAll(subdir, 0750); err != nil { + t.Fatal(err) + } + + // Save original working directory + origDir, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer os.Chdir(origDir) + + // Change to subdir (which is inside the symlinked directory) + if err := os.Chdir(subdir); err != nil { + t.Fatal(err) + } + + // Call FindAllDatabases + databases := FindAllDatabases() + + // Should find exactly ONE database, not two + // Without the fix, it would find the same database twice: + // - Once via symlink_to_real/.beads/test.db + // - Once via real/.beads/test.db (when walking up to parent) + if len(databases) != 1 { + t.Errorf("expected 1 database (with deduplication), got %d", len(databases)) + for i, db := range databases { + t.Logf(" Database %d: %s", i, db.Path) + } + } + + // Verify it's the database we expect + resolvedDbPath, err := filepath.EvalSymlinks(dbPath) + if err == nil { + // Check if the found database matches the canonical path + foundDbPath, err := filepath.EvalSymlinks(databases[0].Path) + if err == nil && foundDbPath != resolvedDbPath { + t.Errorf("expected database %s, got %s", resolvedDbPath, foundDbPath) + } + } +} + +// TestFindAllDatabases_MultipleSymlinksToSameDB tests that multiple symlinks +// pointing to the same database are properly deduplicated +func TestFindAllDatabases_MultipleSymlinksToSameDB(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "beads-multisymlink-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Resolve symlinks + tmpDir, err = filepath.EvalSymlinks(tmpDir) + if err != nil { + t.Fatal(err) + } + + // Create structure: + // tmpDir/ + // real/ + // .beads/test.db + // link1 -> real/ + // link2 -> real/ + // workdir/ + + // Create real directory with database + realDir := filepath.Join(tmpDir, "real") + beadsDir := filepath.Join(realDir, ".beads") + if err := os.MkdirAll(beadsDir, 0750); err != nil { + t.Fatal(err) + } + + dbPath := filepath.Join(beadsDir, "test.db") + if err := os.WriteFile(dbPath, []byte("fake db"), 0600); err != nil { + t.Fatal(err) + } + + // Create multiple symlinks + link1 := filepath.Join(tmpDir, "link1") + if err := os.Symlink(realDir, link1); err != nil { + t.Skip("Cannot create symlinks on this system") + } + + link2 := filepath.Join(tmpDir, "link2") + if err := os.Symlink(realDir, link2); err != nil { + t.Fatal(err) + } + + // Create working directory + workdir := filepath.Join(tmpDir, "workdir") + if err := os.MkdirAll(workdir, 0750); err != nil { + t.Fatal(err) + } + + // Save and change directory + origDir, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer os.Chdir(origDir) + + if err := os.Chdir(workdir); err != nil { + t.Fatal(err) + } + + // Find databases + databases := FindAllDatabases() + + // Should find exactly 1 database (all paths resolve to the same real database) + if len(databases) != 1 { + t.Errorf("expected 1 database with deduplication, got %d", len(databases)) + for i, db := range databases { + t.Logf(" Database %d: %s", i, db.Path) + } + } +}