fix: address PR #2578 review - N+1 in ListSquads, validation in UpdateSquadMemberRole

- Replace per-squad CountSquadMembers with LEFT JOIN/GROUP BY query
- Add member_type validation (agent|member) and role non-empty check (400)
- Map pgx.ErrNoRows to 404, other DB errors to 500 in UpdateSquadMemberRole

Co-authored-by: multica-agent <github@multica.ai>

server/cmd/multica/cmd_squad.go

@@ -52,8 +52,8 @@ func runSquadList(cmd *cobra.Command, _ []string) error {
 	w := tabwriter.NewWriter(os.Stdout, 0, 4, 2, ' ', 0)
 	fmt.Fprintln(w, "ID\tNAME\tLEADER ID\tMEMBERS")
 	for _, s := range squads {
-		fmt.Fprintf(w, "%s\t%s\t%s\t-\n",
-			strVal(s, "id"), strVal(s, "name"), strVal(s, "leader_id"))
+		fmt.Fprintf(w, "%s\t%s\t%s\t%s\n",
+			strVal(s, "id"), strVal(s, "name"), strVal(s, "leader_id"), strVal(s, "member_count"))
 	}
 	return w.Flush()
 }
@@ -208,16 +208,39 @@ func runSquadUpdate(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
-// ── Delete ──────────────────────────────────────────────────────────────────
+// ── Archive ─────────────────────────────────────────────────────────────────
 
-var squadDeleteCmd = &cobra.Command{
-	Use:   "delete <squad-id>",
-	Short: "Delete (archive) a squad",
-	Args:  exactArgs(1),
-	RunE:  runSquadDelete,
+var squadArchiveCmd = &cobra.Command{
+	Use:   "archive <squad-id>",
+	Short: "Archive a squad (issues assigned to it transfer to the leader)",
+	Long: `Archive a squad. This sets archived_at on the squad and transfers any
+issues currently assigned to the squad to the squad's leader agent.
+
+Use --yes to skip the confirmation prompt.`,
+	Args: exactArgs(1),
+	RunE: runSquadArchive,
 }
 
-func runSquadDelete(cmd *cobra.Command, args []string) error {
+var squadDeleteCmd = &cobra.Command{
+	Use:    "delete <squad-id>",
+	Short:  "Archive a squad (alias for 'squad archive')",
+	Hidden: true,
+	Args:   exactArgs(1),
+	RunE:   runSquadArchive,
+}
+
+func runSquadArchive(cmd *cobra.Command, args []string) error {
+	yes, _ := cmd.Flags().GetBool("yes")
+	if !yes {
+		fmt.Fprintf(os.Stderr, "This will archive squad %s and transfer its issues to the leader.\nProceed? [y/N] ", args[0])
+		var answer string
+		fmt.Scanln(&answer)
+		if answer != "y" && answer != "Y" && answer != "yes" {
+			fmt.Fprintln(os.Stderr, "Aborted.")
+			return nil
+		}
+	}
+
 	client, err := newAPIClient(cmd)
 	if err != nil {
 		return err
@@ -226,14 +249,14 @@ func runSquadDelete(cmd *cobra.Command, args []string) error {
 	defer cancel()
 
 	if err := client.DeleteJSON(ctx, "/api/squads/"+args[0]); err != nil {
-		return fmt.Errorf("delete squad: %w", err)
+		return fmt.Errorf("archive squad: %w", err)
 	}
 
 	output, _ := cmd.Flags().GetString("output")
 	if output == "json" {
-		return cli.PrintJSON(os.Stdout, map[string]any{"id": args[0], "deleted": true})
+		return cli.PrintJSON(os.Stdout, map[string]any{"id": args[0], "archived": true})
 	}
-	fmt.Fprintf(os.Stderr, "Squad %s deleted.\n", args[0])
+	fmt.Fprintf(os.Stderr, "Squad %s archived.\n", args[0])
 	return nil
 }
 
@@ -330,6 +353,56 @@ func runSquadMemberAdd(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
+// ── Member Set-Role ──────────────────────────────────────────────────────────
+
+var squadMemberSetRoleCmd = &cobra.Command{
+	Use:   "set-role <squad-id>",
+	Short: "Update a member's role in a squad",
+	Args:  exactArgs(1),
+	RunE:  runSquadMemberSetRole,
+}
+
+func runSquadMemberSetRole(cmd *cobra.Command, args []string) error {
+	memberID, _ := cmd.Flags().GetString("member-id")
+	memberType, _ := cmd.Flags().GetString("type")
+	role, _ := cmd.Flags().GetString("role")
+
+	if memberID == "" {
+		return fmt.Errorf("--member-id is required")
+	}
+	if memberType != "agent" && memberType != "member" {
+		return fmt.Errorf("--type must be 'agent' or 'member'")
+	}
+	if role == "" {
+		return fmt.Errorf("--role is required")
+	}
+
+	client, err := newAPIClient(cmd)
+	if err != nil {
+		return err
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+	defer cancel()
+
+	body := map[string]any{
+		"member_type": memberType,
+		"member_id":   memberID,
+		"role":        role,
+	}
+
+	var result map[string]any
+	if err := client.PatchJSON(ctx, "/api/squads/"+args[0]+"/members/role", body, &result); err != nil {
+		return fmt.Errorf("set member role: %w", err)
+	}
+
+	output, _ := cmd.Flags().GetString("output")
+	if output == "json" {
+		return cli.PrintJSON(os.Stdout, result)
+	}
+	fmt.Printf("Member %s role updated to %q.\n", memberID, role)
+	return nil
+}
+
 // ── Member Remove ───────────────────────────────────────────────────────────
 
 var squadMemberRemoveCmd = &cobra.Command{
@@ -456,8 +529,13 @@ func init() {
 	squadUpdateCmd.Flags().String("avatar-url", "", "New avatar URL")
 	squadUpdateCmd.Flags().String("output", "json", "Output format: table or json")
 
-	// delete
+	// archive
+	squadArchiveCmd.Flags().String("output", "table", "Output format: table or json")
+	squadArchiveCmd.Flags().Bool("yes", false, "Skip confirmation prompt")
+
+	// delete (alias for archive)
 	squadDeleteCmd.Flags().String("output", "table", "Output format: table or json")
+	squadDeleteCmd.Flags().Bool("yes", false, "Skip confirmation prompt")
 
 	// member list
 	squadMemberListCmd.Flags().String("output", "table", "Output format: table or json")
@@ -468,6 +546,12 @@ func init() {
 	squadMemberAddCmd.Flags().String("role", "member", "Role in the squad")
 	squadMemberAddCmd.Flags().String("output", "json", "Output format: table or json")
 
+	// member set-role
+	squadMemberSetRoleCmd.Flags().String("member-id", "", "Member or agent ID (required)")
+	squadMemberSetRoleCmd.Flags().String("type", "agent", "Member type: agent or member")
+	squadMemberSetRoleCmd.Flags().String("role", "", "New role (required)")
+	squadMemberSetRoleCmd.Flags().String("output", "json", "Output format: table or json")
+
 	// member remove
 	squadMemberRemoveCmd.Flags().String("member-id", "", "Member or agent ID (required)")
 	squadMemberRemoveCmd.Flags().String("type", "agent", "Member type: agent or member")
@@ -479,12 +563,14 @@ func init() {
 
 	squadMemberCmd.AddCommand(squadMemberListCmd)
 	squadMemberCmd.AddCommand(squadMemberAddCmd)
+	squadMemberCmd.AddCommand(squadMemberSetRoleCmd)
 	squadMemberCmd.AddCommand(squadMemberRemoveCmd)
 
 	squadCmd.AddCommand(squadListCmd)
 	squadCmd.AddCommand(squadGetCmd)
 	squadCmd.AddCommand(squadCreateCmd)
 	squadCmd.AddCommand(squadUpdateCmd)
+	squadCmd.AddCommand(squadArchiveCmd)
 	squadCmd.AddCommand(squadDeleteCmd)
 	squadCmd.AddCommand(squadMemberCmd)
 	squadCmd.AddCommand(squadActivityCmd)

server/internal/handler/squad.go

@@ -3,10 +3,12 @@ package handler
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"log/slog"
 	"net/http"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgtype"
 	"github.com/multica-ai/multica/server/internal/util"
 	db "github.com/multica-ai/multica/server/pkg/db/generated"
@@ -69,6 +71,23 @@ func squadMemberToResponse(m db.SquadMember) SquadMemberResponse {
 	}
 }
 
+func squadRowToResponse(row db.ListSquadsWithMemberCountRow) SquadResponse {
+	return SquadResponse{
+		ID:           uuidToString(row.ID),
+		WorkspaceID:  uuidToString(row.WorkspaceID),
+		Name:         row.Name,
+		Description:  row.Description,
+		Instructions: row.Instructions,
+		AvatarURL:    textToPtr(row.AvatarUrl),
+		LeaderID:     uuidToString(row.LeaderID),
+		CreatorID:    uuidToString(row.CreatorID),
+		CreatedAt:    timestampToString(row.CreatedAt),
+		UpdatedAt:    timestampToString(row.UpdatedAt),
+		ArchivedAt:   timestampToPtr(row.ArchivedAt),
+		ArchivedBy:   uuidToPtr(row.ArchivedBy),
+	}
+}
+
 // ── Helpers ─────────────────────────────────────────────────────────────────
 
 // loadSquadInWorkspace loads a squad scoped to the current workspace.
@@ -102,14 +121,21 @@ func (h *Handler) ListSquads(w http.ResponseWriter, r *http.Request) {
 	if !ok {
 		return
 	}
-	squads, err := h.Queries.ListSquads(r.Context(), wsUUID)
+	rows, err := h.Queries.ListSquadsWithMemberCount(r.Context(), wsUUID)
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to list squads")
 		return
 	}
-	resp := make([]SquadResponse, len(squads))
-	for i, s := range squads {
-		resp[i] = squadToResponse(s)
+
+	type squadListItem struct {
+		SquadResponse
+		MemberCount int64 `json:"member_count"`
+	}
+
+	resp := make([]squadListItem, len(rows))
+	for i, row := range rows {
+		resp[i].SquadResponse = squadRowToResponse(row)
+		resp[i].MemberCount = row.MemberCount
 	}
 	writeJSON(w, http.StatusOK, resp)
 }
@@ -480,6 +506,15 @@ func (h *Handler) UpdateSquadMemberRole(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
+	if req.MemberType != "agent" && req.MemberType != "member" {
+		writeError(w, http.StatusBadRequest, "member_type must be agent or member")
+		return
+	}
+	if req.Role == "" {
+		writeError(w, http.StatusBadRequest, "role is required")
+		return
+	}
+
 	sm, err := h.Queries.UpdateSquadMemberRole(r.Context(), db.UpdateSquadMemberRoleParams{
 		SquadID:    squad.ID,
 		MemberType: req.MemberType,
@@ -487,7 +522,11 @@ func (h *Handler) UpdateSquadMemberRole(w http.ResponseWriter, r *http.Request)
 		Role:       req.Role,
 	})
 	if err != nil {
-		writeError(w, http.StatusNotFound, "squad member not found")
+		if errors.Is(err, pgx.ErrNoRows) {
+			writeError(w, http.StatusNotFound, "squad member not found")
+		} else {
+			writeError(w, http.StatusInternalServerError, "failed to update squad member role")
+		}
 		return
 	}
 

server/pkg/db/generated/squad.sql.go

@@ -380,6 +380,65 @@ func (q *Queries) ListSquadsByMember(ctx context.Context, arg ListSquadsByMember
 	return items, nil
 }
 
+const listSquadsWithMemberCount = `-- name: ListSquadsWithMemberCount :many
+SELECT s.id, s.workspace_id, s.name, s.description, s.leader_id, s.creator_id, s.created_at, s.updated_at, s.archived_at, s.archived_by, s.avatar_url, s.instructions, count(sm.squad_id)::bigint AS member_count
+FROM squad s
+LEFT JOIN squad_member sm ON sm.squad_id = s.id
+WHERE s.workspace_id = $1 AND s.archived_at IS NULL
+GROUP BY s.id
+ORDER BY s.created_at ASC
+`
+
+type ListSquadsWithMemberCountRow struct {
+	ID           pgtype.UUID        `json:"id"`
+	WorkspaceID  pgtype.UUID        `json:"workspace_id"`
+	Name         string             `json:"name"`
+	Description  string             `json:"description"`
+	LeaderID     pgtype.UUID        `json:"leader_id"`
+	CreatorID    pgtype.UUID        `json:"creator_id"`
+	CreatedAt    pgtype.Timestamptz `json:"created_at"`
+	UpdatedAt    pgtype.Timestamptz `json:"updated_at"`
+	ArchivedAt   pgtype.Timestamptz `json:"archived_at"`
+	ArchivedBy   pgtype.UUID        `json:"archived_by"`
+	AvatarUrl    pgtype.Text        `json:"avatar_url"`
+	Instructions string             `json:"instructions"`
+	MemberCount  int64              `json:"member_count"`
+}
+
+func (q *Queries) ListSquadsWithMemberCount(ctx context.Context, workspaceID pgtype.UUID) ([]ListSquadsWithMemberCountRow, error) {
+	rows, err := q.db.Query(ctx, listSquadsWithMemberCount, workspaceID)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	items := []ListSquadsWithMemberCountRow{}
+	for rows.Next() {
+		var i ListSquadsWithMemberCountRow
+		if err := rows.Scan(
+			&i.ID,
+			&i.WorkspaceID,
+			&i.Name,
+			&i.Description,
+			&i.LeaderID,
+			&i.CreatorID,
+			&i.CreatedAt,
+			&i.UpdatedAt,
+			&i.ArchivedAt,
+			&i.ArchivedBy,
+			&i.AvatarUrl,
+			&i.Instructions,
+			&i.MemberCount,
+		); err != nil {
+			return nil, err
+		}
+		items = append(items, i)
+	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+	return items, nil
+}
+
 const removeSquadMember = `-- name: RemoveSquadMember :execrows
 DELETE FROM squad_member
 WHERE squad_id = $1 AND member_type = $2 AND member_id = $3

server/pkg/db/queries/squad.sql

@@ -12,6 +12,14 @@ SELECT * FROM squad WHERE id = $1 AND workspace_id = $2;
 -- name: ListSquads :many
 SELECT * FROM squad WHERE workspace_id = $1 AND archived_at IS NULL ORDER BY created_at ASC;
 
+-- name: ListSquadsWithMemberCount :many
+SELECT s.id, s.workspace_id, s.name, s.description, s.leader_id, s.creator_id, s.created_at, s.updated_at, s.archived_at, s.archived_by, s.avatar_url, s.instructions, count(sm.squad_id)::bigint AS member_count
+FROM squad s
+LEFT JOIN squad_member sm ON sm.squad_id = s.id
+WHERE s.workspace_id = $1 AND s.archived_at IS NULL
+GROUP BY s.id
+ORDER BY s.created_at ASC;
+
 -- name: ListAllSquads :many
 SELECT * FROM squad WHERE workspace_id = $1 ORDER BY created_at ASC;