fix(exec): treat remote non-zero exit as command failure, not connection failure

main.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	stderrors "errors"
 	"os"
 
 	"github.com/brevdev/brev-cli/pkg/analytics"
@@ -16,6 +17,12 @@ func main() {
 	command := cmd.NewDefaultBrevCommand()
 
 	if err := command.Execute(); err != nil {
+		// Not a CLI error: pass the remote command's exit code straight through.
+		var remoteErr errors.RemoteExitError
+		if stderrors.As(err, &remoteErr) {
+			done()
+			os.Exit(remoteErr.Code) //nolint:gocritic // manually call done
+		}
 		analytics.CaptureCommandError()
 		cmderrors.DisplayAndHandleError(err)
 		done()

pkg/cmd/exec/exec.go

@@ -2,6 +2,7 @@ package exec
 
 import (
 	"bufio"
+	stderrors "errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -52,6 +53,7 @@ type ExecStore interface {
 	refresh.RefreshStore
 	GetOrganizations(options *store.GetOrganizationsOptions) ([]entity.Organization, error)
 	GetWorkspaces(organizationID string, options *store.GetWorkspacesOptions) ([]entity.Workspace, error)
+	GetAuthTokens() (*entity.AuthTokens, error)
 }
 
 func NewCmdExec(t *terminal.Terminal, store ExecStore, noLoginStartStore ExecStore) *cobra.Command {
@@ -86,6 +88,11 @@ func NewCmdExec(t *terminal.Terminal, store ExecStore, noLoginStartStore ExecSto
 				return breverrors.NewValidationError("command is required")
 			}
 
+			// Heads-up only: exec can still succeed without credentials if the SSH config is warm.
+			if hasNoSavedCredentials(store) {
+				fmt.Fprintf(os.Stderr, "No saved Brev credentials. Trying with your existing SSH config; you'll be prompted to log in if it fails.\n")
+			}
+
 			// Run on each instance
 			var errors error
 			for _, instanceName := range instanceNames {
@@ -107,7 +114,7 @@ func NewCmdExec(t *terminal.Terminal, store ExecStore, noLoginStartStore ExecSto
 				}
 			}
 			if errors != nil {
-				return breverrors.WrapAndTrace(errors)
+				return breverrors.WrapAndTrace(flattenMultiInstanceErr(errors))
 			}
 			return nil
 		},
@@ -172,8 +179,45 @@ func parseCommand(command string) (string, error) {
 	return command, nil
 }
 
+// flattenMultiInstanceErr drops error types so one instance's exit code can't become the process's.
+func flattenMultiInstanceErr(err error) error {
+	if err == nil {
+		return nil
+	}
+	return stderrors.New(err.Error())
+}
+
+type authTokenGetter interface {
+	GetAuthTokens() (*entity.AuthTokens, error)
+}
+
+// hasNoSavedCredentials reports whether credentials are missing or empty; it does not validate them.
+func hasNoSavedCredentials(sstore authTokenGetter) bool {
+	tokens, err := sstore.GetAuthTokens()
+	if err != nil {
+		var notFound *breverrors.CredentialsFileNotFound
+		return stderrors.As(err, &notFound)
+	}
+	if tokens == nil {
+		return true
+	}
+	return tokens.AccessToken == "" && tokens.RefreshToken == "" && strings.TrimSpace(tokens.APIKey) == ""
+}
+
 const pollTimeout = 10 * time.Minute
 
+// sshConnectionFailedExitCode is ssh's own failure code; any other code is the remote command's.
+const sshConnectionFailedExitCode = 255
+
+// exitCodeOf returns the process exit code for err, or -1 if err is not an exit error.
+func exitCodeOf(err error) int {
+	var exitErr *exec.ExitError
+	if stderrors.As(err, &exitErr) {
+		return exitErr.ExitCode()
+	}
+	return -1
+}
+
 func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID string, host bool, command string) error {
 	// Determine SSH alias: use the workspace name directly (with -host suffix if needed)
 	sshName := workspaceNameOrID
@@ -192,6 +236,12 @@ func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID st
 		return nil
 	}
 
+	// The connection worked and the command itself failed, so skip the recovery path.
+	var remoteErr breverrors.RemoteExitError
+	if stderrors.As(err, &remoteErr) {
+		return remoteErr
+	}
+
 	// SSH failed — now check what's going on with the instance
 	fmt.Fprintf(os.Stderr, "Connection failed, checking instance status...\n")
 
@@ -228,12 +278,7 @@ func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID st
 		if err != nil {
 			return breverrors.WrapAndTrace(err)
 		}
-		err = runSSH(sshName, command)
-		if err != nil {
-			return breverrors.WrapAndTrace(err)
-		}
-		go trackExecAnalytics(sstore, workspaceNameOrID)
-		return nil
+		return runAndTrack(sstore, sshName, workspaceNameOrID, command)
 	}
 
 	if workspace.Status != "RUNNING" {
@@ -261,12 +306,22 @@ func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID st
 			"could not connect to instance %q: %w\nPlease check with: brev ls",
 			workspaceNameOrID, err))
 	}
-	err = runSSH(sshName, command)
-	if err != nil {
-		return breverrors.WrapAndTrace(err)
+	return runAndTrack(sstore, sshName, workspaceNameOrID, command)
+}
+
+// runAndTrack runs the command after recovery.
+func runAndTrack(sstore ExecStore, sshName string, workspaceNameOrID string, command string) error {
+	err := runSSH(sshName, command)
+	if err == nil {
+		go trackExecAnalytics(sstore, workspaceNameOrID)
+		return nil
+	}
+
+	var remoteErr breverrors.RemoteExitError
+	if stderrors.As(err, &remoteErr) {
+		return remoteErr
 	}
-	go trackExecAnalytics(sstore, workspaceNameOrID)
-	return nil
+	return breverrors.WrapAndTrace(err)
 }
 
 func trackExecAnalytics(sstore ExecStore, workspaceNameOrID string) {
@@ -296,18 +351,26 @@ func runSSHWithTimeout(sshAlias string, command string, connectTimeoutSecs int)
 	// -T disables pseudo-terminal allocation (no "Pseudo-terminal will not be allocated" warning)
 	// Only start ssh-agent if one isn't already running (avoids orphaned agent processes)
 	agentCmd := `if [ -z "$SSH_AUTH_SOCK" ]; then eval $(ssh-agent -s) > /dev/null; fi`
-	cmd := fmt.Sprintf("%s && ssh -T -o ConnectTimeout=%d -o LogLevel=ERROR %s '%s'", agentCmd, connectTimeoutSecs, sshAlias, escapedCmd)
+	// exec replaces bash with ssh so the exit code we see is ssh's own, not bash's.
+	cmd := fmt.Sprintf("%s && exec ssh -T -o ConnectTimeout=%d -o LogLevel=ERROR %s '%s'", agentCmd, connectTimeoutSecs, sshAlias, escapedCmd)
 
 	sshCmd := exec.Command("bash", "-c", cmd) //nolint:gosec //cmd is user input
 	sshCmd.Stderr = os.Stderr
 	sshCmd.Stdout = os.Stdout
 	// Don't attach stdin - exec is non-interactive
 
-	err := sshCmd.Run()
-	if err != nil {
-		return breverrors.WrapAndTrace(err)
+	return classifySSHError(sshCmd.Run())
+}
+
+// classifySSHError separates a remote command failure from an ssh connection failure.
+func classifySSHError(err error) error {
+	if err == nil {
+		return nil
+	}
+	if code := exitCodeOf(err); code > 0 && code != sshConnectionFailedExitCode {
+		return breverrors.RemoteExitError{Code: code}
 	}
-	return nil
+	return breverrors.WrapAndTrace(err)
 }
 
 func runSSH(sshAlias string, command string) error {

pkg/cmd/exec/exec_test.go

@@ -0,0 +1,111 @@
+package exec
+
+import (
+	stderrors "errors"
+	"os/exec"
+	"strconv"
+	"testing"
+
+	"github.com/brevdev/brev-cli/pkg/entity"
+	breverrors "github.com/brevdev/brev-cli/pkg/errors"
+	"github.com/hashicorp/go-multierror"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// exitErrWithCode returns a real *exec.ExitError carrying the given code.
+func exitErrWithCode(t *testing.T, code int) *exec.ExitError {
+	t.Helper()
+	var exitErr *exec.ExitError
+	err := exec.Command("bash", "-c", "exit "+strconv.Itoa(code)).Run()
+	require.ErrorAs(t, err, &exitErr)
+	return exitErr
+}
+
+func TestClassifySSHError(t *testing.T) {
+	assert.NoError(t, classifySSHError(nil))
+
+	// A remote command's own exit code means the connection worked.
+	for _, code := range []int{1, 2, 7, 127} {
+		var remoteErr breverrors.RemoteExitError
+		err := classifySSHError(exitErrWithCode(t, code))
+		require.True(t, stderrors.As(err, &remoteErr), "code %d should be a RemoteExitError", code)
+		assert.Equal(t, code, remoteErr.Code)
+	}
+
+	// 255 is ssh's own failure code, so it must stay a connection error.
+	var remoteErr breverrors.RemoteExitError
+	err := classifySSHError(exitErrWithCode(t, sshConnectionFailedExitCode))
+	require.Error(t, err)
+	assert.False(t, stderrors.As(err, &remoteErr), "255 must not be treated as a remote exit")
+}
+
+func TestExitCodeOf(t *testing.T) {
+	assert.Equal(t, 3, exitCodeOf(exitErrWithCode(t, 3)))
+	assert.Equal(t, -1, exitCodeOf(stderrors.New("not an exit error")))
+	assert.Equal(t, -1, exitCodeOf(nil))
+}
+
+// A multi-instance run must not exit with one instance's remote code.
+func TestFlattenMultiInstanceErr(t *testing.T) {
+	assert.NoError(t, flattenMultiInstanceErr(nil))
+
+	agg := multierror.Append(nil, breverrors.RemoteExitError{Code: 3}, breverrors.RemoteExitError{Code: 7})
+	flat := flattenMultiInstanceErr(agg)
+
+	var remoteErr breverrors.RemoteExitError
+	require.Error(t, flat)
+	assert.False(t, stderrors.As(flat, &remoteErr), "no exit code may leak from a multi-instance run")
+	// The per-instance detail is still readable in the message.
+	assert.Contains(t, flat.Error(), "status 3")
+	assert.Contains(t, flat.Error(), "status 7")
+}
+
+type fakeTokenStore struct {
+	tokens *entity.AuthTokens
+	err    error
+}
+
+func (f fakeTokenStore) GetAuthTokens() (*entity.AuthTokens, error) { return f.tokens, f.err }
+
+func TestHasNoSavedCredentials(t *testing.T) {
+	cases := map[string]struct {
+		store fakeTokenStore
+		want  bool
+	}{
+		"no credentials file": {fakeTokenStore{err: &breverrors.CredentialsFileNotFound{}}, true},
+		"wrapped not found":   {fakeTokenStore{err: breverrors.WrapAndTrace(&breverrors.CredentialsFileNotFound{})}, true},
+		"nil tokens":          {fakeTokenStore{}, true},
+		"empty tokens":        {fakeTokenStore{tokens: &entity.AuthTokens{}}, true},
+		"has access token":    {fakeTokenStore{tokens: &entity.AuthTokens{AccessToken: "a"}}, false},
+		"has refresh token":   {fakeTokenStore{tokens: &entity.AuthTokens{RefreshToken: "r"}}, false},
+		"has api key":         {fakeTokenStore{tokens: &entity.AuthTokens{APIKey: "k"}}, false},
+		// An unrelated read error must not be reported as missing credentials.
+		"other error": {fakeTokenStore{err: stderrors.New("permission denied")}, false},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			assert.Equal(t, tc.want, hasNoSavedCredentials(tc.store))
+		})
+	}
+}
+
+// RemoteExitError must survive wrapping, since main.go recovers the code with errors.As.
+func TestRemoteExitErrorSurvivesWrapping(t *testing.T) {
+	cases := map[string]error{
+		"unwrapped":    breverrors.RemoteExitError{Code: 4},
+		"wrapped":      breverrors.WrapAndTrace(breverrors.RemoteExitError{Code: 4}),
+		"doubleWrap":   breverrors.WrapAndTrace(breverrors.WrapAndTrace(breverrors.RemoteExitError{Code: 4})),
+		"multierror":   multierror.Append(nil, breverrors.RemoteExitError{Code: 4}),
+		"multiWrapped": multierror.Append(nil, breverrors.WrapAndTrace(breverrors.RemoteExitError{Code: 4})),
+	}
+
+	for name, err := range cases {
+		t.Run(name, func(t *testing.T) {
+			var remoteErr breverrors.RemoteExitError
+			require.True(t, stderrors.As(err, &remoteErr), "errors.As must match through %s", name)
+			assert.Equal(t, 4, remoteErr.Code)
+		})
+	}
+}

pkg/errors/errors.go

@@ -295,3 +295,12 @@ func (e NvidiaMigrationError) Directive() string {
 func NewNvidiaMigrationError(msg string) *NvidiaMigrationError {
 	return &NvidiaMigrationError{Message: msg}
 }
+
+// RemoteExitError means a command ran over SSH and exited non-zero; Code is its exit status.
+type RemoteExitError struct {
+	Code int
+}
+
+func (e RemoteExitError) Error() string {
+	return fmt.Sprintf("command exited with status %d", e.Code)
+}