add port forward and all work

Author: patelspratik

pkg/cmd/copy/copy_test.go

@@ -0,0 +1,90 @@
+package copy
+
+import (
+	"testing"
+)
+
+func TestParseCopyArguments_Upload(t *testing.T) {
+	ws, remotePath, localPath, isUpload, err := parseCopyArguments("./local.txt", "my-node:/tmp/dest")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ws != "my-node" {
+		t.Errorf("expected workspace my-node, got %s", ws)
+	}
+	if remotePath != "/tmp/dest" {
+		t.Errorf("expected remotePath /tmp/dest, got %s", remotePath)
+	}
+	if localPath != "./local.txt" {
+		t.Errorf("expected localPath ./local.txt, got %s", localPath)
+	}
+	if !isUpload {
+		t.Error("expected isUpload=true")
+	}
+}
+
+func TestParseCopyArguments_Download(t *testing.T) {
+	ws, remotePath, localPath, isUpload, err := parseCopyArguments("my-node:/tmp/file", "./local.txt")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ws != "my-node" {
+		t.Errorf("expected workspace my-node, got %s", ws)
+	}
+	if remotePath != "/tmp/file" {
+		t.Errorf("expected remotePath /tmp/file, got %s", remotePath)
+	}
+	if localPath != "./local.txt" {
+		t.Errorf("expected localPath ./local.txt, got %s", localPath)
+	}
+	if isUpload {
+		t.Error("expected isUpload=false")
+	}
+}
+
+func TestParseCopyArguments_BothLocal(t *testing.T) {
+	_, _, _, _, err := parseCopyArguments("./a", "./b")
+	if err == nil {
+		t.Fatal("expected error when both paths are local")
+	}
+}
+
+func TestParseCopyArguments_BothRemote(t *testing.T) {
+	_, _, _, _, err := parseCopyArguments("ws1:/a", "ws2:/b")
+	if err == nil {
+		t.Fatal("expected error when both paths are remote")
+	}
+}
+
+func TestParseWorkspacePath_Local(t *testing.T) {
+	ws, fp, err := parseWorkspacePath("/tmp/local/file")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ws != "" {
+		t.Errorf("expected empty workspace, got %s", ws)
+	}
+	if fp != "/tmp/local/file" {
+		t.Errorf("expected /tmp/local/file, got %s", fp)
+	}
+}
+
+func TestParseWorkspacePath_Remote(t *testing.T) {
+	ws, fp, err := parseWorkspacePath("my-instance:/remote/path")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ws != "my-instance" {
+		t.Errorf("expected my-instance, got %s", ws)
+	}
+	if fp != "/remote/path" {
+		t.Errorf("expected /remote/path, got %s", fp)
+	}
+}
+
+func TestParseWorkspacePath_InvalidMultipleColons(t *testing.T) {
+	_, _, err := parseWorkspacePath("ws:path:extra")
+	if err == nil {
+		t.Fatal("expected error for multiple colons")
+	}
+}

pkg/cmd/open/open_test.go

@@ -1 +1,43 @@
 package open
+
+import (
+	"testing"
+)
+
+func TestIsEditorType(t *testing.T) {
+	valid := []string{"code", "cursor", "windsurf", "terminal", "tmux"}
+	for _, v := range valid {
+		if !isEditorType(v) {
+			t.Errorf("expected %q to be valid editor type", v)
+		}
+	}
+
+	invalid := []string{"vim", "emacs", "vscode", "Code", "", "ssh"}
+	for _, v := range invalid {
+		if isEditorType(v) {
+			t.Errorf("expected %q to NOT be valid editor type", v)
+		}
+	}
+}
+
+func TestGetEditorName(t *testing.T) {
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{"code", "VSCode"},
+		{"cursor", "Cursor"},
+		{"windsurf", "Windsurf"},
+		{"terminal", "Terminal"},
+		{"tmux", "tmux"},
+		{"unknown", "VSCode"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			got := getEditorName(tt.input)
+			if got != tt.want {
+				t.Errorf("getEditorName(%q) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}

pkg/cmd/portforward/portforward.go

@@ -6,8 +6,11 @@ import (
 	"os/exec"
 	"os/signal"
 	"path/filepath"
+	"strconv"
 	"strings"
 
+	nodev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1"
+
 	"github.com/brevdev/brev-cli/pkg/cmd/cmderrors"
 	"github.com/brevdev/brev-cli/pkg/cmd/completions"
 	"github.com/brevdev/brev-cli/pkg/cmd/refresh"
@@ -20,7 +23,7 @@ import (
 
 var (
 	sshLinkLong    = "Port forward your Brev machine's port to your local port"
-	sshLinkExample = "brev port-forward <ws_name> -p local_port:remote_port"
+	sshLinkExample = "brev port-forward <name> -p local_port:remote_port"
 )
 
 type PortforwardStore interface {
@@ -53,7 +56,7 @@ func NewCmdPortForwardSSH(pfStore PortforwardStore, t *terminal.Terminal) *cobra
 			return nil
 		},
 	}
-	cmd.Flags().StringVarP(&port, "port", "p", "", "port forward flag describe me better")
+	cmd.Flags().StringVarP(&port, "port", "p", "", "port forward string, local_port:remote_port")
 	cmd.Flags().BoolVar(&useHost, "host", false, "Use the -host version of the instance")
 	err := cmd.RegisterFlagCompletionFunc("port", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		return nil, cobra.ShellCompDirectiveNoSpace
@@ -66,30 +69,47 @@ func NewCmdPortForwardSSH(pfStore PortforwardStore, t *terminal.Terminal) *cobra
 	return cmd
 }
 
+// parsePortString validates and splits a "local:remote" port string.
+func parsePortString(portString string) (localPort, remotePort string, err error) {
+	if !strings.Contains(portString, ":") {
+		return "", "", breverrors.NewValidationError("port format invalid, use local_port:remote_port")
+	}
+	parts := strings.Split(portString, ":")
+	if len(parts) != 2 {
+		return "", "", breverrors.NewValidationError("port format invalid, use local_port:remote_port")
+	}
+	return parts[0], parts[1], nil
+}
+
+// isPortAlreadyAllocatedError returns true if the error indicates the port is already open.
+func isPortAlreadyAllocatedError(err error) bool {
+	return err != nil && strings.Contains(err.Error(), "already allocated")
+}
+
 func RunPortforward(pfStore PortforwardStore, nameOrID string, portString string, useHost bool) error {
-	var portSplit []string
-	if strings.Contains(portString, ":") {
-		portSplit = strings.Split(portString, ":")
-		if len(portSplit) != 2 {
-			return breverrors.NewValidationError("port format invalid, use local_port:remote_port")
-		}
-	} else {
-		return breverrors.NewValidationError("port format invalid, use local_port:remote_port")
+	localPort, remotePort, err := parsePortString(portString)
+	if err != nil {
+		return err
 	}
 
 	res := refresh.RunRefreshAsync(pfStore)
 
 	sshName, err := ConvertNametoSSHName(pfStore, nameOrID, useHost)
 	if err != nil {
-		return breverrors.WrapAndTrace(err)
+		// Workspace not found — try external nodes.
+		node, nodeErr := util.FindExternalNode(pfStore, nameOrID)
+		if nodeErr != nil || node == nil {
+			return breverrors.WrapAndTrace(err) // return original workspace error
+		}
+		return portForwardExternalNode(pfStore, res, node, localPort, remotePort)
 	}
 
 	err = res.Await()
 	if err != nil {
 		return breverrors.WrapAndTrace(err)
 	}
 
-	_, err = RunSSHPortForward("-L", portSplit[0], portSplit[1], sshName)
+	_, err = RunSSHPortForward("-L", localPort, remotePort, sshName)
 	if err != nil {
 		return breverrors.WrapAndTrace(err)
 	}
@@ -108,6 +128,49 @@ func ConvertNametoSSHName(store PortforwardStore, workspaceNameOrID string, useH
 	return sshName, nil
 }
 
+func portForwardExternalNode(pfStore PortforwardStore, res *refresh.RefreshRes, node *nodev1.ExternalNode, localPort, remotePort string) error {
+	info, err := util.ResolveExternalNodeSSH(pfStore, node)
+	if err != nil {
+		return breverrors.WrapAndTrace(err)
+	}
+
+	// Parse the remote port so we can open it via the OpenPort RPC.
+	remotePortNum, err := strconv.ParseInt(remotePort, 10, 32)
+	if err != nil {
+		return breverrors.WrapAndTrace(fmt.Errorf("invalid remote port %q: %w", remotePort, err))
+	}
+
+	// Open the port on the netbird side so it's accessible.
+	// This binding persists after the CLI exits — it won't be closed on Ctrl+C.
+	fmt.Printf("Opening port %s on node %q...\n", remotePort, node.GetName())
+	_, err = util.OpenPort(pfStore, node.GetExternalNodeId(), int32(remotePortNum), nodev1.PortProtocol_PORT_PROTOCOL_TCP)
+	if err != nil {
+		// Port already allocated is not a real error — it's already open.
+		if isPortAlreadyAllocatedError(err) {
+			fmt.Printf("Port %s is already open on the remote node.\n", remotePort)
+		} else {
+			return breverrors.WrapAndTrace(err)
+		}
+	} else {
+		fmt.Printf("Port %s is now bound on the remote node. Note: this binding persists after this command exits.\n", remotePort)
+	}
+
+	if err := res.Await(); err != nil {
+		return breverrors.WrapAndTrace(err)
+	}
+
+	// The SSH tunnel forwards local traffic through the SSH connection to the
+	// actual port on the box. Ctrl+C stops the local forward but not the
+	// remote port binding.
+	alias := info.SSHAlias()
+	fmt.Printf("Setting up local forward: localhost:%s -> %s:%s\n", localPort, alias, remotePort)
+	_, err = RunSSHPortForward("-L", localPort, remotePort, alias)
+	if err != nil {
+		return breverrors.WrapAndTrace(err)
+	}
+	return nil
+}
+
 func RunSSHPortForward(forwardType string, localPort string, remotePort string, sshName string) (*os.Process, error) {
 	signals := make(chan os.Signal, 1)
 	signal.Notify(signals, os.Interrupt)

pkg/cmd/portforward/portforward_test.go

@@ -1 +1,70 @@
 package portforward
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestParsePortString_Valid(t *testing.T) {
+	local, remote, err := parsePortString("8080:3000")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if local != "8080" {
+		t.Errorf("expected local 8080, got %s", local)
+	}
+	if remote != "3000" {
+		t.Errorf("expected remote 3000, got %s", remote)
+	}
+}
+
+func TestParsePortString_SamePort(t *testing.T) {
+	local, remote, err := parsePortString("8080:8080")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if local != "8080" || remote != "8080" {
+		t.Errorf("expected 8080:8080, got %s:%s", local, remote)
+	}
+}
+
+func TestParsePortString_NoColon(t *testing.T) {
+	_, _, err := parsePortString("8080")
+	if err == nil {
+		t.Fatal("expected error for missing colon")
+	}
+}
+
+func TestParsePortString_TooManyColons(t *testing.T) {
+	_, _, err := parsePortString("8080:3000:443")
+	if err == nil {
+		t.Fatal("expected error for too many colons")
+	}
+}
+
+func TestParsePortString_Empty(t *testing.T) {
+	_, _, err := parsePortString("")
+	if err == nil {
+		t.Fatal("expected error for empty string")
+	}
+}
+
+func TestIsPortAlreadyAllocatedError_True(t *testing.T) {
+	err := fmt.Errorf("skybridge API error: 400, body: Port 8080 is already allocated for this client")
+	if !isPortAlreadyAllocatedError(err) {
+		t.Error("expected true for 'already allocated' error")
+	}
+}
+
+func TestIsPortAlreadyAllocatedError_False(t *testing.T) {
+	err := fmt.Errorf("connection refused")
+	if isPortAlreadyAllocatedError(err) {
+		t.Error("expected false for unrelated error")
+	}
+}
+
+func TestIsPortAlreadyAllocatedError_Nil(t *testing.T) {
+	if isPortAlreadyAllocatedError(nil) {
+		t.Error("expected false for nil error")
+	}
+}

pkg/cmd/refresh/refresh.go

@@ -216,7 +216,7 @@ func getExternalNodeSSHEntries(store RefreshStore) []ssh.ExternalNodeSSHEntry {
 		entries = append(entries, ssh.ExternalNodeSSHEntry{
 			Alias:    ssh.SanitizeNodeName(node.GetName()),
 			Hostname: sshPort.GetHostname(),
-			Port:     sshPort.GetPortNumber(),
+			Port:     sshPort.GetServerPort(),
 			User:     linuxUser,
 		})
 	}