From 13e7082517fc759bef490898f776a85b51e2a542 Mon Sep 17 00:00:00 2001
From: Danielle Maywood <danielle@themaywoods.com>
Date: Fri, 29 May 2026 10:55:26 +0000
Subject: [PATCH] fix(cli): require chat share add roles

---
 cli/exp_chat_share.go          | 25 ++++++-------------------
 cli/exp_chat_share_add.go      | 13 ++++++-------
 cli/exp_chat_share_add_test.go | 21 ++++-----------------
 3 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/cli/exp_chat_share.go b/cli/exp_chat_share.go
index 4abaf2ca38..9029c6036e 100644
--- a/cli/exp_chat_share.go
+++ b/cli/exp_chat_share.go
@@ -29,12 +29,11 @@ func (r *RootCmd) chatShareCommand() *serpent.Command {
 const chatShareDefaultGroupDisplay = "-"
 
 type chatRoleLookupParams struct {
-	Client      *codersdk.Client
-	OrgID       uuid.UUID
-	OrgName     string
-	Users       [][2]string
-	Groups      [][2]string
-	DefaultRole codersdk.ChatRole
+	Client  *codersdk.Client
+	OrgID   uuid.UUID
+	OrgName string
+	Users   [][2]string
+	Groups  [][2]string
 }
 
 func parseChatShareID(raw string) (uuid.UUID, error) {
@@ -47,18 +46,12 @@ func parseChatShareID(raw string) (uuid.UUID, error) {
 
 func parseChatShareActorRole(raw string) ([2]string, error) {
 	name, role, hasRole := strings.Cut(raw, ":")
-	if strings.Contains(role, ":") {
+	if !hasRole || role == "" || strings.Contains(role, ":") {
 		return [2]string{}, xerrors.New("must match pattern 'name:role'")
 	}
 	if name == "" || !codersdk.UsernameValidRegex.MatchString(name) {
 		return [2]string{}, xerrors.New("invalid name")
 	}
-	if !hasRole {
-		return [2]string{name, ""}, nil
-	}
-	if role == "" {
-		return [2]string{}, xerrors.New("role cannot be empty")
-	}
 	return [2]string{name, role}, nil
 }
 
@@ -84,9 +77,6 @@ func fetchChatUsersAndGroups(ctx context.Context, params chatRoleLookupParams) (
 		for _, user := range params.Users {
 			username := user[0]
 			role := user[1]
-			if role == "" {
-				role = string(params.DefaultRole)
-			}
 
 			userID := ""
 			for _, member := range orgMembers {
@@ -119,9 +109,6 @@ func fetchChatUsersAndGroups(ctx context.Context, params chatRoleLookupParams) (
 		for _, group := range params.Groups {
 			groupName := group[0]
 			role := group[1]
-			if role == "" {
-				role = string(params.DefaultRole)
-			}
 
 			var orgGroup *codersdk.Group
 			for _, candidate := range orgGroups {
diff --git a/cli/exp_chat_share_add.go b/cli/exp_chat_share_add.go
index b7c97261aa..89a44e4797 100644
--- a/cli/exp_chat_share_add.go
+++ b/cli/exp_chat_share_add.go
@@ -19,12 +19,12 @@ func (r *RootCmd) chatShareAddCommand() *serpent.Command {
 		Options: serpent.OptionSet{
 			{
 				Name:        "user",
-				Description: "A comma separated list of users to share the chat with.",
+				Description: "A comma separated list of users and roles to share the chat with.",
 				Flag:        "user",
 				Value:       serpent.StringArrayOf(&users),
 			}, {
 				Name:        "group",
-				Description: "A comma separated list of groups to share the chat with.",
+				Description: "A comma separated list of groups and roles to share the chat with.",
 				Flag:        "group",
 				Value:       serpent.StringArrayOf(&groups),
 			},
@@ -72,11 +72,10 @@ func (r *RootCmd) chatShareAddCommand() *serpent.Command {
 			}
 
 			userRoles, groupRoles, err := fetchChatUsersAndGroups(inv.Context(), chatRoleLookupParams{
-				Client:      client,
-				OrgID:       chat.OrganizationID,
-				Users:       userRoleStrings,
-				Groups:      groupRoleStrings,
-				DefaultRole: codersdk.ChatRoleRead,
+				Client: client,
+				OrgID:  chat.OrganizationID,
+				Users:  userRoleStrings,
+				Groups: groupRoleStrings,
 			})
 			if err != nil {
 				return err
diff --git a/cli/exp_chat_share_add_test.go b/cli/exp_chat_share_add_test.go
index 36d6685ccb..02d5997a15 100644
--- a/cli/exp_chat_share_add_test.go
+++ b/cli/exp_chat_share_add_test.go
@@ -57,7 +57,7 @@ func TestExpChatShareAdd(t *testing.T) {
 		assert.Contains(t, out.String(), string(codersdk.ChatRoleRead))
 	})
 
-	t.Run("ShareWithUserDefaultReadRole", func(t *testing.T) {
+	t.Run("RequiresRole", func(t *testing.T) {
 		t.Parallel()
 
 		client, db := coderdtest.NewWithDatabase(t, nil)
@@ -68,29 +68,16 @@ func TestExpChatShareAdd(t *testing.T) {
 			OrganizationID:    firstUser.OrganizationID,
 			OwnerID:           firstUser.UserID,
 			LastModelConfigID: modelConfig.ID,
-			Title:             "share add user default role",
+			Title:             "share add user missing role",
 		})
 
 		ctx := testutil.Context(t, testutil.WaitMedium)
 		inv, root := clitest.New(t, "exp", "chat", "share", "add", chat.ID.String(), "--user", toShareWithUser.Username)
 		clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL operations require the chat owner in this fixture.
 
-		out := new(bytes.Buffer)
-		inv.Stdout = out
 		err := inv.WithContext(ctx).Run()
-		require.NoError(t, err)
-
-		acl, err := codersdk.NewExperimentalClient(client).GetChatACL(ctx, chat.ID)
-		require.NoError(t, err)
-		assert.Contains(t, acl.Users, codersdk.ChatUser{
-			MinimalUser: codersdk.MinimalUser{
-				ID:        toShareWithUser.ID,
-				Username:  toShareWithUser.Username,
-				Name:      toShareWithUser.Name,
-				AvatarURL: toShareWithUser.AvatarURL,
-			},
-			Role: codersdk.ChatRoleRead,
-		})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "must match pattern 'name:role'")
 	})
 
 	t.Run("ShareWithMultipleUsers", func(t *testing.T) {