fix(tools): fix handling of abs paths on windows

Andrey Nering created 6 days ago

Change summary

internal/agent/tools/download.go  |  9 ++-------
internal/agent/tools/edit.go      |  5 ++---
internal/agent/tools/ls.go        |  5 ++---
internal/agent/tools/multiedit.go |  5 ++---
internal/agent/tools/view.go      |  6 ++----
internal/agent/tools/write.go     |  6 ++----
internal/filepathext/filepath.go  | 27 +++++++++++++++++++++++++++
7 files changed, 39 insertions(+), 24 deletions(-)

Detailed changes

internal/agent/tools/download.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"charm.land/fantasy"
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"github.com/charmbracelet/crush/internal/permission"
 )
 
@@ -59,13 +60,7 @@ func NewDownloadTool(permissions permission.Service, workingDir string, client *
 				return fantasy.NewTextErrorResponse("URL must start with http:// or https://"), nil
 			}
 
-			// Convert relative path to absolute path
-			var filePath string
-			if filepath.IsAbs(params.FilePath) {
-				filePath = params.FilePath
-			} else {
-				filePath = filepath.Join(workingDir, params.FilePath)
-			}
+			filePath := filepathext.SmartJoin(workingDir, params.FilePath)
 
 			sessionID := GetSessionFromContext(ctx)
 			if sessionID == "" {

internal/agent/tools/edit.go

@@ -13,6 +13,7 @@ import (
 	"charm.land/fantasy"
 	"github.com/charmbracelet/crush/internal/csync"
 	"github.com/charmbracelet/crush/internal/diff"
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"github.com/charmbracelet/crush/internal/fsext"
 	"github.com/charmbracelet/crush/internal/history"
 
@@ -61,9 +62,7 @@ func NewEditTool(lspClients *csync.Map[string, *lsp.Client], permissions permiss
 				return fantasy.NewTextErrorResponse("file_path is required"), nil
 			}
 
-			if !filepath.IsAbs(params.FilePath) {
-				params.FilePath = filepath.Join(workingDir, params.FilePath)
-			}
+			params.FilePath = filepathext.SmartJoin(workingDir, params.FilePath)
 
 			var response fantasy.ToolResponse
 			var err error

internal/agent/tools/ls.go

@@ -11,6 +11,7 @@ import (
 
 	"charm.land/fantasy"
 	"github.com/charmbracelet/crush/internal/config"
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"github.com/charmbracelet/crush/internal/fsext"
 	"github.com/charmbracelet/crush/internal/permission"
 )
@@ -57,9 +58,7 @@ func NewLsTool(permissions permission.Service, workingDir string, lsConfig confi
 				return fantasy.NewTextErrorResponse(fmt.Sprintf("error expanding path: %v", err)), nil
 			}
 
-			if !filepath.IsAbs(searchPath) {
-				searchPath = filepath.Join(workingDir, searchPath)
-			}
+			searchPath = filepathext.SmartJoin(workingDir, searchPath)
 
 			// Check if directory is outside working directory and request permission if needed
 			absWorkingDir, err := filepath.Abs(workingDir)

internal/agent/tools/multiedit.go

@@ -13,6 +13,7 @@ import (
 	"charm.land/fantasy"
 	"github.com/charmbracelet/crush/internal/csync"
 	"github.com/charmbracelet/crush/internal/diff"
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"github.com/charmbracelet/crush/internal/fsext"
 	"github.com/charmbracelet/crush/internal/history"
 	"github.com/charmbracelet/crush/internal/lsp"
@@ -62,9 +63,7 @@ func NewMultiEditTool(lspClients *csync.Map[string, *lsp.Client], permissions pe
 				return fantasy.NewTextErrorResponse("at least one edit operation is required"), nil
 			}
 
-			if !filepath.IsAbs(params.FilePath) {
-				params.FilePath = filepath.Join(workingDir, params.FilePath)
-			}
+			params.FilePath = filepathext.SmartJoin(workingDir, params.FilePath)
 
 			// Validate all edits before applying any
 			if err := validateEdits(params.Edits); err != nil {

internal/agent/tools/view.go

@@ -13,6 +13,7 @@ import (
 
 	"charm.land/fantasy"
 	"github.com/charmbracelet/crush/internal/csync"
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"github.com/charmbracelet/crush/internal/lsp"
 	"github.com/charmbracelet/crush/internal/permission"
 )
@@ -60,10 +61,7 @@ func NewViewTool(lspClients *csync.Map[string, *lsp.Client], permissions permiss
 			}
 
 			// Handle relative paths
-			filePath := params.FilePath
-			if !filepath.IsAbs(filePath) {
-				filePath = filepath.Join(workingDir, filePath)
-			}
+			filePath := filepathext.SmartJoin(workingDir, params.FilePath)
 
 			// Check if file is outside working directory and request permission if needed
 			absWorkingDir, err := filepath.Abs(workingDir)

internal/agent/tools/write.go

@@ -13,6 +13,7 @@ import (
 	"charm.land/fantasy"
 	"github.com/charmbracelet/crush/internal/csync"
 	"github.com/charmbracelet/crush/internal/diff"
+	"github.com/charmbracelet/crush/internal/filepathext"
 	"github.com/charmbracelet/crush/internal/fsext"
 	"github.com/charmbracelet/crush/internal/history"
 
@@ -62,10 +63,7 @@ func NewWriteTool(lspClients *csync.Map[string, *lsp.Client], permissions permis
 				return fantasy.NewTextErrorResponse("content is required"), nil
 			}
 
-			filePath := params.FilePath
-			if !filepath.IsAbs(filePath) {
-				filePath = filepath.Join(workingDir, filePath)
-			}
+			filePath := filepathext.SmartJoin(workingDir, params.FilePath)
 
 			fileInfo, err := os.Stat(filePath)
 			if err == nil {

internal/filepathext/filepath.go

@@ -0,0 +1,27 @@
+package filepathext
+
+import (
+	"path/filepath"
+	"runtime"
+	"strings"
+)
+
+// SmartJoin joins two paths, treating the second path as absolute if it is an
+// absolute path.
+func SmartJoin(one, two string) string {
+	if SmartIsAbs(two) {
+		return two
+	}
+	return filepath.Join(one, two)
+}
+
+// SmartIsAbs checks if a path is absolute, considering both OS-specific and
+// Unix-style paths.
+func SmartIsAbs(path string) bool {
+	switch runtime.GOOS {
+	case "windows":
+		return filepath.IsAbs(path) || strings.HasPrefix(filepath.ToSlash(path), "/")
+	default:
+		return filepath.IsAbs(path)
+	}
+}