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.
This commit is contained in:
@@ -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,
|
||||
|
||||
174
internal/beads/beads_symlink_test.go
Normal file
174
internal/beads/beads_symlink_test.go
Normal file
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user