test(cmd): pin the empty-items guard in uiUnifiedSelector.SelectItem

cmd/unified_selector_guard_test.go

@@ -0,0 +1,77 @@
+package cmd
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/aaearon/grant-cli/internal/ui"
+)
+
+// TestUIUnifiedSelector_EmptyItemsGuard pins the two entry guards on
+// uiUnifiedSelector.SelectItem, including the order they run in.
+//
+// The empty-items guard used to have an equivalent in ui.SelectGroup; PR #68
+// deleted that function as dead code and its test went with it, leaving this
+// guard unpinned. Note the message is "available", which is what distinguishes
+// it from the earlier "...found, check your SCA policies" guard in
+// resolveUnifiedSelection — asserting the exact string keeps the two apart.
+//
+// Not parallel: mutates the package-global ui.IsTerminalFunc via
+// withInteractiveTTY.
+func TestUIUnifiedSelector_EmptyItemsGuard(t *testing.T) {
+	const wantEmptyMsg = "no eligible targets or groups available"
+
+	t.Run("interactive with no items returns the empty-items error", func(t *testing.T) {
+		withInteractiveTTY(t, true)
+		withDiscardedStdout(t)
+
+		selector := &uiUnifiedSelector{}
+		got, err := selector.SelectItem(nil)
+		if err == nil {
+			t.Fatalf("SelectItem(nil) returned no error (got item %+v); the len(items)==0 guard is missing", got)
+		}
+		if got != nil {
+			t.Errorf("SelectItem(nil) returned item %+v, want nil", got)
+		}
+		if err.Error() != wantEmptyMsg {
+			t.Errorf("SelectItem(nil) error = %q, want exactly %q", err.Error(), wantEmptyMsg)
+		}
+		if errors.Is(err, ui.ErrNotInteractive) {
+			t.Errorf("SelectItem(nil) in a TTY wrapped ui.ErrNotInteractive: %v", err)
+		}
+	})
+
+	t.Run("interactive with empty slice returns the empty-items error", func(t *testing.T) {
+		withInteractiveTTY(t, true)
+		withDiscardedStdout(t)
+
+		selector := &uiUnifiedSelector{}
+		_, err := selector.SelectItem([]selectionItem{})
+		if err == nil {
+			t.Fatal("SelectItem([]) returned no error; the len(items)==0 guard is missing")
+		}
+		if err.Error() != wantEmptyMsg {
+			t.Errorf("SelectItem([]) error = %q, want exactly %q", err.Error(), wantEmptyMsg)
+		}
+	})
+
+	// Guard order: the interactivity check must run first. With the guards
+	// swapped, a non-interactive caller with nothing eligible would be told
+	// "no eligible targets" instead of being pointed at the non-interactive
+	// flags, which is the wrong remedy.
+	t.Run("non-interactive with no items reports non-interactive, not empty", func(t *testing.T) {
+		withInteractiveTTY(t, false)
+
+		selector := &uiUnifiedSelector{}
+		_, err := selector.SelectItem(nil)
+		if err == nil {
+			t.Fatal("SelectItem(nil) when non-interactive returned no error")
+		}
+		if !errors.Is(err, ui.ErrNotInteractive) {
+			t.Errorf("SelectItem(nil) when non-interactive = %v, want ui.ErrNotInteractive (guards may be in the wrong order)", err)
+		}
+		if err.Error() == wantEmptyMsg {
+			t.Errorf("SelectItem(nil) when non-interactive returned the empty-items error %q; the interactivity guard must run first", err.Error())
+		}
+	})
+}

docs/mutation-ledger.md

@@ -235,7 +235,7 @@ premise does not hold).
 | UI-05 | internal/ui | `internal/ui/role_selector.go:36` | Make the role sort case-**sensitive** (drop the `strings.ToLower` normalization in the `sort.SliceStable` less-func). Note: the raw mutation orphans the `strings` import — remove it too | CONFIRMED | test | `TestBuildRoleOptions_MixedCaseSort` — the sort lives in `BuildRoleOptions`; no `sortRolesForDisplay` helper exists or was needed | PR7 | done |
 | UI-06 | internal/ui | `internal/ui/selector.go:95` | Delete the `if len(targets) == 0` guard in `SelectTarget` (`internal/ui/selector.go:49` is the identically-worded guard inside `BuildOptions`, and mutating *that* one is an **equivalent mutant** — `make([]string, 0)` + `sort.Strings` yields the same empty non-nil slice — so it is not an escape). (`SelectRole`/`SelectRequest` equivalents are **killed**; these three are not) | CONFIRMED | test | `TestSelectTarget_EmptyList`; guard **order** (non-interactive first) is pinned separately by `TestSelectTarget_NonTTYEmptyList` (mutation also orphans the `errors` import — removed so the package compiles; survey then fails with "please provide options to select from") | PR7 | done |
 | UI-07 | internal/ui | `internal/ui/session_selector.go:122` | Delete the `if len(sessions) == 0` guard in `SelectSessions` | CONFIRMED | test | `TestSelectSessions_EmptyList`; guard **order** pinned by `TestSelectSessions_NonTTYEmptyList` (the `errors` import stays live via "no sessions selected"; survey fails with "please provide options to select from") | PR7 | done |
-| UI-08 | cmd/root (unified selector) | `cmd/root.go:1005` (`uiUnifiedSelector.SelectItem`). **The original site is gone:** this was recorded against the `if len(groups) == 0` guard in `ui.SelectGroup` (`internal/ui/group_selector.go`), which PR7 (#68) deleted along with `SelectGroup` itself | Delete the `if len(items) == 0 { return nil, errors.New("no eligible targets or groups available") }` guard in `uiUnifiedSelector.SelectItem`. Note: the raw mutation orphans the `errors` import — remove it too | CONFIRMED | test | **Not pinned on `main`.** `TestSelectGroup_EmptyList` / `TestSelectGroup_NonTTYEmptyList` went with `SelectGroup` and have no successor: `TestUnifiedSelector_NonTTY` (`cmd/root_test.go:348`) covers only the non-interactive guard that precedes it, and nothing exercises `SelectItem` with an empty slice. Needs a `TestUnifiedSelector_EmptyList` plus a non-TTY sibling to pin guard **order**, mirroring UI-06/UI-07 | PR7 (#68) | todo |
+| UI-08 | cmd/root (unified selector) | `cmd/root.go:1005` (`uiUnifiedSelector.SelectItem`). **The original site is gone:** this was recorded against the `if len(groups) == 0` guard in `ui.SelectGroup` (`internal/ui/group_selector.go`), which PR7 (#68) deleted along with `SelectGroup` itself | Delete the `if len(items) == 0 { return nil, errors.New("no eligible targets or groups available") }` guard in `uiUnifiedSelector.SelectItem`. Note: the raw mutation orphans the `errors` import — remove it too | CONFIRMED | test | `TestUIUnifiedSelector_EmptyItemsGuard` (`cmd/unified_selector_guard_test.go`). Three subtests: `nil` and empty-slice inputs in a TTY assert the **exact** message `no eligible targets or groups available` — a loose substring would also match the earlier `...found, check your SCA policies` guard in `resolveUnifiedSelection` — and a non-TTY subtest asserts `errors.Is(err, ui.ErrNotInteractive)`, pinning guard **order** the way UI-06/UI-07 do. Verified by three mutations, all killed: deleting the guard, swapping the two guards, and rewording `available` | PR7 (#68) | done |
 | UI-09 | internal/ui | `internal/ui/role_selector.go` (post-`survey` index bounds check) | Disable the returned-index bounds check in `SelectRole` | CONFIRMED | wont-fix | none — defensive-only and unreachable through `survey`, which can only return a string it was given. Closed as **wont-fix in PR7**: the guard stays in place, deliberately uncovered; do not chase it | PR7 | wont-fix (closed) |
 | UI-10 | internal/ui | `internal/ui/request_selector.go` (post-`survey` index bounds check) | Disable the returned-index bounds check in `SelectRequest` | CONFIRMED | wont-fix | none — same rationale as UI-09. Closed as **wont-fix in PR7**: the guard stays in place, deliberately uncovered; do not chase it | PR7 | wont-fix (closed) |
 | COV-01 | cmd/root Execute | `cmd/root.go` (the `if shouldShowVerboseHint(...) { Fprintln(...) }` block in `Execute()`) | Delete the whole block. **Killed only on the integration leg.** `go test -count=1 ./cmd/` stays green — the package-level `Execute()` calls `os.Exit`, so only the compiled binary exercises the wiring. It fails under `-tags=integration` via `TestIntegration_ElevateWithoutLogin` and `TestIntegration_VerboseHint`. CI runs that leg unguarded on both OSes, so the call site *is* covered; the default local `make test` loop is not. Do not "simplify" CI by guarding the integration step | CONFIRMED | test | `TestIntegration_VerboseHint` + `TestIntegration_ElevateWithoutLogin` (`-tags=integration` only) | PR4 | done |
@@ -367,11 +367,12 @@ closed by review alone (disposition `refuted`: OUT-28, OUT-29, SCA-17 — OUT-27
 also REFUTED but a test was added anyway), so **179 rows require work**, of which 7
 carry a production change (OUT-26, SFU-02, SFU-07, SFU-23, CACHE-07, CFG-02, UI-02).
 
-**Status on merged `main`:** 184 rows are `done`, 2 are `wont-fix (closed)` (UI-09,
-UI-10) and 2 remain `todo` — WF-25 (form-metadata wire tags, deliberately deferred
-out of PR8; `Validator.Regex` is still unasserted anywhere in `internal/workflows`)
-and UI-08 (its guard moved to `uiUnifiedSelector.SelectItem` when PR7 deleted
-`ui.SelectGroup`, and no test succeeded `TestSelectGroup_EmptyList`).
+**Status on merged `main`:** 185 rows are `done`, 2 are `wont-fix (closed)` (UI-09,
+UI-10) and 1 remains `todo` — WF-25 (form-metadata wire tags, deliberately deferred
+out of PR8; `Validator.Regex` is still unasserted anywhere in `internal/workflows`).
+UI-08 is now `done`: its guard moved to `uiUnifiedSelector.SelectItem` when PR7
+deleted `ui.SelectGroup`, and `TestUIUnifiedSelector_EmptyItemsGuard` succeeds
+`TestSelectGroup_EmptyList`.
 
 ### Reconciliation against "145"