[ENG-6795] Skip Sentry capture for CLI flag validation errors

cmd/changes_submit_plan.go

@@ -182,11 +182,7 @@ func SubmitPlan(cmd *cobra.Command, args []string) error {
 
 	enrichedTags, err := parseTagsArgument()
 	if err != nil {
-		return loggedError{
-			err:     err,
-			fields:  lf,
-			message: "Failed to parse tags",
-		}
+		return err
 	}
 
 	labels, err := parseLabelsArgument()

cmd/flags.go

@@ -48,7 +48,7 @@ func parseTagsArgument() (*sdp.EnrichedTags, error) {
 	for _, tag := range viper.GetStringSlice("tags") {
 		parts := strings.SplitN(tag, "=", 2)
 		if len(parts) != 2 {
-			return nil, fmt.Errorf("invalid tag format: %s", tag)
+			return nil, flagError{usage: fmt.Sprintf("invalid --tags value '%s', expected format 'key=value'", tag)}
 		}
 		tags[parts[0]] = parts[1]
 	}

cmd/flags_test.go

@@ -1,13 +1,89 @@
 package cmd
 
 import (
+	"errors"
 	"strings"
 	"testing"
 
 	"github.com/overmindtech/cli/go/sdp-go"
 	"github.com/spf13/viper"
 )
 
+func TestParseTagsArgument(t *testing.T) {
+	tests := []struct {
+		name          string
+		tags          []string
+		want          map[string]string
+		errorContains string
+		flagError     bool
+	}{
+		{
+			name: "empty tags",
+			tags: []string{},
+			want: map[string]string{},
+		},
+		{
+			name: "single tag",
+			tags: []string{"env=prod"},
+			want: map[string]string{"env": "prod"},
+		},
+		{
+			name: "multiple tags",
+			tags: []string{"env=prod", "team=platform"},
+			want: map[string]string{"env": "prod", "team": "platform"},
+		},
+		{
+			name:          "missing equals sign",
+			tags:          []string{"bad"},
+			errorContains: "invalid --tags value 'bad', expected format 'key=value'",
+			flagError:     true,
+		},
+		{
+			name: "empty value is allowed",
+			tags: []string{"key="},
+			want: map[string]string{"key": ""},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			viper.Reset()
+			viper.Set("tags", tt.tags)
+
+			got, err := parseTagsArgument()
+			if tt.errorContains != "" {
+				if err == nil {
+					t.Fatalf("parseTagsArgument() expected error containing %q, got nil", tt.errorContains)
+				}
+				if !strings.Contains(err.Error(), tt.errorContains) {
+					t.Fatalf("parseTagsArgument() error = %v, want containing %q", err, tt.errorContains)
+				}
+				if tt.flagError {
+					if _, ok := errors.AsType[flagError](err); !ok {
+						t.Fatalf("parseTagsArgument() expected flagError, got %T", err)
+					}
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("parseTagsArgument() unexpected error: %v", err)
+			}
+			if len(got.GetTagValue()) != len(tt.want) {
+				t.Fatalf("parseTagsArgument() returned %d tags, want %d", len(got.GetTagValue()), len(tt.want))
+			}
+			for key, value := range tt.want {
+				tag, ok := got.GetTagValue()[key]
+				if !ok {
+					t.Fatalf("parseTagsArgument() missing tag %q", key)
+				}
+				if tag.GetUserTagValue().GetValue() != value {
+					t.Fatalf("parseTagsArgument() tag %q = %q, want %q", key, tag.GetUserTagValue().GetValue(), value)
+				}
+			}
+		})
+	}
+}
+
 func TestParseLabelsArgument(t *testing.T) {
 	tests := []struct {
 		name          string

cmd/root.go

@@ -149,24 +149,7 @@ func Execute() {
 
 		err := rootCmd.ExecuteContext(ctx)
 		if err != nil {
-			switch err := err.(type) { //nolint:errorlint // the selected error types are all top-level wrappers used by the CLI implementation
-			case flagError:
-				// print errors from viper with usage to stderr
-				fmt.Fprintln(os.Stderr, err)
-			case loggedError:
-				log.WithContext(ctx).WithError(err.err).WithFields(err.fields).Error(err.message)
-			}
-			if cmdSpan != nil {
-				// if printing the error was not requested by the appropriate
-				// wrapper, only record the data to honeycomb and sentry, the
-				// command already has handled logging
-				cmdSpan.SetAttributes(
-					attribute.Bool("ovm.cli.fatalError", true),
-					attribute.String("ovm.cli.fatalError.msg", err.Error()),
-				)
-				cmdSpan.RecordError(err)
-			}
-			sentry.CaptureException(err)
+			reportCommandError(ctx, err, sentry.CaptureException)
 		}
 
 		return err
@@ -402,6 +385,41 @@ func (f flagError) Error() string {
 	return f.usage
 }
 
+// captureExceptionFunc is the Sentry capture hook used by reportCommandError.
+type captureExceptionFunc func(error) *sentry.EventID
+
+// reportCommandError prints expected CLI errors, records them on the command
+// span for Honeycomb, and reports unexpected failures to Sentry.
+//
+// flagError is returned for malformed user input (invalid flags, missing
+// required arguments, and similar validation). Those are expected exit paths:
+// the user already sees the validation message and a non-zero exit. Sending
+// them to Sentry creates high-priority noise (ENG-6795 / CLI-Q).
+func reportCommandError(ctx context.Context, err error, capture captureExceptionFunc) {
+	reportToSentry := true
+	switch e := err.(type) { //nolint:errorlint // the selected error types are all top-level wrappers used by the CLI implementation
+	case flagError:
+		// print errors from viper with usage to stderr
+		fmt.Fprintln(os.Stderr, e)
+		reportToSentry = false
+	case loggedError:
+		log.WithContext(ctx).WithError(e.err).WithFields(e.fields).Error(e.message)
+	}
+	if cmdSpan != nil {
+		// if printing the error was not requested by the appropriate
+		// wrapper, only record the data to honeycomb and sentry, the
+		// command already has handled logging
+		cmdSpan.SetAttributes(
+			attribute.Bool("ovm.cli.fatalError", true),
+			attribute.String("ovm.cli.fatalError.msg", err.Error()),
+		)
+		cmdSpan.RecordError(err)
+	}
+	if reportToSentry && capture != nil {
+		capture(err)
+	}
+}
+
 type loggedError struct {
 	err     error
 	fields  log.Fields

cmd/root_error_test.go

@@ -0,0 +1,96 @@
+package cmd
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/getsentry/sentry-go"
+	log "github.com/sirupsen/logrus"
+)
+
+func TestReportCommandErrorSkipsSentryForFlagError(t *testing.T) {
+	readStderr := captureStderr(t)
+	var captured []error
+	capture := func(err error) *sentry.EventID {
+		captured = append(captured, err)
+		return nil
+	}
+
+	err := flagError{usage: "invalid --tags value 'bad', expected format 'key=value'"}
+	reportCommandError(context.Background(), err, capture)
+
+	if len(captured) != 0 {
+		t.Fatalf("flagError should not be sent to Sentry, got %d events: %v", len(captured), captured)
+	}
+	got := readStderr()
+	if !strings.Contains(got, "invalid --tags value 'bad', expected format 'key=value'") {
+		t.Fatalf("flagError should still be printed to stderr, got %q", got)
+	}
+}
+
+func TestReportCommandErrorSendsUnexpectedErrorsToSentry(t *testing.T) {
+	var captured []error
+	capture := func(err error) *sentry.EventID {
+		captured = append(captured, err)
+		return nil
+	}
+
+	unexpected := errors.New("backend unavailable")
+	reportCommandError(context.Background(), unexpected, capture)
+	if len(captured) != 1 || captured[0] != unexpected {
+		t.Fatalf("unexpected errors should be sent to Sentry, got %v", captured)
+	}
+
+	logged := loggedError{
+		err:     errors.New("dial tcp: lookup api.overmind.tech"),
+		fields:  log.Fields{"app": "https://app.overmind.tech"},
+		message: "Failed searching for existing changes",
+	}
+	reportCommandError(context.Background(), logged, capture)
+	if len(captured) != 2 {
+		t.Fatalf("loggedError should still be sent to Sentry, got %d events", len(captured))
+	}
+	if _, ok := captured[1].(loggedError); !ok {
+		t.Fatalf("expected loggedError to be captured, got %T", captured[1])
+	}
+}
+
+func TestReportCommandErrorNilCaptureIsSafe(t *testing.T) {
+	// Should not panic when Sentry is not configured.
+	reportCommandError(context.Background(), errors.New("boom"), nil)
+	reportCommandError(context.Background(), flagError{usage: "invalid --timeout value 'nope'"}, nil)
+}
+
+func captureStderr(t *testing.T) func() string {
+	t.Helper()
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("pipe: %v", err)
+	}
+	old := os.Stderr
+	os.Stderr = w
+	buf := &bytes.Buffer{}
+	done := make(chan struct{})
+	go func() {
+		_, _ = io.Copy(buf, r)
+		close(done)
+	}()
+	restore := func() string {
+		os.Stderr = old
+		_ = w.Close()
+		<-done
+		_ = r.Close()
+		return buf.String()
+	}
+	t.Cleanup(func() {
+		if os.Stderr == w {
+			_ = restore()
+		}
+	})
+	return restore
+}

cmd/terraform_plan.go

@@ -245,7 +245,7 @@ func TerraformPlanImpl(ctx context.Context, cmd *cobra.Command, oi sdp.OvermindI
 	enrichedTags, err := parseTagsArgument()
 	if err != nil {
 		uploadChangesSpinner.Fail(fmt.Sprintf("Uploading planned changes: failed to parse tags: %v", err))
-		return nil
+		return err
 	}
 
 	labels, err := parseLabelsArgument()