Refactor: Extract path canonicalization into utils.CanonicalizePath()
Extracts duplicated path canonicalization logic (filepath.Abs + EvalSymlinks) into a reusable helper function utils.CanonicalizePath() in internal/utils/path.go. Changes: - Add internal/utils/path.go with CanonicalizePath() function - Add comprehensive tests in internal/utils/path_test.go - Replace inline canonicalization in beads.go:131-140 - Replace inline canonicalization in cmd/bd/main.go:446-454 - Replace inline canonicalization in cmd/bd/nodb.go:25-33 The new helper maintains identical behavior: 1. Converts path to absolute form via filepath.Abs 2. Resolves symlinks via filepath.EvalSymlinks 3. Falls back gracefully on errors (returns absPath if EvalSymlinks fails, returns original path if Abs fails) Fixes bd-efe8 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
12
beads.go
12
beads.go
@@ -18,6 +18,7 @@ import (
|
|||||||
"github.com/steveyegge/beads/internal/storage"
|
"github.com/steveyegge/beads/internal/storage"
|
||||||
"github.com/steveyegge/beads/internal/storage/sqlite"
|
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||||
"github.com/steveyegge/beads/internal/types"
|
"github.com/steveyegge/beads/internal/types"
|
||||||
|
"github.com/steveyegge/beads/internal/utils"
|
||||||
)
|
)
|
||||||
|
|
||||||
// CanonicalDatabaseName is the required database filename for all beads repositories
|
// CanonicalDatabaseName is the required database filename for all beads repositories
|
||||||
@@ -127,16 +128,7 @@ func FindDatabasePath() string {
|
|||||||
// 1. Check BEADS_DIR environment variable (preferred)
|
// 1. Check BEADS_DIR environment variable (preferred)
|
||||||
if beadsDir := os.Getenv("BEADS_DIR"); beadsDir != "" {
|
if beadsDir := os.Getenv("BEADS_DIR"); beadsDir != "" {
|
||||||
// Canonicalize the path to prevent nested .beads directories
|
// Canonicalize the path to prevent nested .beads directories
|
||||||
var absBeadsDir string
|
absBeadsDir := utils.CanonicalizePath(beadsDir)
|
||||||
if absPath, err := filepath.Abs(beadsDir); err == nil {
|
|
||||||
if canonical, err := filepath.EvalSymlinks(absPath); err == nil {
|
|
||||||
absBeadsDir = canonical
|
|
||||||
} else {
|
|
||||||
absBeadsDir = absPath
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
absBeadsDir = beadsDir
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check for config.json first (single source of truth)
|
// Check for config.json first (single source of truth)
|
||||||
if cfg, err := configfile.Load(absBeadsDir); err == nil && cfg != nil {
|
if cfg, err := configfile.Load(absBeadsDir); err == nil && cfg != nil {
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import (
|
|||||||
"github.com/steveyegge/beads/internal/storage"
|
"github.com/steveyegge/beads/internal/storage"
|
||||||
"github.com/steveyegge/beads/internal/storage/memory"
|
"github.com/steveyegge/beads/internal/storage/memory"
|
||||||
"github.com/steveyegge/beads/internal/storage/sqlite"
|
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||||
|
"github.com/steveyegge/beads/internal/utils"
|
||||||
)
|
)
|
||||||
|
|
||||||
// DaemonStatus captures daemon connection state for the current command
|
// DaemonStatus captures daemon connection state for the current command
|
||||||
@@ -443,15 +444,7 @@ var rootCmd = &cobra.Command{
|
|||||||
var beadsDir string
|
var beadsDir string
|
||||||
if envDir := os.Getenv("BEADS_DIR"); envDir != "" {
|
if envDir := os.Getenv("BEADS_DIR"); envDir != "" {
|
||||||
// Canonicalize the path
|
// Canonicalize the path
|
||||||
if absDir, err := filepath.Abs(envDir); err == nil {
|
beadsDir = utils.CanonicalizePath(envDir)
|
||||||
if canonical, err := filepath.EvalSymlinks(absDir); err == nil {
|
|
||||||
beadsDir = canonical
|
|
||||||
} else {
|
|
||||||
beadsDir = absDir
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
beadsDir = envDir
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
// Fall back to current directory
|
// Fall back to current directory
|
||||||
cwd, err := os.Getwd()
|
cwd, err := os.Getwd()
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ import (
|
|||||||
"github.com/steveyegge/beads/internal/config"
|
"github.com/steveyegge/beads/internal/config"
|
||||||
"github.com/steveyegge/beads/internal/storage/memory"
|
"github.com/steveyegge/beads/internal/storage/memory"
|
||||||
"github.com/steveyegge/beads/internal/types"
|
"github.com/steveyegge/beads/internal/types"
|
||||||
|
"github.com/steveyegge/beads/internal/utils"
|
||||||
)
|
)
|
||||||
|
|
||||||
// initializeNoDbMode sets up in-memory storage from JSONL file
|
// initializeNoDbMode sets up in-memory storage from JSONL file
|
||||||
@@ -23,15 +24,7 @@ func initializeNoDbMode() error {
|
|||||||
// Check BEADS_DIR environment variable first
|
// Check BEADS_DIR environment variable first
|
||||||
if envDir := os.Getenv("BEADS_DIR"); envDir != "" {
|
if envDir := os.Getenv("BEADS_DIR"); envDir != "" {
|
||||||
// Canonicalize the path
|
// Canonicalize the path
|
||||||
if absDir, err := filepath.Abs(envDir); err == nil {
|
beadsDir = utils.CanonicalizePath(envDir)
|
||||||
if canonical, err := filepath.EvalSymlinks(absDir); err == nil {
|
|
||||||
beadsDir = canonical
|
|
||||||
} else {
|
|
||||||
beadsDir = absDir
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
beadsDir = envDir
|
|
||||||
}
|
|
||||||
} else {
|
} else {
|
||||||
// Fall back to current directory
|
// Fall back to current directory
|
||||||
cwd, err := os.Getwd()
|
cwd, err := os.Getwd()
|
||||||
|
|||||||
34
internal/utils/path.go
Normal file
34
internal/utils/path.go
Normal file
@@ -0,0 +1,34 @@
|
|||||||
|
// Package utils provides utility functions for issue ID parsing and path handling.
|
||||||
|
package utils
|
||||||
|
|
||||||
|
import (
|
||||||
|
"path/filepath"
|
||||||
|
)
|
||||||
|
|
||||||
|
// CanonicalizePath converts a path to its canonical form by:
|
||||||
|
// 1. Converting to absolute path
|
||||||
|
// 2. Resolving symlinks
|
||||||
|
//
|
||||||
|
// If either step fails, it falls back to the best available form:
|
||||||
|
// - If symlink resolution fails, returns absolute path
|
||||||
|
// - If absolute path conversion fails, returns original path
|
||||||
|
//
|
||||||
|
// This function is used to ensure consistent path handling across the codebase,
|
||||||
|
// particularly for BEADS_DIR environment variable processing.
|
||||||
|
func CanonicalizePath(path string) string {
|
||||||
|
// Try to get absolute path
|
||||||
|
absPath, err := filepath.Abs(path)
|
||||||
|
if err != nil {
|
||||||
|
// If we can't get absolute path, return original
|
||||||
|
return path
|
||||||
|
}
|
||||||
|
|
||||||
|
// Try to resolve symlinks
|
||||||
|
canonical, err := filepath.EvalSymlinks(absPath)
|
||||||
|
if err != nil {
|
||||||
|
// If we can't resolve symlinks, return absolute path
|
||||||
|
return absPath
|
||||||
|
}
|
||||||
|
|
||||||
|
return canonical
|
||||||
|
}
|
||||||
105
internal/utils/path_test.go
Normal file
105
internal/utils/path_test.go
Normal file
@@ -0,0 +1,105 @@
|
|||||||
|
package utils
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestCanonicalizePath(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
input string
|
||||||
|
validate func(t *testing.T, result string)
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "absolute path",
|
||||||
|
input: "/tmp/test",
|
||||||
|
validate: func(t *testing.T, result string) {
|
||||||
|
if !filepath.IsAbs(result) {
|
||||||
|
t.Errorf("expected absolute path, got %q", result)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "relative path",
|
||||||
|
input: ".",
|
||||||
|
validate: func(t *testing.T, result string) {
|
||||||
|
if !filepath.IsAbs(result) {
|
||||||
|
t.Errorf("expected absolute path, got %q", result)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "current directory",
|
||||||
|
input: ".",
|
||||||
|
validate: func(t *testing.T, result string) {
|
||||||
|
cwd, err := os.Getwd()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to get cwd: %v", err)
|
||||||
|
}
|
||||||
|
// Result should be canonical form of current directory
|
||||||
|
if !filepath.IsAbs(result) {
|
||||||
|
t.Errorf("expected absolute path, got %q", result)
|
||||||
|
}
|
||||||
|
// The result should be related to cwd (could be same or canonical version)
|
||||||
|
if result != cwd {
|
||||||
|
// Try to canonicalize cwd to compare
|
||||||
|
canonicalCwd, err := filepath.EvalSymlinks(cwd)
|
||||||
|
if err == nil && result != canonicalCwd {
|
||||||
|
t.Errorf("expected %q or %q, got %q", cwd, canonicalCwd, result)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty path",
|
||||||
|
input: "",
|
||||||
|
validate: func(t *testing.T, result string) {
|
||||||
|
// Empty path should be handled (likely becomes "." then current dir)
|
||||||
|
if result == "" {
|
||||||
|
t.Error("expected non-empty result for empty input")
|
||||||
|
}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
result := CanonicalizePath(tt.input)
|
||||||
|
tt.validate(t, result)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCanonicalizePathSymlink(t *testing.T) {
|
||||||
|
// Create a temporary directory
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
|
||||||
|
// Create a test file
|
||||||
|
testFile := filepath.Join(tmpDir, "test.txt")
|
||||||
|
if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil {
|
||||||
|
t.Fatalf("failed to create test file: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create a symlink to the temp directory
|
||||||
|
symlinkPath := filepath.Join(tmpDir, "link")
|
||||||
|
if err := os.Symlink(tmpDir, symlinkPath); err != nil {
|
||||||
|
t.Skipf("failed to create symlink (may not be supported): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Canonicalize the symlink path
|
||||||
|
result := CanonicalizePath(symlinkPath)
|
||||||
|
|
||||||
|
// The result should be the resolved path (tmpDir), not the symlink
|
||||||
|
if result != tmpDir {
|
||||||
|
// Try to get canonical form of tmpDir for comparison
|
||||||
|
canonicalTmpDir, err := filepath.EvalSymlinks(tmpDir)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to canonicalize tmpDir: %v", err)
|
||||||
|
}
|
||||||
|
if result != canonicalTmpDir {
|
||||||
|
t.Errorf("expected %q or %q, got %q", tmpDir, canonicalTmpDir, result)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user