feat(review): role + setup foundation (PR 1 of 2)

cmd/entire/cli/review/cmd.go

@@ -209,6 +209,7 @@ Subcommands:
 	if deps.AttachCmd != nil {
 		cmd.AddCommand(deps.AttachCmd)
 	}
+	cmd.AddCommand(newReviewSetupCmd(deps))
 	return cmd
 }
 

cmd/entire/cli/review/roles.go

@@ -0,0 +1,84 @@
+// Package review — see env.go for package-level rationale.
+//
+// roles.go provides role helpers used by runReview and the setup
+// subcommand. The migration body lives in the settings package
+// (MigrateLegacyRoles); this file thin-wraps it so review-package
+// tests can exercise everything in one place.
+package review
+
+import (
+	"sort"
+
+	"github.com/entireio/cli/cmd/entire/cli/settings"
+)
+
+// NormalizeRoles enforces the at-most-one-fixer invariant. Empty roles
+// upgrade to Reviewer. Duplicates: alphabetical winner keeps its role;
+// the rest are demoted to Reviewer. Returns a new map.
+func NormalizeRoles(in map[string]settings.ReviewConfig) map[string]settings.ReviewConfig {
+	out := make(map[string]settings.ReviewConfig, len(in))
+	if len(in) == 0 {
+		return out
+	}
+	var fixerCandidates []string
+	for name, cfg := range in {
+		if cfg.Role == "" {
+			cfg.Role = settings.RoleReviewer
+		}
+		if cfg.Role.IsFixer() {
+			fixerCandidates = append(fixerCandidates, name)
+		}
+		out[name] = cfg
+	}
+	if len(fixerCandidates) > 1 {
+		sort.Strings(fixerCandidates)
+		for _, loser := range fixerCandidates[1:] {
+			cfg := out[loser]
+			cfg.Role = settings.RoleReviewer
+			out[loser] = cfg
+		}
+	}
+	return out
+}
+
+// MigrateLegacyRoles is a thin wrapper around settings.MigrateLegacyRoles
+// to keep the review-package test surface cohesive.
+func MigrateLegacyRoles(s *settings.EntireSettings) bool {
+	return settings.MigrateLegacyRoles(s)
+}
+
+// ReviewersOf returns the sorted set of agent names with RoleReviewer
+// or RoleBoth.
+func ReviewersOf(s *settings.EntireSettings) []string {
+	if s == nil {
+		return nil
+	}
+	var out []string
+	for name, cfg := range s.Review {
+		if cfg.Role.IsReviewer() {
+			out = append(out, name)
+		}
+	}
+	sort.Strings(out)
+	return out
+}
+
+// FixerOf returns the agent name with RoleFixer or RoleBoth. Returns ""
+// when no Fixer is configured. Assumes NormalizeRoles has been called;
+// in the duplicate-fixer case returns the alphabetically-first.
+func FixerOf(s *settings.EntireSettings) string {
+	if s == nil {
+		return ""
+	}
+	var candidates []string
+	for name, cfg := range s.Review {
+		if cfg.Role.IsFixer() {
+			candidates = append(candidates, name)
+		}
+	}
+	if len(candidates) == 0 {
+		return ""
+	}
+	sort.Strings(candidates)
+	return candidates[0]
+}

cmd/entire/cli/review/roles_test.go

@@ -0,0 +1,106 @@
+package review_test
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/entireio/cli/cmd/entire/cli/review"
+	"github.com/entireio/cli/cmd/entire/cli/settings"
+)
+
+func TestNormalizeRoles_DefaultsEmptyToReviewer(t *testing.T) {
+	t.Parallel()
+	in := map[string]settings.ReviewConfig{
+		"claude-code": {Skills: []string{"/review"}},
+	}
+	out := review.NormalizeRoles(in)
+	if out["claude-code"].Role != settings.RoleReviewer {
+		t.Errorf("expected RoleReviewer, got %q", out["claude-code"].Role)
+	}
+}
+
+func TestNormalizeRoles_AtMostOneFixer(t *testing.T) {
+	t.Parallel()
+	in := map[string]settings.ReviewConfig{
+		"claude-code": {Role: settings.RoleFixer},
+		"codex":       {Role: settings.RoleFixer},
+		"gemini":      {Role: settings.RoleBoth},
+	}
+	out := review.NormalizeRoles(in)
+	fixers := 0
+	for _, c := range out {
+		if c.Role.IsFixer() {
+			fixers++
+		}
+	}
+	if fixers != 1 {
+		t.Errorf("expected exactly 1 fixer, got %d", fixers)
+	}
+	if !out["claude-code"].Role.IsFixer() {
+		t.Errorf("expected claude-code (alphabetical first) to keep fixer role, got %+v", out)
+	}
+}
+
+func TestNormalizeRoles_EmptyInputReturnsFreshEmptyMap(t *testing.T) {
+	t.Parallel()
+	in := map[string]settings.ReviewConfig{}
+	out := review.NormalizeRoles(in)
+	if out == nil {
+		t.Fatal("expected non-nil empty map, got nil")
+	}
+	// Mutating `out` must not mutate `in`.
+	out["x"] = settings.ReviewConfig{Role: settings.RoleReviewer}
+	if _, leaked := in["x"]; leaked {
+		t.Errorf("NormalizeRoles returned the input map; mutations leaked")
+	}
+}
+
+func TestNormalizeRoles_SkipPreserved(t *testing.T) {
+	t.Parallel()
+	in := map[string]settings.ReviewConfig{"gemini": {Role: settings.RoleSkip}}
+	out := review.NormalizeRoles(in)
+	if out["gemini"].Role != settings.RoleSkip {
+		t.Errorf("Skip role should be preserved, got %q", out["gemini"].Role)
+	}
+}
+
+func TestReviewersOf_FiltersByRole(t *testing.T) {
+	t.Parallel()
+	s := &settings.EntireSettings{
+		Review: map[string]settings.ReviewConfig{
+			"claude-code": {Role: settings.RoleReviewer},
+			"codex":       {Role: settings.RoleFixer},
+			"gemini":      {Role: settings.RoleBoth},
+		},
+	}
+	got := review.ReviewersOf(s)
+	want := []string{"claude-code", "gemini"}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("ReviewersOf = %v, want %v", got, want)
+	}
+}
+
+func TestFixerOf_AlphabeticalWinnerWhenMultiple(t *testing.T) {
+	t.Parallel()
+	s := &settings.EntireSettings{
+		Review: map[string]settings.ReviewConfig{
+			"codex":  {Role: settings.RoleFixer},
+			"gemini": {Role: settings.RoleBoth},
+		},
+	}
+	if got := review.FixerOf(s); got != "codex" {
+		t.Errorf("FixerOf = %q, want codex (alphabetical first)", got)
+	}
+}
+
+func TestFixerOf_EmptyWhenNoFixer(t *testing.T) {
+	t.Parallel()
+	s := &settings.EntireSettings{
+		Review: map[string]settings.ReviewConfig{
+			"claude-code": {Role: settings.RoleReviewer},
+		},
+	}
+	if got := review.FixerOf(s); got != "" {
+		t.Errorf("FixerOf = %q, want empty", got)
+	}
+}