refactor: improve open.go error handling and consolidate editor commands

open.go:
- Add 'terminal' to valid editors in error message
- Use multierror for better multi-instance error aggregation
- Add scanner error check for stdin reading

util.go:
- Extract common runEditorCommand helper for WSL compatibility
- Simplify runVsCodeCommand, runCursorCommand, runWindsurfCommand

Author: theFong

pkg/cmd/open/open.go

@@ -140,7 +140,7 @@ func NewCmdOpen(t *terminal.Terminal, store OpenStore, noLoginStartStore OpenSto
 
 			// Validate editor flag if provided
 			if editor != "" && !isEditorType(editor) {
-				return breverrors.NewValidationError(fmt.Sprintf("invalid editor: %s. Must be 'code', 'cursor', 'windsurf', or 'tmux'", editor))
+				return breverrors.NewValidationError(fmt.Sprintf("invalid editor: %s. Must be 'code', 'cursor', 'windsurf', 'terminal', or 'tmux'", editor))
 			}
 
 			// Get instance names and editor type from args or stdin
@@ -155,7 +155,7 @@ func NewCmdOpen(t *terminal.Terminal, store OpenStore, noLoginStartStore OpenSto
 			}
 
 			// Open each instance
-			var lastErr error
+			var errors error
 			for _, instanceName := range instanceNames {
 				if len(instanceNames) > 1 {
 					fmt.Fprintf(os.Stderr, "Opening %s...\n", instanceName)
@@ -164,14 +164,14 @@ func NewCmdOpen(t *terminal.Terminal, store OpenStore, noLoginStartStore OpenSto
 				if err != nil {
 					if len(instanceNames) > 1 {
 						fmt.Fprintf(os.Stderr, "Error opening %s: %v\n", instanceName, err)
-						lastErr = err
+						errors = multierror.Append(errors, err)
 						continue
 					}
 					return breverrors.WrapAndTrace(err)
 				}
 			}
-			if lastErr != nil {
-				return breverrors.NewValidationError("one or more instances failed to open")
+			if errors != nil {
+				return breverrors.WrapAndTrace(errors)
 			}
 			return nil
 		},
@@ -216,6 +216,9 @@ func getInstanceNamesAndEditor(args []string, editorFlag string) ([]string, stri
 				names = append(names, name)
 			}
 		}
+		if err := scanner.Err(); err != nil {
+			return nil, "", breverrors.WrapAndTrace(err)
+		}
 	}
 
 	if len(names) == 0 {
@@ -710,10 +713,7 @@ end tell`, command)
 	}
 }
 
-func openTerminal(sshAlias string, path string, store OpenStore) error {
-	_ = store // unused parameter required by interface
-	_ = path  // unused, just opens SSH
-
+func openTerminal(sshAlias string, _ string, _ OpenStore) error {
 	sshCmd := fmt.Sprintf("ssh %s", sshAlias)
 	err := openInNewTerminalWindow(sshCmd)
 	if err != nil {

pkg/util/util.go

@@ -59,7 +59,10 @@ func runWindowsExeInWSL(exePath string, args []string) ([]byte, error) {
 
 	cmd := exec.Command("cmd.exe", cmdArgs...) // #nosec G204
 	output, err := cmd.CombinedOutput()
-	return output, breverrors.WrapAndTrace(err)
+	if err != nil {
+		return nil, breverrors.WrapAndTrace(err)
+	}
+	return output, nil
 }
 
 // This package should only be used as a holding pattern to be later moved into more specific packages
@@ -253,40 +256,27 @@ func runManyCursorCommand(cursorpaths []string, args []string) ([]byte, error) {
 	return nil, breverrors.WrapAndTrace(errs.ErrorOrNil())
 }
 
-func runVsCodeCommand(vscodepath string, args []string) ([]byte, error) {
+// runEditorCommand runs an editor executable with the given args, handling WSL compatibility
+func runEditorCommand(path string, args []string) ([]byte, error) {
 	// In WSL, Windows .exe files need to be run through cmd.exe
-	if isWSL() && (strings.HasSuffix(vscodepath, ".exe") || strings.HasPrefix(vscodepath, "/mnt/")) {
-		res, err := runWindowsExeInWSL(vscodepath, args)
-		if err != nil {
-			return nil, breverrors.WrapAndTrace(err)
-		}
-		return res, nil
+	if isWSL() && (strings.HasSuffix(path, ".exe") || strings.HasPrefix(path, "/mnt/")) {
+		return runWindowsExeInWSL(path, args)
 	}
 
-	cmd := exec.Command(vscodepath, args...) // #nosec G204
+	cmd := exec.Command(path, args...) // #nosec G204
 	res, err := cmd.CombinedOutput()
 	if err != nil {
 		return nil, breverrors.WrapAndTrace(err)
 	}
 	return res, nil
 }
 
-func runCursorCommand(cursorpath string, args []string) ([]byte, error) {
-	// In WSL, Windows .exe files need to be run through cmd.exe
-	if isWSL() && (strings.HasSuffix(cursorpath, ".exe") || strings.HasPrefix(cursorpath, "/mnt/")) {
-		res, err := runWindowsExeInWSL(cursorpath, args)
-		if err != nil {
-			return nil, breverrors.WrapAndTrace(err)
-		}
-		return res, nil
-	}
+func runVsCodeCommand(vscodepath string, args []string) ([]byte, error) {
+	return runEditorCommand(vscodepath, args)
+}
 
-	cmd := exec.Command(cursorpath, args...) // #nosec G204
-	res, err := cmd.CombinedOutput()
-	if err != nil {
-		return nil, breverrors.WrapAndTrace(err)
-	}
-	return res, nil
+func runCursorCommand(cursorpath string, args []string) ([]byte, error) {
+	return runEditorCommand(cursorpath, args)
 }
 
 func runManyWindsurfCommand(windsurfpaths []string, args []string) ([]byte, error) {
@@ -303,21 +293,7 @@ func runManyWindsurfCommand(windsurfpaths []string, args []string) ([]byte, erro
 }
 
 func runWindsurfCommand(windsurfpath string, args []string) ([]byte, error) {
-	// In WSL, Windows .exe files need to be run through cmd.exe
-	if isWSL() && (strings.HasSuffix(windsurfpath, ".exe") || strings.HasPrefix(windsurfpath, "/mnt/")) {
-		res, err := runWindowsExeInWSL(windsurfpath, args)
-		if err != nil {
-			return nil, breverrors.WrapAndTrace(err)
-		}
-		return res, nil
-	}
-
-	cmd := exec.Command(windsurfpath, args...) // #nosec G204
-	res, err := cmd.CombinedOutput()
-	if err != nil {
-		return nil, breverrors.WrapAndTrace(err)
-	}
-	return res, nil
+	return runEditorCommand(windsurfpath, args)
 }
 
 var commonVSCodePaths = []string{