refactor(cli): mcp create validation and parseKVSlice test polish

Move --server-name empty-check inside runCommand closure to match the
project convention established in alert.go (validation flows through the
same error formatter as all other business-logic errors).

Rewrite TestMCPCreateRejectsEmptyServerName to use execCommand +
mockClient injection so it survives the guard moving inside runCommand
without depending on real config or network.

Add TestParseKVSlice table-driven tests covering nil/empty/single/multi/
value-with-equals/empty-value/empty-key/missing-equals cases.

Author: ysyneu

internal/cli/helpers_test.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"reflect"
 	"strings"
 	"testing"
 )
@@ -125,3 +126,40 @@ func TestOrDash(t *testing.T) {
 func TestMemberPersonInfosDisplay(t *testing.T) {
 	t.Skip("requires injection seam for fake client (Phase 3)")
 }
+
+func TestParseKVSlice(t *testing.T) {
+	cases := []struct {
+		name    string
+		input   []string
+		want    map[string]string
+		wantErr bool
+	}{
+		{"nil input", nil, nil, false},
+		{"empty input", []string{}, nil, false},
+		{"single pair", []string{"K=V"}, map[string]string{"K": "V"}, false},
+		{"multiple pairs", []string{"A=1", "B=2"}, map[string]string{"A": "1", "B": "2"}, false},
+		// Value contains additional '=' signs — only the first splits key from value.
+		{"value contains equals", []string{"K=a=b=c"}, map[string]string{"K": "a=b=c"}, false},
+		{"empty value", []string{"K="}, map[string]string{"K": ""}, false},
+		// Empty-key is the current behaviour when the entry starts with '='; documented here.
+		{"empty key", []string{"=V"}, map[string]string{"": "V"}, false},
+		{"missing equals", []string{"NOEQ"}, nil, true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := parseKVSlice(tc.input)
+			if tc.wantErr {
+				if err == nil {
+					t.Fatal("expected error, got nil")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("got %v, want %v", got, tc.want)
+			}
+		})
+	}
+}

internal/cli/mcp.go

@@ -36,10 +36,10 @@ func newMCPCreateCmd() *cobra.Command {
 		Use:   "create",
 		Short: "Register an MCP server",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if strings.TrimSpace(serverName) == "" {
-				return fmt.Errorf("--server-name is required")
-			}
 			return runCommand(cmd, args, func(ctx *RunContext) error {
+				if strings.TrimSpace(serverName) == "" {
+					return fmt.Errorf("--server-name is required")
+				}
 				envMap, err := parseKVSlice(envEntries)
 				if err != nil {
 					return fmt.Errorf("invalid --env: %w", err)

internal/cli/mcp_test.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"strings"
 	"testing"
 )
 
@@ -19,8 +20,16 @@ func TestMCPCreateFlagSurface(t *testing.T) {
 }
 
 func TestMCPCreateRejectsEmptyServerName(t *testing.T) {
-	cmd := newMCPCreateCmd()
-	if err := cmd.RunE(cmd, nil); err == nil {
+	saveAndResetGlobals(t)
+	// The empty-name guard fires inside runCommand before CreateMCPServer is
+	// ever called, so a no-op stub is sufficient.
+	newClientFn = func() (flashdutyClient, error) { return &mockClient{}, nil }
+
+	_, err := execCommand("mcp", "create")
+	if err == nil {
 		t.Fatal("expected error for empty --server-name, got nil")
 	}
+	if !strings.Contains(err.Error(), "--server-name is required") {
+		t.Fatalf("expected error %q, got %q", "--server-name is required", err.Error())
+	}
 }