Fix move & copy for prefix filesystems

frontend/app/view/preview/directorypreview.tsx

@@ -48,7 +48,7 @@ type FileCopyStatus = {
 
 declare module "@tanstack/react-table" {
     interface TableMeta<TData extends RowData> {
-        updateName: (path: string) => void;
+        updateName: (path: string, isDir: boolean) => void;
         newFile: () => void;
         newDirectory: () => void;
     }
@@ -291,7 +291,7 @@ function DirectoryTable({
 
     const setEntryManagerProps = useSetAtom(entryManagerOverlayPropsAtom);
 
-    const updateName = useCallback((path: string) => {
+    const updateName = useCallback((path: string, isDir: boolean) => {
         const fileName = path.split("/").at(-1);
         setEntryManagerProps({
             entryManagerType: EntryManagerType.EditName,
@@ -304,8 +304,12 @@ function DirectoryTable({
                     console.log(`replacing ${fileName} with ${newName}: ${path}`);
                     fireAndForget(async () => {
                         try {
+                            let srcuri = await model.formatRemoteUri(path, globalStore.get);
+                            if (isDir) {
+                                srcuri += "/";
+                            }
                             await RpcApi.FileMoveCommand(TabRpcClient, {
-                                srcuri: await model.formatRemoteUri(path, globalStore.get),
+                                srcuri,
                                 desturi: await model.formatRemoteUri(newPath, globalStore.get),
                                 opts: {
                                     recursive: true,
@@ -547,7 +551,7 @@ function TableBody({
                 {
                     label: "Rename",
                     click: () => {
-                        table.options.meta.updateName(finfo.path);
+                        table.options.meta.updateName(finfo.path, finfo.isdir);
                     },
                 },
                 {
@@ -854,17 +858,18 @@ function DirectoryPreview({ model }: DirectoryPreviewProps) {
     });
 
     const handleDropCopy = useCallback(
-        async (data: CommandFileCopyData, isDir) => {
+        async (data: CommandFileCopyData, isDir: boolean) => {
             try {
                 await RpcApi.FileCopyCommand(TabRpcClient, data, { timeout: data.opts.timeout });
                 setCopyStatus(null);
             } catch (e) {
                 console.log("copy failed:", e);
                 const copyError = `${e}`;
                 const allowRetry =
-                    copyError.includes("overwrite not specified") ||
-                    copyError.includes("neither overwrite nor merge specified") ||
-                    copyError.includes("neither merge nor overwrite specified");
+                    copyError.includes("set overwrite flag to delete the existing file") ||
+                    copyError.includes(
+                        "set overwrite flag to delete the existing contents or set merge flag to merge the contents"
+                    );
                 const copyStatus: FileCopyStatus = {
                     copyError,
                     copyData: data,

go.mod

@@ -34,6 +34,7 @@ require (
 	github.com/wavetermdev/htmltoken v0.2.0
 	golang.org/x/crypto v0.33.0
 	golang.org/x/mod v0.23.0
+	golang.org/x/sync v0.11.0
 	golang.org/x/sys v0.30.0
 	golang.org/x/term v0.29.0
 	google.golang.org/api v0.221.0
@@ -95,7 +96,6 @@ require (
 	go.uber.org/atomic v1.7.0 // indirect
 	golang.org/x/net v0.35.0 // indirect
 	golang.org/x/oauth2 v0.26.0 // indirect
-	golang.org/x/sync v0.11.0 // indirect
 	golang.org/x/text v0.22.0 // indirect
 	golang.org/x/time v0.10.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 // indirect

pkg/remote/fileshare/fileshare.go

@@ -129,18 +129,11 @@ func Move(ctx context.Context, data wshrpc.CommandFileCopyData) error {
 		return fmt.Errorf("error creating fileshare client, could not parse destination connection %s", data.DestUri)
 	}
 	if srcConn.Host != destConn.Host {
-		finfo, err := srcClient.Stat(ctx, srcConn)
-		if err != nil {
-			return fmt.Errorf("cannot stat %q: %w", data.SrcUri, err)
-		}
-		recursive := data.Opts != nil && data.Opts.Recursive
-		if finfo.IsDir && data.Opts != nil && !recursive {
-			return fmt.Errorf("cannot move directory %q to %q without recursive flag", data.SrcUri, data.DestUri)
-		}
-		err = destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts)
+		err := destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts)
 		if err != nil {
 			return fmt.Errorf("cannot copy %q to %q: %w", data.SrcUri, data.DestUri, err)
 		}
+		recursive := data.Opts != nil && data.Opts.Recursive
 		return srcClient.Delete(ctx, srcConn, recursive)
 	} else {
 		return srcClient.MoveInternal(ctx, srcConn, destConn, data.Opts)
@@ -149,6 +142,11 @@ func Move(ctx context.Context, data wshrpc.CommandFileCopyData) error {
 
 func Copy(ctx context.Context, data wshrpc.CommandFileCopyData) error {
 	log.Printf("Copy: %v", data)
+	opts := data.Opts
+	if opts == nil {
+		opts = &wshrpc.FileCopyOpts{}
+	}
+	opts.Recursive = true
 	srcClient, srcConn := CreateFileShareClient(ctx, data.SrcUri)
 	if srcConn == nil || srcClient == nil {
 		return fmt.Errorf("error creating fileshare client, could not parse source connection %s", data.SrcUri)
@@ -158,9 +156,9 @@ func Copy(ctx context.Context, data wshrpc.CommandFileCopyData) error {
 		return fmt.Errorf("error creating fileshare client, could not parse destination connection %s", data.DestUri)
 	}
 	if srcConn.Host != destConn.Host {
-		return destClient.CopyRemote(ctx, srcConn, destConn, srcClient, data.Opts)
+		return destClient.CopyRemote(ctx, srcConn, destConn, srcClient, opts)
 	} else {
-		return srcClient.CopyInternal(ctx, srcConn, destConn, data.Opts)
+		return srcClient.CopyInternal(ctx, srcConn, destConn, opts)
 	}
 }
 

pkg/remote/fileshare/fstype/fstype.go

@@ -14,9 +14,12 @@ import (
 )
 
 const (
-	DefaultTimeout             = 30 * time.Second
-	FileMode       os.FileMode = 0644
-	DirMode        os.FileMode = 0755 | os.ModeDir
+	DefaultTimeout                 = 30 * time.Second
+	FileMode           os.FileMode = 0644
+	DirMode            os.FileMode = 0755 | os.ModeDir
+	RecursiveCopyError             = "recursive flag must be set for directory operations"
+	MergeCopyError                 = "directory already exists at %q, set overwrite flag to delete the existing contents or set merge flag to merge the contents"
+	OverwriteCopyError             = "file already exists at %q, set overwrite flag to delete the existing file"
 )
 
 type FileShareClient interface {
@@ -40,9 +43,9 @@ type FileShareClient interface {
 	Mkdir(ctx context.Context, conn *connparse.Connection) error
 	// Move moves the file within the same connection
 	MoveInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error
-	// Copy copies the file within the same connection
+	// Copy copies the file within the same connection. Returns whether the copy source was a directory
 	CopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, opts *wshrpc.FileCopyOpts) error
-	// CopyRemote copies the file between different connections
+	// CopyRemote copies the file between different connections. Returns whether the copy source was a directory
 	CopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient FileShareClient, opts *wshrpc.FileCopyOpts) error
 	// Delete deletes the entry at the given path
 	Delete(ctx context.Context, conn *connparse.Connection, recursive bool) error

pkg/remote/fileshare/fsutil/fsutil.go

@@ -28,7 +28,7 @@ func GetParentPath(conn *connparse.Connection) string {
 
 func GetParentPathString(hostAndPath string) string {
 	if hostAndPath == "" || hostAndPath == fspath.Separator {
-		return fspath.Separator
+		return ""
 	}
 
 	// Remove trailing slash if present
@@ -38,75 +38,23 @@ func GetParentPathString(hostAndPath string) string {
 
 	lastSlash := strings.LastIndex(hostAndPath, fspath.Separator)
 	if lastSlash <= 0 {
-		return fspath.Separator
-	}
-	return hostAndPath[:lastSlash+1]
-}
-
-const minURILength = 10 // Minimum length for a valid URI (e.g., "s3://bucket")
-
-func GetPathPrefix(conn *connparse.Connection) string {
-	fullUri := conn.GetFullURI()
-	if fullUri == "" {
 		return ""
 	}
-	pathPrefix := fullUri
-	lastSlash := strings.LastIndex(fullUri, fspath.Separator)
-	if lastSlash > minURILength && lastSlash < len(fullUri)-1 {
-		pathPrefix = fullUri[:lastSlash+1]
-	}
-	return pathPrefix
+	return hostAndPath[:lastSlash+1]
 }
 
 func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connection, c fstype.FileShareClient, opts *wshrpc.FileCopyOpts, listEntriesPrefix func(ctx context.Context, host string, path string) ([]string, error), copyFunc func(ctx context.Context, host string, path string) error) error {
 	log.Printf("PrefixCopyInternal: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI())
-	merge := opts != nil && opts.Merge
-	overwrite := opts != nil && opts.Overwrite
-	if overwrite && merge {
-		return fmt.Errorf("cannot specify both overwrite and merge")
-	}
 	srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator)
-	srcPath, err := CleanPathPrefix(srcConn.Path)
-	if err != nil {
-		return fmt.Errorf("error cleaning source path: %w", err)
-	}
-	destHasSlash := strings.HasSuffix(destConn.Path, fspath.Separator)
-	destPath, err := CleanPathPrefix(destConn.Path)
-	if err != nil {
-		return fmt.Errorf("error cleaning destination path: %w", err)
-	}
-	if !srcHasSlash {
-		if !destHasSlash {
-			destPath += fspath.Separator
-		}
-		destPath += fspath.Base(srcPath)
-	}
-	destConn.Path = destPath
-	destInfo, err := c.Stat(ctx, destConn)
-	destExists := err == nil && !destInfo.NotFound
-	if err != nil && !errors.Is(err, fs.ErrNotExist) {
-		return fmt.Errorf("error getting destination file info: %w", err)
-	}
-
-	srcInfo, err := c.Stat(ctx, srcConn)
+	srcPath, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, c, c, opts)
 	if err != nil {
-		return fmt.Errorf("error getting source file info: %w", err)
-	}
-	if destExists {
-		if overwrite {
-			err = c.Delete(ctx, destConn, true)
-			if err != nil {
-				return fmt.Errorf("error deleting conflicting destination file: %w", err)
-			}
-		} else if destInfo.IsDir && srcInfo.IsDir {
-			if !merge {
-				return fmt.Errorf("destination and source are both directories, neither merge nor overwrite specified: %v", destConn.GetFullURI())
-			}
-		} else {
-			return fmt.Errorf("destination already exists, overwrite not specified: %v", destConn.GetFullURI())
-		}
+		return err
 	}
+	recursive := opts != nil && opts.Recursive
 	if srcInfo.IsDir {
+		if !recursive {
+			return fmt.Errorf(fstype.RecursiveCopyError)
+		}
 		if !srcHasSlash {
 			srcPath += fspath.Separator
 		}
@@ -143,53 +91,18 @@ func PrefixCopyInternal(ctx context.Context, srcConn, destConn *connparse.Connec
 }
 
 func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, destPutFile func(host string, path string, size int64, reader io.Reader) error, opts *wshrpc.FileCopyOpts) error {
-	merge := opts != nil && opts.Merge
-	overwrite := opts != nil && opts.Overwrite
-	if overwrite && merge {
-		return fmt.Errorf("cannot specify both overwrite and merge")
-	}
-	srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator)
-	destHasSlash := strings.HasSuffix(destConn.Path, fspath.Separator)
-	destPath, err := CleanPathPrefix(destConn.Path)
+	// prefix to be used if the destination is a directory. The destPath returned in the following call only applies if the destination is not a directory.
+	destPathPrefix, err := CleanPathPrefix(destConn.Path)
 	if err != nil {
 		return fmt.Errorf("error cleaning destination path: %w", err)
 	}
-	if !srcHasSlash {
-		if !destHasSlash {
-			destPath += fspath.Separator
-		}
-		destPath += fspath.Base(srcConn.Path)
-	}
-	destConn.Path = destPath
-	destInfo, err := destClient.Stat(ctx, destConn)
-	destExists := err == nil && !destInfo.NotFound
-	if err != nil && !errors.Is(err, fs.ErrNotExist) {
-		return fmt.Errorf("error getting destination file info: %w", err)
-	}
+	destPathPrefix += fspath.Separator
 
-	srcInfo, err := srcClient.Stat(ctx, srcConn)
-	if err != nil {
-		return fmt.Errorf("error getting source file info: %w", err)
-	}
-	if destExists {
-		if overwrite {
-			err = destClient.Delete(ctx, destConn, true)
-			if err != nil {
-				return fmt.Errorf("error deleting conflicting destination file: %w", err)
-			}
-		} else if destInfo.IsDir && srcInfo.IsDir {
-			if !merge {
-				return fmt.Errorf("destination and source are both directories, neither merge nor overwrite specified: %v", destConn.GetFullURI())
-			}
-		} else {
-			return fmt.Errorf("destination already exists, overwrite not specified: %v", destConn.GetFullURI())
-		}
-	}
+	_, destPath, srcInfo, err := DetermineCopyDestPath(ctx, srcConn, destConn, srcClient, destClient, opts)
 	if err != nil {
-		if !errors.Is(err, fs.ErrNotExist) {
-			return err
-		}
+		return err
 	}
+
 	log.Printf("Copying: %v -> %v", srcConn.GetFullURI(), destConn.GetFullURI())
 	readCtx, cancel := context.WithCancelCause(ctx)
 	defer cancel(nil)
@@ -201,9 +114,9 @@ func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connecti
 		if singleFile && srcInfo.IsDir {
 			return fmt.Errorf("protocol error: source is a directory, but only a single file is being copied")
 		}
-		fileName, err := CleanPathPrefix(fspath.Join(destPath, next.Name))
-		if singleFile && !destHasSlash {
-			fileName, err = CleanPathPrefix(destConn.Path)
+		fileName, err := CleanPathPrefix(fspath.Join(destPathPrefix, next.Name))
+		if singleFile {
+			fileName = destPath
 		}
 		if err != nil {
 			return fmt.Errorf("error cleaning path: %w", err)
@@ -218,10 +131,63 @@ func PrefixCopyRemote(ctx context.Context, srcConn, destConn *connparse.Connecti
 	return nil
 }
 
+func DetermineCopyDestPath(ctx context.Context, srcConn, destConn *connparse.Connection, srcClient, destClient fstype.FileShareClient, opts *wshrpc.FileCopyOpts) (srcPath, destPath string, srcInfo *wshrpc.FileInfo, err error) {
+	merge := opts != nil && opts.Merge
+	overwrite := opts != nil && opts.Overwrite
+	if overwrite && merge {
+		return "", "", nil, fmt.Errorf("cannot specify both overwrite and merge")
+	}
+
+	srcHasSlash := strings.HasSuffix(srcConn.Path, fspath.Separator)
+	srcPath = srcConn.Path
+	destHasSlash := strings.HasSuffix(destConn.Path, fspath.Separator)
+	destPath, err = CleanPathPrefix(destConn.Path)
+	if err != nil {
+		return "", "", nil, fmt.Errorf("error cleaning destination path: %w", err)
+	}
+
+	srcInfo, err = srcClient.Stat(ctx, srcConn)
+	if err != nil {
+		return "", "", nil, fmt.Errorf("error getting source file info: %w", err)
+	}
+	destInfo, err := destClient.Stat(ctx, destConn)
+	destExists := err == nil && !destInfo.NotFound
+	if err != nil && !errors.Is(err, fs.ErrNotExist) {
+		return "", "", nil, fmt.Errorf("error getting destination file info: %w", err)
+	}
+	originalDestPath := destPath
+	if !srcHasSlash {
+		if destInfo.IsDir || (!destExists && !destHasSlash && srcInfo.IsDir) {
+			destPath = fspath.Join(destPath, fspath.Base(srcConn.Path))
+		}
+	}
+	destConn.Path = destPath
+	if originalDestPath != destPath {
+		destInfo, err = destClient.Stat(ctx, destConn)
+		destExists = err == nil && !destInfo.NotFound
+		if err != nil && !errors.Is(err, fs.ErrNotExist) {
+			return "", "", nil, fmt.Errorf("error getting destination file info: %w", err)
+		}
+	}
+	if destExists {
+		if overwrite {
+			err = destClient.Delete(ctx, destConn, destInfo.IsDir)
+			if err != nil {
+				return "", "", nil, fmt.Errorf("error deleting conflicting destination file: %w", err)
+			}
+		} else if destInfo.IsDir && srcInfo.IsDir && !merge {
+			return "", "", nil, fmt.Errorf(fstype.MergeCopyError, destConn.GetFullURI())
+		} else {
+			return "", "", nil, fmt.Errorf(fstype.OverwriteCopyError, destConn.GetFullURI())
+		}
+	}
+	return srcPath, destPath, srcInfo, nil
+}
+
 // CleanPathPrefix corrects paths for prefix filesystems (i.e. ones that don't have directories)
 func CleanPathPrefix(path string) (string, error) {
 	if path == "" {
-		return "", fmt.Errorf("path is empty")
+		return "", nil
 	}
 	if strings.HasPrefix(path, fspath.Separator) {
 		path = path[1:]