fix(group): report a member the server dropped instead of claiming it was added

apps/cinc/cmd/group.go

@@ -204,20 +204,56 @@ cinc group member remove admins alice`),
 			if _, _, err := c.Groups.Update(cmd.Context(), current); err != nil {
 				return err
 			}
+			// erchef accepts a group PUT naming an actor that does not exist
+			// and silently drops it, answering with the body as sent. Read
+			// the group back so an add reports only what actually landed.
+			var dropped []string
+			if add {
+				if changed, dropped, err = keptMembers(cmd, c, group, memberKind(kind), changed); err != nil {
+					return err
+				}
+			}
 			note := ""
 			if len(unchanged) > 0 {
 				note = fmt.Sprintf(" (%s %s it)", strings.Join(unchanged, ", "),
 					memberState(add, len(unchanged) > 1, true))
 			}
-			fmt.Fprintf(out, "%s %s %s group %q%s\n",
-				cases(add, "Added", "Removed"), strings.Join(changed, ", "), preposition, group, note)
+			if len(changed) > 0 {
+				fmt.Fprintf(out, "%s %s %s group %q%s\n",
+					cases(add, "Added", "Removed"), strings.Join(changed, ", "), preposition, group, note)
+			}
+			if len(dropped) > 0 {
+				return fmt.Errorf("the server didn't add %s to group %q. Check that a %s by that name exists in this organization",
+					strings.Join(dropped, ", "), group, kind)
+			}
 			return nil
 		},
 	}
 	cmd.Flags().StringVar(&kind, "type", string(memberUser), "actor type to change: user, client, or group")
 	return cmd
 }
 
+// keptMembers reads group back after an add and splits the names the add
+// sent into those the group now holds and those the server dropped.
+func keptMembers(cmd *cobra.Command, c *cinc.Client, group string, kind memberKind, added []string) (kept, dropped []string, err error) {
+	after, _, err := c.Groups.Get(cmd.Context(), group)
+	if err != nil {
+		return nil, nil, err
+	}
+	members, err := memberSlice(after, kind)
+	if err != nil {
+		return nil, nil, err
+	}
+	for _, name := range added {
+		if slices.Contains(*members, name) {
+			kept = append(kept, name)
+		} else {
+			dropped = append(dropped, name)
+		}
+	}
+	return kept, dropped, nil
+}
+
 // applyMemberChange adds or removes names from the actor list of the
 // given kind on group, in place. It reports which names it changed and
 // which were already as asked (already a member for add, not a member for

apps/cinc/cmd/group_member_test.go

@@ -129,3 +129,25 @@ func TestGroupMemberRemoveReportsOnlyRemovedMembers(t *testing.T) {
 		t.Errorf("output = %q, want %q", out, want)
 	}
 }
+
+// TestGroupMemberAddReportsDroppedMembers adds a real user and one the
+// server has never heard of. erchef accepts the group PUT but silently
+// drops the unknown name, so the command checks what the group holds
+// afterwards: it reports only the member that was added, and fails naming
+// the one that wasn't.
+func TestGroupMemberAddReportsDroppedMembers(t *testing.T) {
+	var gotUsers []string
+	srv := groupMemberServerKnowing(t, "admins", []string{"alice"}, &gotUsers, []string{"alice", "bob"})
+	cfg := groupMemberConfig(t, srv)
+
+	out, err := runGroupMember(t, cfg, "add", "admins", "bob", "ghost")
+	if err == nil {
+		t.Fatalf("adding an unknown user succeeded: %q", out)
+	}
+	if !strings.Contains(err.Error(), "ghost") || strings.Contains(err.Error(), "bob") {
+		t.Errorf("error = %v, want it to name ghost and only ghost", err)
+	}
+	if want := "Added bob to group \"admins\"\n"; out != want {
+		t.Errorf("output = %q, want %q", out, want)
+	}
+}

apps/cinc/cmd/group_test.go

@@ -185,6 +185,14 @@ client_key      = %q
 // groupMemberServer serves GET and PUT for a single group, recording
 // the actors.users slice from any PUT body.
 func groupMemberServer(t *testing.T, name string, users []string, gotUsers *[]string) *httptest.Server {
+	t.Helper()
+	return groupMemberServerKnowing(t, name, users, gotUsers, nil)
+}
+
+// groupMemberServerKnowing is groupMemberServer for a server that knows
+// only the users in known (nil means every name exists): like erchef, a PUT
+// silently drops a member it cannot find, and later GETs show what it kept.
+func groupMemberServerKnowing(t *testing.T, name string, users []string, gotUsers *[]string, known []string) *httptest.Server {
 	t.Helper()
 	mux := http.NewServeMux()
 	mux.HandleFunc("/organizations/acme/groups/"+name, func(w http.ResponseWriter, r *http.Request) {
@@ -200,6 +208,13 @@ func groupMemberServer(t *testing.T, name string, users []string, gotUsers *[]st
 			}
 			_ = json.NewDecoder(r.Body).Decode(&body)
 			*gotUsers = body.Actors.Users
+			users = nil
+			for _, u := range body.Actors.Users {
+				if known == nil || slices.Contains(known, u) {
+					users = append(users, u)
+				}
+			}
+			// erchef answers with the body as sent, not what it stored.
 			_ = json.NewEncoder(w).Encode(cinc.Group{GroupName: name, Name: name, Users: body.Actors.Users})
 		default:
 			t.Errorf("unexpected method %q", r.Method)

integration/cincserverng/cincserverng_test.go

@@ -75,9 +75,6 @@ func run(m *testing.M) int {
 		Gaps: map[string]string{
 			"keys/client-edit-partial": "key PUT drops fields the body omits: https://github.com/cinc-project/cinc-server-ng/issues/163 (fixed on main by #202, not yet released)",
 			"keys/client-edit-rename":  "key PUT ignores a new name: https://github.com/cinc-project/cinc-server-ng/issues/163 (fixed on main by #202, not yet released)",
-			// v0.14.0 drops unknown members as erchef does; what remains is
-			// the CLI reporting a dropped member as added, fixed separately.
-			"groups/member-unknown": "group PUT dropping unknown members (https://github.com/cinc-project/cinc-server-ng/issues/186) is fixed; the CLI does not yet report a dropped member",
 		},
 	}
 	return m.Run()