fix(config): always resolve the data directory to an absolute path (#2883)

Christian Rocha and Charm Crush created 1 month ago

Co-authored-by: Charm Crush <[e-mail]>

Change summary

internal/agent/prompt/prompt.go |  6 +--
internal/agent/tools/glob.go    |  6 +--
internal/config/load.go         |  2 +
internal/config/load_test.go    | 49 +++++++++++++++++++++++++----------
internal/shell/dispatch.go      |  6 +--
5 files changed, 43 insertions(+), 26 deletions(-)

Detailed changes

internal/agent/prompt/prompt.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/charmbracelet/crush/internal/config"
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"github.com/charmbracelet/crush/internal/home"
 	"github.com/charmbracelet/crush/internal/shell"
 	"github.com/charmbracelet/crush/internal/skills"
@@ -107,10 +108,7 @@ func processFile(filePath string) *ContextFile {
 
 func processContextPath(p string, store *config.ConfigStore) []ContextFile {
 	var contexts []ContextFile
-	fullPath := p
-	if !filepath.IsAbs(p) {
-		fullPath = filepath.Join(store.WorkingDir(), p)
-	}
+	fullPath := filepathext.SmartJoin(store.WorkingDir(), p)
 	info, err := os.Stat(fullPath)
 	if err != nil {
 		return contexts

internal/agent/tools/glob.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 
 	"charm.land/fantasy"
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"github.com/charmbracelet/crush/internal/fsext"
 )
 
@@ -96,10 +97,7 @@ func runRipgrep(cmd *exec.Cmd, searchRoot string, limit int) ([]string, error) {
 		if len(p) == 0 {
 			continue
 		}
-		absPath := string(p)
-		if !filepath.IsAbs(absPath) {
-			absPath = filepath.Join(searchRoot, absPath)
-		}
+		absPath := filepathext.SmartJoin(searchRoot, string(p))
 		if fsext.SkipHidden(absPath) {
 			continue
 		}

internal/config/load.go

@@ -21,6 +21,7 @@ import (
 	"github.com/charmbracelet/crush/internal/agent/hyper"
 	"github.com/charmbracelet/crush/internal/csync"
 	"github.com/charmbracelet/crush/internal/env"
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"github.com/charmbracelet/crush/internal/fsext"
 	"github.com/charmbracelet/crush/internal/home"
 	powernapConfig "github.com/charmbracelet/x/powernap/pkg/config"
@@ -429,6 +430,7 @@ func (c *Config) setDefaults(workingDir, dataDir string) {
 			c.Options.DataDirectory = filepath.Join(workingDir, defaultDataDirectory)
 		}
 	}
+	c.Options.DataDirectory = filepath.Clean(filepathext.SmartJoin(workingDir, c.Options.DataDirectory))
 	if c.Providers == nil {
 		c.Providers = csync.NewMap[string, ProviderConfig]()
 	}

internal/config/load_test.go

@@ -74,22 +74,43 @@ func testStore(cfg *Config) *ConfigStore {
 }
 
 func TestConfig_setDefaults(t *testing.T) {
-	cfg := &Config{}
+	t.Run("sets default data directory", func(t *testing.T) {
+		cfg := &Config{}
+		workingDir := t.TempDir()
+
+		cfg.setDefaults(workingDir, "")
+
+		require.NotNil(t, cfg.Options)
+		require.NotNil(t, cfg.Options.TUI)
+		require.NotNil(t, cfg.Options.ContextPaths)
+		require.NotNil(t, cfg.Providers)
+		require.NotNil(t, cfg.Models)
+		require.NotNil(t, cfg.LSP)
+		require.NotNil(t, cfg.MCP)
+		require.Equal(t, filepath.Join(workingDir, ".crush"), cfg.Options.DataDirectory)
+		require.Equal(t, "AGENTS.md", cfg.Options.InitializeAs)
+		for _, path := range defaultContextPaths {
+			require.Contains(t, cfg.Options.ContextPaths, path)
+		}
+	})
 
-	cfg.setDefaults("/tmp", "")
+	t.Run("resolves relative configured data directory from working directory", func(t *testing.T) {
+		cfg := &Config{Options: &Options{DataDirectory: "."}}
+		workingDir := filepath.Join(t.TempDir(), "worktree")
 
-	require.NotNil(t, cfg.Options)
-	require.NotNil(t, cfg.Options.TUI)
-	require.NotNil(t, cfg.Options.ContextPaths)
-	require.NotNil(t, cfg.Providers)
-	require.NotNil(t, cfg.Models)
-	require.NotNil(t, cfg.LSP)
-	require.NotNil(t, cfg.MCP)
-	require.Equal(t, filepath.Join("/tmp", ".crush"), cfg.Options.DataDirectory)
-	require.Equal(t, "AGENTS.md", cfg.Options.InitializeAs)
-	for _, path := range defaultContextPaths {
-		require.Contains(t, cfg.Options.ContextPaths, path)
-	}
+		cfg.setDefaults(workingDir, "")
+
+		require.Equal(t, workingDir, cfg.Options.DataDirectory)
+	})
+
+	t.Run("resolves relative flag data directory from working directory", func(t *testing.T) {
+		cfg := &Config{}
+		workingDir := filepath.Join(t.TempDir(), "worktree")
+
+		cfg.setDefaults(workingDir, "./state")
+
+		require.Equal(t, filepath.Join(workingDir, "state"), cfg.Options.DataDirectory)
+	})
 }
 
 func TestConfig_configureProviders(t *testing.T) {

internal/shell/dispatch.go

@@ -14,6 +14,7 @@ import (
 	"runtime"
 	"strings"
 
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"mvdan.cc/sh/v3/expand"
 	"mvdan.cc/sh/v3/interp"
 	"mvdan.cc/sh/v3/syntax"
@@ -51,13 +52,10 @@ func scriptDispatchHandler(blockFuncs []BlockFunc) func(next interp.ExecHandlerF
 				return next(ctx, args)
 			}
 
-			scriptPath := args[0]
 			// Resolve relative paths against the interpreter's cwd, not
 			// the process cwd — hook commands are authored with the hook
 			// Runner's cwd in mind and sub-shells can cd before an exec.
-			if !filepath.IsAbs(scriptPath) {
-				scriptPath = filepath.Join(interp.HandlerCtx(ctx).Dir, scriptPath)
-			}
+			scriptPath := filepathext.SmartJoin(interp.HandlerCtx(ctx).Dir, args[0])
 			probe, err := probeFile(scriptPath)
 			if err != nil {
 				return err