From 2d88d06db37ee7f9d69ee2c7b67c2623193c7e3f Mon Sep 17 00:00:00 2001
From: Tim Schindler <tim@iosharp.com>
Date: Sat, 15 Aug 2026 10:01:38 +0200
Subject: [PATCH 1/8] test(ui): pin stdin fd, group sort order and selector
 boundaries
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes the eight actionable internal/ui mutation-ledger rows (UI-01..UI-08)
and records UI-09/UI-10 as closed wont-fix.

- IsInteractive() now has a test asserting WHICH fd is probed; every previous
  stub ignored its argument, so swapping os.Stdin.Fd() for os.Stdout.Fd()
  survived — the swap that would make `grant revoke < /dev/null` hang.
- Extract sortGroupsForDisplay from SelectGroup (behaviour-preserving) so the
  display-collision ordering fix is testable without a TTY.
- Add the exactly-zero remaining-time boundary, an RFC3339Nano timestamp case,
  a mixed-case role sort fixture, and the missing empty-list guards for
  SelectTarget, SelectSessions and SelectGroup.
---
 docs/mutation-ledger.md              | 20 ++++-----
 internal/ui/group_selector.go        | 19 +++++---
 internal/ui/group_selector_test.go   | 66 ++++++++++++++++++++++++++++
 internal/ui/request_selector_test.go | 38 ++++++++++++++++
 internal/ui/role_selector_test.go    | 21 +++++++++
 internal/ui/selector_test.go         | 15 +++++++
 internal/ui/session_selector_test.go | 36 +++++++++++++++
 internal/ui/tty_test.go              | 32 +++++++++++++-
 8 files changed, 231 insertions(+), 16 deletions(-)

diff --git a/docs/mutation-ledger.md b/docs/mutation-ledger.md
index fd3e343..86f587e 100644
--- a/docs/mutation-ledger.md
+++ b/docs/mutation-ledger.md
@@ -190,16 +190,16 @@ premise does not hold).
 | CFG-07 | internal/config | `internal/config/config.go:103` | `filepath.Join(home, ".grant")` → `".grantx"` | CONFIRMED | test | `TestConfigDir_EndsInDotGrant` | PR6 | todo |
 | CFG-08 | internal/config | `internal/config/config.go:84` | `os.WriteFile(path, data, 0o600)` → `0o644` | CONFIRMED | test | `TestSave_FileMode` with the `runtime.GOOS == "windows"` skip | PR6 | todo |
 | CFG-09 | internal/config | `internal/config/config.go:75` | `if err := os.MkdirAll(dir, 0o700); err != nil { ... }` → ignore the error | CONFIRMED | test | `TestSave_MkdirAllFailure` — force it portably by pointing at a path whose parent component is an existing **regular file** (ENOTDIR / ERROR_DIRECTORY), never a hardcoded `/dev/null/...` | PR6 | todo |
-| UI-01 | internal/ui | `internal/ui/tty.go:18` | `return IsTerminalFunc(os.Stdin.Fd())` → `IsTerminalFunc(os.Stdout.Fd())`. This swap is what makes `grant revoke < /dev/null` hang in a terminal. All twelve prompt-level guards are well covered (8 spot-checked, all killed with `errors.Is` + flag hints); `IsInteractive()` itself is not, because every stub ignores `fd` | CONFIRMED | test | `TestIsInteractive_ChecksStdinFd` — a stub that records the fd and asserts `os.Stdin.Fd()` | PR7 | todo |
-| UI-02 | internal/ui | `internal/ui/group_selector.go:60` | Delete the `sort.Slice(sorted, ...)` call in the group selector | CONFIRMED | test + prod-fix | Extract `sortGroupsForDisplay` so ordering is testable without a TTY; `TestSortGroupsForDisplay_CollisionOrdering` | PR7 | todo |
-| UI-03 | internal/ui | `internal/ui/session_selector.go:57` | `if remaining <= 0 {` → `if remaining < 0 {`. The fixture only supplies `-5m` (`session_selector_test.go:154`), so exactly-zero is unpinned | CONFIRMED | test | `TestFormatSessionOption_ExactlyZeroRemaining` | PR7 | todo |
-| UI-04 | internal/ui | `internal/ui/request_selector.go:18` | Delete the `time.Parse(time.RFC3339Nano, ts)` branch in the timestamp formatter | CONFIRMED | test | `TestFormatRequestOption_RFC3339Nano` | PR7 | todo |
-| 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 | `TestSortRolesForDisplay_MixedCase` | PR7 | todo |
-| UI-06 | internal/ui | `internal/ui/selector.go:49` | Delete the `if len(targets) == 0` guard in `SelectTarget`. (`SelectRole`/`SelectRequest` equivalents are **killed**; these three are not) | CONFIRMED | test | `TestSelectTarget_EmptyList` | PR7 | todo |
-| UI-07 | internal/ui | `internal/ui/session_selector.go:112` | Delete the `if len(sessions) == 0` guard in `SelectSessions` | CONFIRMED | test | `TestSelectSessions_EmptyList` | PR7 | todo |
-| UI-08 | internal/ui | `internal/ui/group_selector.go:54` | Delete the `if len(groups) == 0` guard in `SelectGroup`. Note: the raw mutation orphans the `errors` import — remove it too | CONFIRMED | test | `TestSelectGroup_EmptyList` | PR7 | todo |
-| 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 | PR7 | todo |
-| 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 | PR7 | todo |
+| UI-01 | internal/ui | `internal/ui/tty.go:18` | `return IsTerminalFunc(os.Stdin.Fd())` → `IsTerminalFunc(os.Stdout.Fd())`. This swap is what makes `grant revoke < /dev/null` hang in a terminal. All twelve prompt-level guards are well covered (8 spot-checked, all killed with `errors.Is` + flag hints); `IsInteractive()` itself is not, because every stub ignores `fd` | CONFIRMED | test | `TestIsInteractive_ChecksStdinFd` — a stub that records the fd and asserts `os.Stdin.Fd()` | PR7 | done |
+| UI-02 | internal/ui | `internal/ui/group_selector.go:60` | Delete the `sort.Slice(sorted, ...)` call in the group selector | CONFIRMED | test + prod-fix | Extract `sortGroupsForDisplay` so ordering is testable without a TTY; `TestSortGroupsForDisplay_CollisionOrdering` | PR7 | done |
+| UI-03 | internal/ui | `internal/ui/session_selector.go:57` | `if remaining <= 0 {` → `if remaining < 0 {`. The fixture only supplies `-5m` (`session_selector_test.go:154`), so exactly-zero is unpinned | CONFIRMED | test | `TestFormatSessionOption_ExactlyZeroRemaining` | PR7 | done |
+| UI-04 | internal/ui | `internal/ui/request_selector.go:18` | Delete the `time.Parse(time.RFC3339Nano, ts)` branch in the timestamp formatter | CONFIRMED | test | `TestFormatRequestOption_RFC3339Nano` | PR7 | done |
+| 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:49` | Delete the `if len(targets) == 0` guard in `SelectTarget`. (`SelectRole`/`SelectRequest` equivalents are **killed**; these three are not) | CONFIRMED | test | `TestSelectTarget_EmptyList` (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:112` | Delete the `if len(sessions) == 0` guard in `SelectSessions` | CONFIRMED | test | `TestSelectSessions_EmptyList` (the `errors` import stays live via "no sessions selected"; survey fails with "please provide options to select from") | PR7 | done |
+| UI-08 | internal/ui | `internal/ui/group_selector.go:54` | Delete the `if len(groups) == 0` guard in `SelectGroup`. Note: the raw mutation orphans the `errors` import — remove it too | CONFIRMED | test | `TestSelectGroup_EmptyList` | PR7 | 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) |
 
 ---
 
diff --git a/internal/ui/group_selector.go b/internal/ui/group_selector.go
index 8087dba..8af6a5b 100644
--- a/internal/ui/group_selector.go
+++ b/internal/ui/group_selector.go
@@ -43,6 +43,19 @@ func FindGroupByDisplay(groups []models.GroupsEligibleTarget, display string) (*
 	return nil, fmt.Errorf("group not found: %s", display)
 }
 
+// sortGroupsForDisplay returns a copy of groups ordered by display string, leaving
+// the caller's slice untouched. SelectGroup renders its options from this copy and
+// resolves the user's answer against the same copy, so a display collision cannot
+// make the rendered list and the lookup disagree about which group a string denotes.
+func sortGroupsForDisplay(groups []models.GroupsEligibleTarget) []models.GroupsEligibleTarget {
+	sorted := make([]models.GroupsEligibleTarget, len(groups))
+	copy(sorted, groups)
+	sort.Slice(sorted, func(i, j int) bool {
+		return FormatGroupOption(sorted[i]) < FormatGroupOption(sorted[j])
+	})
+	return sorted
+}
+
 // SelectGroup presents an interactive selector for choosing a group.
 // It sorts a copy of the groups so that FindGroupByDisplay searches the same
 // ordered slice the user saw, avoiding wrong-group selection on display collisions.
@@ -55,11 +68,7 @@ func SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTa
 		return nil, errors.New("no eligible groups available")
 	}
 
-	sorted := make([]models.GroupsEligibleTarget, len(groups))
-	copy(sorted, groups)
-	sort.Slice(sorted, func(i, j int) bool {
-		return FormatGroupOption(sorted[i]) < FormatGroupOption(sorted[j])
-	})
+	sorted := sortGroupsForDisplay(groups)
 
 	options := make([]string, len(sorted))
 	for i := range sorted {
diff --git a/internal/ui/group_selector_test.go b/internal/ui/group_selector_test.go
index 578f73a..aa9c516 100644
--- a/internal/ui/group_selector_test.go
+++ b/internal/ui/group_selector_test.go
@@ -2,6 +2,7 @@ package ui
 
 import (
 	"errors"
+	"sort"
 	"strings"
 	"testing"
 
@@ -199,6 +200,71 @@ func TestFindGroupByDisplay(t *testing.T) {
 	}
 }
 
+// TestSortGroupsForDisplay_CollisionOrdering pins a previously fixed bug: SelectGroup
+// used to render options built from a sorted copy while resolving the user's answer
+// against the caller's unsorted slice. Two groups can render to the same display
+// string, so the two slices disagree about which group that string denotes and the
+// user could be elevated into a group they did not pick. survey.Select cannot be
+// driven from a test, so the ordering is asserted on the extracted helper: the slice
+// FindGroupByDisplay searches must be the same ordered slice whose options were shown.
+func TestSortGroupsForDisplay_CollisionOrdering(t *testing.T) {
+	t.Parallel()
+	// Deliberately unsorted input containing a display collision: the two
+	// "Engineering" groups have no DirectoryName, so both render identically.
+	groups := []models.GroupsEligibleTarget{
+		{DirectoryID: "dir-z", GroupID: "grp-zebra", GroupName: "Zebra Team"},
+		{DirectoryID: "dir-1", GroupID: "grp-eng-1", GroupName: "Engineering"},
+		{DirectoryID: "dir-2", GroupID: "grp-eng-2", GroupName: "Engineering"},
+		{DirectoryID: "dir-a", GroupID: "grp-alpha", GroupName: "Alpha Team"},
+	}
+
+	sorted := sortGroupsForDisplay(groups)
+
+	if len(sorted) != len(groups) {
+		t.Fatalf("sortGroupsForDisplay() length = %d, want %d", len(sorted), len(groups))
+	}
+
+	options := make([]string, len(sorted))
+	for i := range sorted {
+		options[i] = FormatGroupOption(sorted[i])
+	}
+	if !sort.StringsAreSorted(options) {
+		t.Errorf("rendered options are not in display order: %q", options)
+	}
+
+	// Every option the user could pick must resolve, inside this same slice, to a
+	// group that renders back to exactly that option.
+	for i, opt := range options {
+		got, err := FindGroupByDisplay(sorted, opt)
+		if err != nil {
+			t.Fatalf("option %d (%q) not found in the slice it was rendered from: %v", i, opt, err)
+		}
+		if back := FormatGroupOption(*got); back != opt {
+			t.Errorf("option %d (%q) resolved to a group rendering as %q", i, opt, back)
+		}
+	}
+
+	// The caller's slice must not be reordered underneath it.
+	if groups[0].GroupID != "grp-zebra" || groups[3].GroupID != "grp-alpha" {
+		t.Errorf("sortGroupsForDisplay() mutated the caller's slice: %+v", groups)
+	}
+}
+
+// Not parallel: mutates the package-global IsTerminalFunc.
+func TestSelectGroup_EmptyList(t *testing.T) {
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+	IsTerminalFunc = func(fd uintptr) bool { return true }
+
+	_, err := SelectGroup(nil)
+	if err == nil {
+		t.Fatal("expected error for empty list")
+	}
+	if !strings.Contains(err.Error(), "no eligible groups available") {
+		t.Errorf("unexpected error: %v", err)
+	}
+}
+
 // Not parallel: mutates the package-global IsTerminalFunc.
 func TestSelectGroup_NonTTY(t *testing.T) {
 	original := IsTerminalFunc
diff --git a/internal/ui/request_selector_test.go b/internal/ui/request_selector_test.go
index a4dbc6f..99d1b59 100644
--- a/internal/ui/request_selector_test.go
+++ b/internal/ui/request_selector_test.go
@@ -94,6 +94,44 @@ func TestBuildRequestOptions_SortedCorrectlyWithOffsets(t *testing.T) {
 	}
 }
 
+// TestFormatRequestOption_RFC3339Nano pins the RFC3339Nano parse branch of
+// formatSelectorTimestamp. The textual fallback below it truncates at the '.'
+// and therefore drops the timezone offset, so the two paths genuinely differ
+// for any timestamp carrying fractional seconds — which the API does emit.
+func TestFormatRequestOption_RFC3339Nano(t *testing.T) {
+	tests := []struct {
+		name      string
+		createdAt string
+		want      string
+	}{
+		{
+			name:      "fractional seconds with offset keeps the offset",
+			createdAt: "2026-04-20T10:00:00.123456789+02:00",
+			want:      "2026-04-20T10:00:00+02:00",
+		},
+		{
+			name:      "fractional seconds in UTC",
+			createdAt: "2026-04-20T10:00:00.5Z",
+			want:      "2026-04-20T10:00:00Z",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := wfmodels.AccessRequest{
+				RequestID:    "req-nano",
+				RequestState: wfmodels.RequestStatePending,
+				CreatedBy:    "user@test",
+				CreatedAt:    tt.createdAt,
+			}
+			got := FormatRequestOption(r)
+			if !strings.Contains(got, tt.want) {
+				t.Errorf("FormatRequestOption() = %q, want it to contain %q", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestSelectRequest_EmptyList(t *testing.T) {
 	orig := IsTerminalFunc
 	defer func() { IsTerminalFunc = orig }()
diff --git a/internal/ui/role_selector_test.go b/internal/ui/role_selector_test.go
index 821d5c7..21c5c1f 100644
--- a/internal/ui/role_selector_test.go
+++ b/internal/ui/role_selector_test.go
@@ -78,6 +78,27 @@ func TestSelectRole_NonInteractive(t *testing.T) {
 	}
 }
 
+// TestBuildRoleOptions_MixedCaseSort pins the strings.ToLower normalisation in the
+// role sort. A case-sensitive comparison puts every capitalised name ahead of every
+// lower-case one ("Zone" < "admin" in ASCII), scattering the list the user scans.
+// The custom-first fixture above is uniformly capitalised and cannot see that.
+func TestBuildRoleOptions_MixedCaseSort(t *testing.T) {
+	roles := []models.OnDemandResource{
+		{ResourceName: "cost-reader"},
+		{ResourceName: "Backup Operator"},
+		{ResourceName: "admin-lite"},
+		{ResourceName: "Zone Editor"},
+	}
+	_, sorted := BuildRoleOptions(roles)
+
+	want := []string{"admin-lite", "Backup Operator", "cost-reader", "Zone Editor"}
+	for i, name := range want {
+		if sorted[i].ResourceName != name {
+			t.Errorf("position %d: got %q, want %q", i, sorted[i].ResourceName, name)
+		}
+	}
+}
+
 func TestSelectRole_EmptyList(t *testing.T) {
 	orig := IsTerminalFunc
 	defer func() { IsTerminalFunc = orig }()
diff --git a/internal/ui/selector_test.go b/internal/ui/selector_test.go
index b73b7df..6cdf2e8 100644
--- a/internal/ui/selector_test.go
+++ b/internal/ui/selector_test.go
@@ -317,3 +317,18 @@ func TestFindTargetByDisplay(t *testing.T) {
 		})
 	}
 }
+
+// Not parallel: mutates the package-global IsTerminalFunc.
+func TestSelectTarget_EmptyList(t *testing.T) {
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+	IsTerminalFunc = func(fd uintptr) bool { return true }
+
+	_, err := SelectTarget(nil)
+	if err == nil {
+		t.Fatal("expected error for empty list")
+	}
+	if !strings.Contains(err.Error(), "no eligible targets available") {
+		t.Errorf("unexpected error: %v", err)
+	}
+}
diff --git a/internal/ui/session_selector_test.go b/internal/ui/session_selector_test.go
index 86f8bbb..60f336f 100644
--- a/internal/ui/session_selector_test.go
+++ b/internal/ui/session_selector_test.go
@@ -289,3 +289,39 @@ func TestConfirmRevocation_NonTTY(t *testing.T) {
 		t.Errorf("error should mention --yes, got: %v", err)
 	}
 }
+
+// TestFormatSessionOption_ExactlyZeroRemaining exercises the `remaining <= 0`
+// boundary itself. The table above only supplies a negative duration, so a
+// `<= 0` → `< 0` regression would leave a just-expired session rendering as
+// "remaining: 0m" instead of "expired".
+func TestFormatSessionOption_ExactlyZeroRemaining(t *testing.T) {
+	t.Parallel()
+	session := models.SessionInfo{
+		SessionID:       "session-rem-zero",
+		CSP:             models.CSPAzure,
+		WorkspaceID:     "/subscriptions/sub-1",
+		RoleID:          "Reader",
+		SessionDuration: 3600,
+	}
+	remainingMap := map[string]time.Duration{"session-rem-zero": 0}
+
+	want := "Reader on /subscriptions/sub-1 - expired (session: session-rem-zero)"
+	if got := FormatSessionOption(session, nil, nil, remainingMap); got != want {
+		t.Errorf("FormatSessionOption() = %q, want %q", got, want)
+	}
+}
+
+// Not parallel: mutates the package-global IsTerminalFunc.
+func TestSelectSessions_EmptyList(t *testing.T) {
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+	IsTerminalFunc = func(fd uintptr) bool { return true }
+
+	_, err := SelectSessions(nil, nil)
+	if err == nil {
+		t.Fatal("expected error for empty list")
+	}
+	if !strings.Contains(err.Error(), "no sessions available") {
+		t.Errorf("unexpected error: %v", err)
+	}
+}
diff --git a/internal/ui/tty_test.go b/internal/ui/tty_test.go
index c0c94b5..1e10808 100644
--- a/internal/ui/tty_test.go
+++ b/internal/ui/tty_test.go
@@ -1,6 +1,36 @@
 package ui
 
-import "testing"
+import (
+	"os"
+	"testing"
+)
+
+// TestIsInteractive_ChecksStdinFd pins WHICH descriptor is probed, not just the
+// boolean result. Every other stub in this package ignores its fd argument, so
+// swapping os.Stdin.Fd() for os.Stdout.Fd() survives them all — and that swap is
+// exactly what would make `grant revoke < /dev/null` in a terminal report an
+// interactive session and then hang on a prompt reading closed stdin.
+//
+// Not parallel: mutates the package-global IsTerminalFunc.
+func TestIsInteractive_ChecksStdinFd(t *testing.T) {
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+
+	var gotFDs []uintptr
+	IsTerminalFunc = func(fd uintptr) bool {
+		gotFDs = append(gotFDs, fd)
+		return true
+	}
+
+	IsInteractive()
+
+	if len(gotFDs) != 1 {
+		t.Fatalf("IsTerminalFunc called %d times, want exactly 1 (fds: %v)", len(gotFDs), gotFDs)
+	}
+	if want := os.Stdin.Fd(); gotFDs[0] != want {
+		t.Errorf("IsInteractive() probed fd %d, want stdin fd %d (stdout is %d)", gotFDs[0], want, os.Stdout.Fd())
+	}
+}
 
 // Not parallel: mutates the package-global IsTerminalFunc.
 func TestIsInteractive_WhenTerminal(t *testing.T) {

From 2725d3fc2fc4182ac5d14b541a54de1ed241ce7c Mon Sep 17 00:00:00 2001
From: Tim Schindler <tim@iosharp.com>
Date: Sat, 15 Aug 2026 11:02:06 +0200
Subject: [PATCH 2/8] fix(ui): resolve group selection by index, not display
 text

SelectGroup bound survey.Select to a string and recovered the group with
FindGroupByDisplay. Two groups with the same name in different directories
render identically, so the lookup returned the first match regardless of which
row the user highlighted - highlighting the second elevated into the first.
Sorting a copy never fixed that; it only made the wrong answer deterministic.

SelectGroup now binds an int and resolves through resolveGroupSelection,
matching SelectRole and SelectRequest. The sort becomes SliceStable so
colliding rows keep their input order.

Tests:
- TestResolveGroupSelection_DuplicateDisplayStrings pins the index path; it
  fails when the resolver is reverted to a display lookup.
- TestSortGroupsForDisplay_Ordering replaces the collision test, dropping its
  tautological FindGroupByDisplay round-trip and claiming only what it pins:
  display ordering plus a full-snapshot caller-slice immutability check.
- TestSelect{Target,Sessions,Group,Role,Request}_Non{TTY,Interactive}EmptyList
  pin the non-interactive guard ahead of the empty-list guard; all five fail
  when the guards are swapped.
- Role option/role parallelism and a length assertion; exact-match assertion
  in the request timestamp test.

Docs: CHANGELOG Fixed entry; mutation-ledger UI-02/06/08 line references
repointed and UI-02's narrative corrected to list ordering only.
---
 CHANGELOG.md                         |   1 +
 docs/mutation-ledger.md              |  11 +--
 internal/ui/group_selector.go        |  34 ++++++---
 internal/ui/group_selector_test.go   | 101 ++++++++++++++++++++++-----
 internal/ui/request_selector_test.go |  22 +++++-
 internal/ui/role_selector_test.go    |  28 +++++++-
 internal/ui/selector_test.go         |  18 +++++
 internal/ui/session_selector_test.go |  18 +++++
 8 files changed, 196 insertions(+), 37 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f5229a0..6856c7e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.
 
 - `grant favorites add` now fails immediately without a terminal instead of authenticating first
 - `grant favorites add`'s non-interactive error now mentions the required favorite name, not only the flags
+- Picking one of two identically named Entra ID groups in the interactive selector no longer elevates into the other
 
 ## [0.9.0] - 2026-08-14
 
diff --git a/docs/mutation-ledger.md b/docs/mutation-ledger.md
index 86f587e..8106d9b 100644
--- a/docs/mutation-ledger.md
+++ b/docs/mutation-ledger.md
@@ -191,13 +191,13 @@ premise does not hold).
 | CFG-08 | internal/config | `internal/config/config.go:84` | `os.WriteFile(path, data, 0o600)` → `0o644` | CONFIRMED | test | `TestSave_FileMode` with the `runtime.GOOS == "windows"` skip | PR6 | todo |
 | CFG-09 | internal/config | `internal/config/config.go:75` | `if err := os.MkdirAll(dir, 0o700); err != nil { ... }` → ignore the error | CONFIRMED | test | `TestSave_MkdirAllFailure` — force it portably by pointing at a path whose parent component is an existing **regular file** (ENOTDIR / ERROR_DIRECTORY), never a hardcoded `/dev/null/...` | PR6 | todo |
 | UI-01 | internal/ui | `internal/ui/tty.go:18` | `return IsTerminalFunc(os.Stdin.Fd())` → `IsTerminalFunc(os.Stdout.Fd())`. This swap is what makes `grant revoke < /dev/null` hang in a terminal. All twelve prompt-level guards are well covered (8 spot-checked, all killed with `errors.Is` + flag hints); `IsInteractive()` itself is not, because every stub ignores `fd` | CONFIRMED | test | `TestIsInteractive_ChecksStdinFd` — a stub that records the fd and asserts `os.Stdin.Fd()` | PR7 | done |
-| UI-02 | internal/ui | `internal/ui/group_selector.go:60` | Delete the `sort.Slice(sorted, ...)` call in the group selector | CONFIRMED | test + prod-fix | Extract `sortGroupsForDisplay` so ordering is testable without a TTY; `TestSortGroupsForDisplay_CollisionOrdering` | PR7 | done |
+| UI-02 | internal/ui | `internal/ui/group_selector.go:55` | Delete the `sort.SliceStable(sorted, ...)` call in `sortGroupsForDisplay` (orphans nothing — `sort` stays live via `BuildGroupOptions`) | CONFIRMED | test + prod-fix | Extract `sortGroupsForDisplay` so **list ordering** is testable without a TTY; `TestSortGroupsForDisplay_Ordering`. This row guards the order options are rendered in, and nothing else: sorting a copy never prevented wrong-group selection, it only made a display collision resolve consistently to the *first* matching row. Which group a selection denotes is `resolveGroupSelection`'s job (see the `SelectGroup` index fix in the production-changes table below) | PR7 | done |
 | UI-03 | internal/ui | `internal/ui/session_selector.go:57` | `if remaining <= 0 {` → `if remaining < 0 {`. The fixture only supplies `-5m` (`session_selector_test.go:154`), so exactly-zero is unpinned | CONFIRMED | test | `TestFormatSessionOption_ExactlyZeroRemaining` | PR7 | done |
 | UI-04 | internal/ui | `internal/ui/request_selector.go:18` | Delete the `time.Parse(time.RFC3339Nano, ts)` branch in the timestamp formatter | CONFIRMED | test | `TestFormatRequestOption_RFC3339Nano` | PR7 | done |
 | 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:49` | Delete the `if len(targets) == 0` guard in `SelectTarget`. (`SelectRole`/`SelectRequest` equivalents are **killed**; these three are not) | CONFIRMED | test | `TestSelectTarget_EmptyList` (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:112` | Delete the `if len(sessions) == 0` guard in `SelectSessions` | CONFIRMED | test | `TestSelectSessions_EmptyList` (the `errors` import stays live via "no sessions selected"; survey fails with "please provide options to select from") | PR7 | done |
-| UI-08 | internal/ui | `internal/ui/group_selector.go:54` | Delete the `if len(groups) == 0` guard in `SelectGroup`. Note: the raw mutation orphans the `errors` import — remove it too | CONFIRMED | test | `TestSelectGroup_EmptyList` | PR7 | done |
+| UI-06 | internal/ui | `internal/ui/selector.go:78` | Delete the `if len(targets) == 0` guard in `SelectTarget` (`: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:112` | 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 | internal/ui | `internal/ui/group_selector.go:79` | Delete the `if len(groups) == 0` guard in `SelectGroup`. Note: the raw mutation orphans the `errors` import — remove it too | CONFIRMED | test | `TestSelectGroup_EmptyList`; guard **order** pinned by `TestSelectGroup_NonTTYEmptyList` | PR7 | 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) |
 
@@ -239,7 +239,7 @@ premise does not hold).
 | `refuted` | 5 |
 | **Total** | **165** |
 
-The seven production changes, matching the plan's table:
+The seven production changes from the plan's table, plus one added by the PR7 review:
 
 | Row | Change | PR | CHANGELOG |
 |---|---|---|---|
@@ -250,6 +250,7 @@ The seven production changes, matching the plan's table:
 | CACHE-07 | `maxSessionAge` → `sessionTimestampRetention` | PR6 | no (internal) |
 | UI-02 | `sortGroupsForDisplay` extraction | PR7 | no (refactor) |
 | SFU-10 | `syncStagedFileFn` seam | PR3 | no (test seam; **not** a production gap) |
+| — | `SelectGroup` resolves the answer by **index** (`resolveGroupSelection`) instead of display text, matching `SelectRole`/`SelectRequest`. Two groups with the same name in different directories render identically, so the old text lookup returned the first match whatever the user highlighted. Not an audit row — found by the PR7 review. Pinned by `TestResolveGroupSelection_DuplicateDisplayStrings` | PR7 | `### Fixed` |
 
 ### Total CONFIRMED
 
diff --git a/internal/ui/group_selector.go b/internal/ui/group_selector.go
index 8af6a5b..04cacf8 100644
--- a/internal/ui/group_selector.go
+++ b/internal/ui/group_selector.go
@@ -33,7 +33,9 @@ func BuildGroupOptions(groups []models.GroupsEligibleTarget) []string {
 	return options
 }
 
-// FindGroupByDisplay finds a group by its formatted display string.
+// FindGroupByDisplay finds a group by its formatted display string. SelectGroup no
+// longer uses it — it resolves by index — so this has no production caller today.
+// On a display collision it returns the first match in the slice it is given.
 func FindGroupByDisplay(groups []models.GroupsEligibleTarget, display string) (*models.GroupsEligibleTarget, error) {
 	for i := range groups {
 		if FormatGroupOption(groups[i]) == display {
@@ -44,21 +46,31 @@ func FindGroupByDisplay(groups []models.GroupsEligibleTarget, display string) (*
 }
 
 // sortGroupsForDisplay returns a copy of groups ordered by display string, leaving
-// the caller's slice untouched. SelectGroup renders its options from this copy and
-// resolves the user's answer against the same copy, so a display collision cannot
-// make the rendered list and the lookup disagree about which group a string denotes.
+// the caller's slice untouched. It only fixes the order the options are rendered in;
+// which group a selection denotes is decided by index in resolveGroupSelection.
+// The sort is stable, so groups that render identically keep their input order.
 func sortGroupsForDisplay(groups []models.GroupsEligibleTarget) []models.GroupsEligibleTarget {
 	sorted := make([]models.GroupsEligibleTarget, len(groups))
 	copy(sorted, groups)
-	sort.Slice(sorted, func(i, j int) bool {
+	sort.SliceStable(sorted, func(i, j int) bool {
 		return FormatGroupOption(sorted[i]) < FormatGroupOption(sorted[j])
 	})
 	return sorted
 }
 
-// SelectGroup presents an interactive selector for choosing a group.
-// It sorts a copy of the groups so that FindGroupByDisplay searches the same
-// ordered slice the user saw, avoiding wrong-group selection on display collisions.
+// resolveGroupSelection recovers the group at the index survey returned. Resolving by
+// index rather than by display text is what makes duplicate display strings safe: the
+// same group name in two directories renders identically, and a text lookup would
+// return the first match no matter which row the user highlighted.
+func resolveGroupSelection(sorted []models.GroupsEligibleTarget, idx int) (*models.GroupsEligibleTarget, error) {
+	if idx < 0 || idx >= len(sorted) {
+		return nil, fmt.Errorf("invalid group selection index %d", idx)
+	}
+	return &sorted[idx], nil
+}
+
+// SelectGroup presents an interactive selector for choosing a group. Uses the selected
+// index (not display text) to recover the group, so duplicate display strings are safe.
 func SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTarget, error) {
 	if !IsInteractive() {
 		return nil, fmt.Errorf("%w; use --group flag for non-interactive mode", ErrNotInteractive)
@@ -75,16 +87,16 @@ func SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTa
 		options[i] = FormatGroupOption(sorted[i])
 	}
 
-	var selected string
+	var selectedIdx int
 	prompt := &survey.Select{
 		Message: "Select a group:",
 		Options: options,
 		Filter:  nil,
 	}
 
-	if err := survey.AskOne(prompt, &selected, survey.WithStdio(os.Stdin, os.Stderr, os.Stderr)); err != nil {
+	if err := survey.AskOne(prompt, &selectedIdx, survey.WithStdio(os.Stdin, os.Stderr, os.Stderr)); err != nil {
 		return nil, fmt.Errorf("group selection failed: %w", err)
 	}
 
-	return FindGroupByDisplay(sorted, selected)
+	return resolveGroupSelection(sorted, selectedIdx)
 }
diff --git a/internal/ui/group_selector_test.go b/internal/ui/group_selector_test.go
index aa9c516..fce433b 100644
--- a/internal/ui/group_selector_test.go
+++ b/internal/ui/group_selector_test.go
@@ -2,6 +2,7 @@ package ui
 
 import (
 	"errors"
+	"reflect"
 	"sort"
 	"strings"
 	"testing"
@@ -200,14 +201,12 @@ func TestFindGroupByDisplay(t *testing.T) {
 	}
 }
 
-// TestSortGroupsForDisplay_CollisionOrdering pins a previously fixed bug: SelectGroup
-// used to render options built from a sorted copy while resolving the user's answer
-// against the caller's unsorted slice. Two groups can render to the same display
-// string, so the two slices disagree about which group that string denotes and the
-// user could be elevated into a group they did not pick. survey.Select cannot be
-// driven from a test, so the ordering is asserted on the extracted helper: the slice
-// FindGroupByDisplay searches must be the same ordered slice whose options were shown.
-func TestSortGroupsForDisplay_CollisionOrdering(t *testing.T) {
+// TestSortGroupsForDisplay_Ordering pins exactly two properties of the helper, and no
+// more: the options rendered from its result are in display order, and the caller's
+// slice is not reordered underneath it. It says nothing about which group a selection
+// denotes — that is resolveGroupSelection's job and is pinned by
+// TestResolveGroupSelection_DuplicateDisplayStrings below.
+func TestSortGroupsForDisplay_Ordering(t *testing.T) {
 	t.Parallel()
 	// Deliberately unsorted input containing a display collision: the two
 	// "Engineering" groups have no DirectoryName, so both render identically.
@@ -217,6 +216,7 @@ func TestSortGroupsForDisplay_CollisionOrdering(t *testing.T) {
 		{DirectoryID: "dir-2", GroupID: "grp-eng-2", GroupName: "Engineering"},
 		{DirectoryID: "dir-a", GroupID: "grp-alpha", GroupName: "Alpha Team"},
 	}
+	before := append([]models.GroupsEligibleTarget(nil), groups...)
 
 	sorted := sortGroupsForDisplay(groups)
 
@@ -232,21 +232,65 @@ func TestSortGroupsForDisplay_CollisionOrdering(t *testing.T) {
 		t.Errorf("rendered options are not in display order: %q", options)
 	}
 
-	// Every option the user could pick must resolve, inside this same slice, to a
-	// group that renders back to exactly that option.
-	for i, opt := range options {
-		got, err := FindGroupByDisplay(sorted, opt)
+	// The caller's slice must not be reordered underneath it — full snapshot, not
+	// just the endpoints, so an in-place sort cannot hide in the middle.
+	if !reflect.DeepEqual(groups, before) {
+		t.Errorf("sortGroupsForDisplay() mutated the caller's slice:\n got %+v\nwant %+v", groups, before)
+	}
+}
+
+// TestResolveGroupSelection_DuplicateDisplayStrings pins the wrong-group fix. Two
+// groups with the same name in different directories render to the same display
+// string, so recovering the answer by text returns the first match regardless of
+// which row the user highlighted — highlight the second, elevate into the first.
+// survey.Select cannot be driven from a test, so the index path is asserted on the
+// extracted resolver, mirroring SelectRole/SelectRequest.
+func TestResolveGroupSelection_DuplicateDisplayStrings(t *testing.T) {
+	t.Parallel()
+	groups := []models.GroupsEligibleTarget{
+		{DirectoryID: "dir-1", GroupID: "grp-eng-1", GroupName: "Engineering"},
+		{DirectoryID: "dir-2", GroupID: "grp-eng-2", GroupName: "Engineering"},
+	}
+	sorted := sortGroupsForDisplay(groups)
+	if len(sorted) != 2 {
+		t.Fatalf("sortGroupsForDisplay() length = %d, want 2", len(sorted))
+	}
+	if FormatGroupOption(sorted[0]) != FormatGroupOption(sorted[1]) {
+		t.Fatalf("fixture no longer collides: %q vs %q", FormatGroupOption(sorted[0]), FormatGroupOption(sorted[1]))
+	}
+	if sorted[0].GroupID == sorted[1].GroupID {
+		t.Fatalf("fixture groups are indistinguishable: %+v", sorted)
+	}
+
+	// The sort is stable and the two entries compare equal, so the rendered order is
+	// the input order: row 0 is grp-eng-1, row 1 is grp-eng-2.
+	tests := []struct {
+		idx         int
+		wantGroupID string
+		wantDirID   string
+	}{
+		{idx: 0, wantGroupID: "grp-eng-1", wantDirID: "dir-1"},
+		{idx: 1, wantGroupID: "grp-eng-2", wantDirID: "dir-2"},
+	}
+	for _, tt := range tests {
+		got, err := resolveGroupSelection(sorted, tt.idx)
 		if err != nil {
-			t.Fatalf("option %d (%q) not found in the slice it was rendered from: %v", i, opt, err)
+			t.Fatalf("resolveGroupSelection(_, %d) error = %v", tt.idx, err)
 		}
-		if back := FormatGroupOption(*got); back != opt {
-			t.Errorf("option %d (%q) resolved to a group rendering as %q", i, opt, back)
+		if got.GroupID != tt.wantGroupID || got.DirectoryID != tt.wantDirID {
+			t.Errorf("selecting row %d returned GroupID=%q DirectoryID=%q, want %q/%q",
+				tt.idx, got.GroupID, got.DirectoryID, tt.wantGroupID, tt.wantDirID)
 		}
 	}
+}
 
-	// The caller's slice must not be reordered underneath it.
-	if groups[0].GroupID != "grp-zebra" || groups[3].GroupID != "grp-alpha" {
-		t.Errorf("sortGroupsForDisplay() mutated the caller's slice: %+v", groups)
+func TestResolveGroupSelection_OutOfRange(t *testing.T) {
+	t.Parallel()
+	groups := []models.GroupsEligibleTarget{{DirectoryID: "dir-1", GroupID: "grp1", GroupName: "Engineering"}}
+	for _, idx := range []int{-1, 1} {
+		if _, err := resolveGroupSelection(groups, idx); err == nil {
+			t.Errorf("resolveGroupSelection(_, %d) = nil error, want out-of-range error", idx)
+		}
 	}
 }
 
@@ -286,3 +330,24 @@ func TestSelectGroup_NonTTY(t *testing.T) {
 		t.Errorf("error should mention --group, got: %v", err)
 	}
 }
+
+// TestSelectGroup_NonTTYEmptyList pins the order of the two guards. _NonTTY passes a
+// non-empty list and _EmptyList forces a TTY, so their inputs never intersect and
+// swapping the guards survives both. This case satisfies both conditions at once and
+// demands the non-interactive error.
+// Not parallel: mutates the package-global IsTerminalFunc.
+// The package restores globals with defer rather than t.Cleanup — deliberate, it is
+// the convention every other test in internal/ui already follows.
+func TestSelectGroup_NonTTYEmptyList(t *testing.T) {
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+	IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	_, err := SelectGroup(nil)
+	if err == nil {
+		t.Fatal("expected error for non-TTY with an empty list")
+	}
+	if !errors.Is(err, ErrNotInteractive) {
+		t.Errorf("expected ErrNotInteractive to win over the empty-list guard, got: %v", err)
+	}
+}
diff --git a/internal/ui/request_selector_test.go b/internal/ui/request_selector_test.go
index 99d1b59..80183cb 100644
--- a/internal/ui/request_selector_test.go
+++ b/internal/ui/request_selector_test.go
@@ -78,6 +78,23 @@ func TestSelectRequest_NonInteractive(t *testing.T) {
 	}
 }
 
+// TestSelectRequest_NonInteractiveEmptyList pins the order of the two guards: the
+// non-interactive check must win when both conditions hold at once.
+// Not parallel: mutates the package-global IsTerminalFunc.
+func TestSelectRequest_NonInteractiveEmptyList(t *testing.T) {
+	orig := IsTerminalFunc
+	defer func() { IsTerminalFunc = orig }()
+	IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	_, err := SelectRequest(nil)
+	if err == nil {
+		t.Fatal("expected error for non-TTY with an empty list")
+	}
+	if !errors.Is(err, ErrNotInteractive) {
+		t.Errorf("expected ErrNotInteractive to win over the empty-list guard, got: %v", err)
+	}
+}
+
 func TestBuildRequestOptions_SortedCorrectlyWithOffsets(t *testing.T) {
 	// "2026-04-20T10:00:00+02:00" == 08:00 UTC — earlier than 09:30Z
 	// String sort would put +02:00 after Z; time sort must put 09:30Z first.
@@ -125,8 +142,9 @@ func TestFormatRequestOption_RFC3339Nano(t *testing.T) {
 				CreatedAt:    tt.createdAt,
 			}
 			got := FormatRequestOption(r)
-			if !strings.Contains(got, tt.want) {
-				t.Errorf("FormatRequestOption() = %q, want it to contain %q", got, tt.want)
+			want := "PENDING  - / -  (by user@test, " + tt.want + ")  [req-nano]"
+			if got != want {
+				t.Errorf("FormatRequestOption() = %q, want %q", got, want)
 			}
 		})
 	}
diff --git a/internal/ui/role_selector_test.go b/internal/ui/role_selector_test.go
index 21c5c1f..6eeb85f 100644
--- a/internal/ui/role_selector_test.go
+++ b/internal/ui/role_selector_test.go
@@ -78,6 +78,23 @@ func TestSelectRole_NonInteractive(t *testing.T) {
 	}
 }
 
+// TestSelectRole_NonInteractiveEmptyList pins the order of the two guards: the
+// non-interactive check must win when both conditions hold at once.
+// Not parallel: mutates the package-global IsTerminalFunc.
+func TestSelectRole_NonInteractiveEmptyList(t *testing.T) {
+	orig := IsTerminalFunc
+	defer func() { IsTerminalFunc = orig }()
+	IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	_, err := SelectRole(nil)
+	if err == nil {
+		t.Fatal("expected error for non-TTY with an empty list")
+	}
+	if !errors.Is(err, ErrNotInteractive) {
+		t.Errorf("expected ErrNotInteractive to win over the empty-list guard, got: %v", err)
+	}
+}
+
 // TestBuildRoleOptions_MixedCaseSort pins the strings.ToLower normalisation in the
 // role sort. A case-sensitive comparison puts every capitalised name ahead of every
 // lower-case one ("Zone" < "admin" in ASCII), scattering the list the user scans.
@@ -89,13 +106,22 @@ func TestBuildRoleOptions_MixedCaseSort(t *testing.T) {
 		{ResourceName: "admin-lite"},
 		{ResourceName: "Zone Editor"},
 	}
-	_, sorted := BuildRoleOptions(roles)
+	opts, sorted := BuildRoleOptions(roles)
 
 	want := []string{"admin-lite", "Backup Operator", "cost-reader", "Zone Editor"}
+	// Length first: without it a regression that shortens the slice panics on the
+	// index below instead of failing with a readable message.
+	if len(sorted) != len(want) || len(opts) != len(want) {
+		t.Fatalf("BuildRoleOptions() returned %d options / %d roles, want %d of each", len(opts), len(sorted), len(want))
+	}
 	for i, name := range want {
 		if sorted[i].ResourceName != name {
 			t.Errorf("position %d: got %q, want %q", i, sorted[i].ResourceName, name)
 		}
+		// options and roles must stay index-parallel — SelectRole resolves by index.
+		if opts[i] != FormatRoleOption(sorted[i]) {
+			t.Errorf("position %d: option %q does not render roles[%d] (%q)", i, opts[i], i, FormatRoleOption(sorted[i]))
+		}
 	}
 }
 
diff --git a/internal/ui/selector_test.go b/internal/ui/selector_test.go
index 6cdf2e8..5d1fcd2 100644
--- a/internal/ui/selector_test.go
+++ b/internal/ui/selector_test.go
@@ -237,6 +237,24 @@ func TestSelectTarget_NonTTY(t *testing.T) {
 	}
 }
 
+// TestSelectTarget_NonTTYEmptyList pins the order of the two guards. _NonTTY passes a
+// non-empty list and _EmptyList forces a TTY, so their inputs never intersect and
+// swapping the guards survives both. This case satisfies both conditions at once.
+// Not parallel: mutates the package-global IsTerminalFunc.
+func TestSelectTarget_NonTTYEmptyList(t *testing.T) {
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+	IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	_, err := SelectTarget(nil)
+	if err == nil {
+		t.Fatal("expected error for non-TTY with an empty list")
+	}
+	if !errors.Is(err, ErrNotInteractive) {
+		t.Errorf("expected ErrNotInteractive to win over the empty-list guard, got: %v", err)
+	}
+}
+
 func TestFindTargetByDisplay(t *testing.T) {
 	t.Parallel()
 	targets := []models.EligibleTarget{
diff --git a/internal/ui/session_selector_test.go b/internal/ui/session_selector_test.go
index 60f336f..43faf2b 100644
--- a/internal/ui/session_selector_test.go
+++ b/internal/ui/session_selector_test.go
@@ -272,6 +272,24 @@ func TestSelectSessions_NonTTY(t *testing.T) {
 	}
 }
 
+// TestSelectSessions_NonTTYEmptyList pins the order of the two guards. _NonTTY passes
+// a non-empty list and _EmptyList forces a TTY, so their inputs never intersect and
+// swapping the guards survives both. This case satisfies both conditions at once.
+// Not parallel: mutates the package-global IsTerminalFunc.
+func TestSelectSessions_NonTTYEmptyList(t *testing.T) {
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+	IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	_, err := SelectSessions(nil, nil)
+	if err == nil {
+		t.Fatal("expected error for non-TTY with an empty list")
+	}
+	if !errors.Is(err, ErrNotInteractive) {
+		t.Errorf("expected ErrNotInteractive to win over the empty-list guard, got: %v", err)
+	}
+}
+
 // Not parallel: mutates the package-global IsTerminalFunc.
 func TestConfirmRevocation_NonTTY(t *testing.T) {
 	original := IsTerminalFunc

From a9e01676d8c5f25ca5679cd343c51a3d442a696a Mon Sep 17 00:00:00 2001
From: Tim Schindler <tim@iosharp.com>
Date: Sat, 15 Aug 2026 11:06:37 +0200
Subject: [PATCH 3/8] fix(ui): resolve target selection by index, not display
 text

SelectTarget rendered BuildOptions(targets) - a sorted string slice - and then
recovered the answer with FindTargetByDisplay against the caller's unsorted
slice. FormatTargetOption carries no ID, so two eligible targets with the same
workspace name and role in different subscriptions or accounts render
identically; the lookup returned the first match whatever the user highlighted,
and because the rendered and searched slices were in different orders the two
could disagree even without a collision. Wrong-target elevation either way.

SelectTarget now renders from sortTargetsForDisplay and resolves through
resolveTargetSelection by index, matching SelectGroup/SelectRole/SelectRequest.
The sort is stable, so colliding rows keep their input order. Guard order is
unchanged: non-interactive still fires before the empty-list check.

SelectSessions keeps its display-text lookup and gains a comment saying why:
every option string embeds the session ID, so collisions are unreachable.

Tests:
- TestResolveTargetSelection_DuplicateDisplayStrings pins the index path; it
  fails when the resolver is reverted to a display lookup.
- TestResolveTargetSelection_OutOfRange mirrors the group equivalent.
- TestSortTargetsForDisplay_Ordering pins display ordering, stable ordering of
  colliding rows, and caller-slice immutability.

Docs: CHANGELOG Fixed entry; mutation-ledger production-changes row (not an
audit row - found by the PR7 review), UI-06 line reference repointed, and both
FindTargetByDisplay and FindGroupByDisplay noted as deletion candidates.
---
 CHANGELOG.md                    |  1 +
 docs/mutation-ledger.md         |  9 ++-
 internal/ui/selector.go         | 45 +++++++++++++--
 internal/ui/selector_test.go    | 99 +++++++++++++++++++++++++++++++++
 internal/ui/session_selector.go |  4 ++
 5 files changed, 150 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6856c7e..8ddc366 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file.
 - `grant favorites add` now fails immediately without a terminal instead of authenticating first
 - `grant favorites add`'s non-interactive error now mentions the required favorite name, not only the flags
 - Picking one of two identically named Entra ID groups in the interactive selector no longer elevates into the other
+- The interactive target selector now elevates the row you picked, not another target that renders the same way
 
 ## [0.9.0] - 2026-08-14
 
diff --git a/docs/mutation-ledger.md b/docs/mutation-ledger.md
index 8106d9b..847f8d3 100644
--- a/docs/mutation-ledger.md
+++ b/docs/mutation-ledger.md
@@ -195,7 +195,7 @@ premise does not hold).
 | UI-03 | internal/ui | `internal/ui/session_selector.go:57` | `if remaining <= 0 {` → `if remaining < 0 {`. The fixture only supplies `-5m` (`session_selector_test.go:154`), so exactly-zero is unpinned | CONFIRMED | test | `TestFormatSessionOption_ExactlyZeroRemaining` | PR7 | done |
 | UI-04 | internal/ui | `internal/ui/request_selector.go:18` | Delete the `time.Parse(time.RFC3339Nano, ts)` branch in the timestamp formatter | CONFIRMED | test | `TestFormatRequestOption_RFC3339Nano` | PR7 | done |
 | 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:78` | Delete the `if len(targets) == 0` guard in `SelectTarget` (`: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-06 | internal/ui | `internal/ui/selector.go:107` | Delete the `if len(targets) == 0` guard in `SelectTarget` (`: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:112` | 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 | internal/ui | `internal/ui/group_selector.go:79` | Delete the `if len(groups) == 0` guard in `SelectGroup`. Note: the raw mutation orphans the `errors` import — remove it too | CONFIRMED | test | `TestSelectGroup_EmptyList`; guard **order** pinned by `TestSelectGroup_NonTTYEmptyList` | PR7 | 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) |
@@ -239,7 +239,7 @@ premise does not hold).
 | `refuted` | 5 |
 | **Total** | **165** |
 
-The seven production changes from the plan's table, plus one added by the PR7 review:
+The seven production changes from the plan's table, plus two added by the PR7 review:
 
 | Row | Change | PR | CHANGELOG |
 |---|---|---|---|
@@ -251,6 +251,11 @@ The seven production changes from the plan's table, plus one added by the PR7 re
 | UI-02 | `sortGroupsForDisplay` extraction | PR7 | no (refactor) |
 | SFU-10 | `syncStagedFileFn` seam | PR3 | no (test seam; **not** a production gap) |
 | — | `SelectGroup` resolves the answer by **index** (`resolveGroupSelection`) instead of display text, matching `SelectRole`/`SelectRequest`. Two groups with the same name in different directories render identically, so the old text lookup returned the first match whatever the user highlighted. Not an audit row — found by the PR7 review. Pinned by `TestResolveGroupSelection_DuplicateDisplayStrings` | PR7 | `### Fixed` |
+| — | `SelectTarget` resolves the answer by **index** (`resolveTargetSelection`) against the same sorted copy it renders (`sortTargetsForDisplay`), instead of looking the display text up in the caller's **unsorted** slice. `FormatTargetOption` carries no ID, so two targets with the same workspace name and role in different subscriptions/accounts render identically; worse than the group case, because the rendered and searched slices were in different orders. Not an audit row — found by the PR7 review. Pinned by `TestResolveTargetSelection_DuplicateDisplayStrings` and `TestSortTargetsForDisplay_Ordering`. `SelectSessions` still resolves by text and is deliberately left alone — its option strings embed the session ID, so collisions are unreachable | PR7 | `### Fixed` |
+
+`FindGroupByDisplay` and `FindTargetByDisplay` are exported and still covered by tests,
+but neither has a production caller after the two index fixes. Both are follow-up
+deletion candidates; they were kept here rather than removed in a fix commit.
 
 ### Total CONFIRMED
 
diff --git a/internal/ui/selector.go b/internal/ui/selector.go
index b124b96..14f9157 100644
--- a/internal/ui/selector.go
+++ b/internal/ui/selector.go
@@ -59,7 +59,9 @@ func BuildOptions(targets []models.EligibleTarget) []string {
 	return options
 }
 
-// FindTargetByDisplay finds a target by its formatted display string.
+// FindTargetByDisplay finds a target by its formatted display string. SelectTarget no
+// longer uses it — it resolves by index — so this has no production caller today.
+// On a display collision it returns the first match in the slice it is given.
 func FindTargetByDisplay(targets []models.EligibleTarget, display string) (*models.EligibleTarget, error) {
 	for i := range targets {
 		if FormatTargetOption(targets[i]) == display {
@@ -69,7 +71,34 @@ func FindTargetByDisplay(targets []models.EligibleTarget, display string) (*mode
 	return nil, fmt.Errorf("target not found: %s", display)
 }
 
-// SelectTarget presents an interactive selector for choosing a target.
+// sortTargetsForDisplay returns a copy of targets ordered by display string, leaving
+// the caller's slice untouched. It only fixes the order the options are rendered in;
+// which target a selection denotes is decided by index in resolveTargetSelection.
+// The sort is stable, so targets that render identically keep their input order.
+func sortTargetsForDisplay(targets []models.EligibleTarget) []models.EligibleTarget {
+	sorted := make([]models.EligibleTarget, len(targets))
+	copy(sorted, targets)
+	sort.SliceStable(sorted, func(i, j int) bool {
+		return FormatTargetOption(sorted[i]) < FormatTargetOption(sorted[j])
+	})
+	return sorted
+}
+
+// resolveTargetSelection recovers the target at the index survey returned. Resolving
+// by index rather than by display text is what makes duplicate display strings safe:
+// FormatTargetOption carries no ID, so the same workspace name and role in two
+// subscriptions or accounts renders identically, and a text lookup would return the
+// first match no matter which row the user highlighted.
+func resolveTargetSelection(sorted []models.EligibleTarget, idx int) (*models.EligibleTarget, error) {
+	if idx < 0 || idx >= len(sorted) {
+		return nil, fmt.Errorf("invalid target selection index %d", idx)
+	}
+	return &sorted[idx], nil
+}
+
+// SelectTarget presents an interactive selector for choosing a target. Uses the
+// selected index (not display text) to recover the target, so duplicate display
+// strings are safe, and renders from the same sorted copy it resolves against.
 func SelectTarget(targets []models.EligibleTarget) (*models.EligibleTarget, error) {
 	if !IsInteractive() {
 		return nil, fmt.Errorf("%w; use --target and --role flags for non-interactive mode", ErrNotInteractive)
@@ -79,18 +108,22 @@ func SelectTarget(targets []models.EligibleTarget) (*models.EligibleTarget, erro
 		return nil, errors.New("no eligible targets available")
 	}
 
-	options := BuildOptions(targets)
+	sorted := sortTargetsForDisplay(targets)
+	options := make([]string, len(sorted))
+	for i := range sorted {
+		options[i] = FormatTargetOption(sorted[i])
+	}
 
-	var selected string
+	var selectedIdx int
 	prompt := &survey.Select{
 		Message: "Select a target:",
 		Options: options,
 		Filter:  nil, // Enable default fuzzy filter
 	}
 
-	if err := survey.AskOne(prompt, &selected, survey.WithStdio(os.Stdin, os.Stderr, os.Stderr)); err != nil {
+	if err := survey.AskOne(prompt, &selectedIdx, survey.WithStdio(os.Stdin, os.Stderr, os.Stderr)); err != nil {
 		return nil, fmt.Errorf("target selection failed: %w", err)
 	}
 
-	return FindTargetByDisplay(targets, selected)
+	return resolveTargetSelection(sorted, selectedIdx)
 }
diff --git a/internal/ui/selector_test.go b/internal/ui/selector_test.go
index 5d1fcd2..77831f5 100644
--- a/internal/ui/selector_test.go
+++ b/internal/ui/selector_test.go
@@ -2,6 +2,8 @@ package ui
 
 import (
 	"errors"
+	"reflect"
+	"sort"
 	"strings"
 	"testing"
 
@@ -336,6 +338,103 @@ func TestFindTargetByDisplay(t *testing.T) {
 	}
 }
 
+// TestSortTargetsForDisplay_Ordering pins the order options are rendered in, and
+// nothing else. Which target a selection denotes is resolveTargetSelection's job.
+func TestSortTargetsForDisplay_Ordering(t *testing.T) {
+	t.Parallel()
+	// Deliberately unsorted input containing a display collision: the two "Shared"
+	// subscriptions carry the same workspace name and role, so both render identically.
+	targets := []models.EligibleTarget{
+		{OrganizationID: "org-z", WorkspaceID: "sub-zebra", WorkspaceName: "Zebra Sub", WorkspaceType: models.WorkspaceTypeSubscription, RoleInfo: models.RoleInfo{ID: "r0", Name: "Reader"}},
+		{OrganizationID: "org-1", WorkspaceID: "sub-shared-1", WorkspaceName: "Shared", WorkspaceType: models.WorkspaceTypeSubscription, RoleInfo: models.RoleInfo{ID: "r1", Name: "Owner"}},
+		{OrganizationID: "org-2", WorkspaceID: "sub-shared-2", WorkspaceName: "Shared", WorkspaceType: models.WorkspaceTypeSubscription, RoleInfo: models.RoleInfo{ID: "r1", Name: "Owner"}},
+		{OrganizationID: "org-a", WorkspaceID: "sub-alpha", WorkspaceName: "Alpha Sub", WorkspaceType: models.WorkspaceTypeSubscription, RoleInfo: models.RoleInfo{ID: "r2", Name: "Contributor"}},
+	}
+	before := append([]models.EligibleTarget(nil), targets...)
+
+	sorted := sortTargetsForDisplay(targets)
+
+	if len(sorted) != len(targets) {
+		t.Fatalf("sortTargetsForDisplay() length = %d, want %d", len(sorted), len(targets))
+	}
+
+	options := make([]string, len(sorted))
+	for i := range sorted {
+		options[i] = FormatTargetOption(sorted[i])
+	}
+	if !sort.StringsAreSorted(options) {
+		t.Errorf("rendered options are not in display order: %q", options)
+	}
+
+	// The sort is stable, so the two colliding rows keep their input order.
+	if sorted[1].WorkspaceID != "sub-shared-1" || sorted[2].WorkspaceID != "sub-shared-2" {
+		t.Errorf("colliding rows lost input order: got %q, %q", sorted[1].WorkspaceID, sorted[2].WorkspaceID)
+	}
+
+	if !reflect.DeepEqual(targets, before) {
+		t.Errorf("sortTargetsForDisplay() mutated the caller's slice: got %+v, want %+v", targets, before)
+	}
+}
+
+// TestResolveTargetSelection_DuplicateDisplayStrings pins the wrong-target fix.
+// FormatTargetOption carries no ID, so two eligible targets with the same workspace
+// name and role in different subscriptions/accounts render to the same string.
+// Recovering the answer by text returns the first match regardless of which row the
+// user highlighted — and SelectTarget searched the caller's *unsorted* slice while
+// rendering a sorted one, so the two could disagree even without a collision.
+// survey.Select cannot be driven from a test, so the index path is asserted on the
+// extracted resolver, mirroring SelectGroup/SelectRole/SelectRequest.
+func TestResolveTargetSelection_DuplicateDisplayStrings(t *testing.T) {
+	t.Parallel()
+	targets := []models.EligibleTarget{
+		{OrganizationID: "org-1", WorkspaceID: "sub-1", WorkspaceName: "Shared", WorkspaceType: models.WorkspaceTypeSubscription, RoleInfo: models.RoleInfo{ID: "role-1", Name: "Owner"}},
+		{OrganizationID: "org-2", WorkspaceID: "sub-2", WorkspaceName: "Shared", WorkspaceType: models.WorkspaceTypeSubscription, RoleInfo: models.RoleInfo{ID: "role-1", Name: "Owner"}},
+	}
+	sorted := sortTargetsForDisplay(targets)
+	if len(sorted) != 2 {
+		t.Fatalf("sortTargetsForDisplay() length = %d, want 2", len(sorted))
+	}
+	if FormatTargetOption(sorted[0]) != FormatTargetOption(sorted[1]) {
+		t.Fatalf("fixture no longer collides: %q vs %q", FormatTargetOption(sorted[0]), FormatTargetOption(sorted[1]))
+	}
+	if sorted[0].WorkspaceID == sorted[1].WorkspaceID {
+		t.Fatalf("fixture targets are indistinguishable: %+v", sorted)
+	}
+
+	// The sort is stable and the two entries compare equal, so the rendered order is
+	// the input order: row 0 is sub-1, row 1 is sub-2.
+	tests := []struct {
+		idx             int
+		wantWorkspaceID string
+		wantOrgID       string
+	}{
+		{idx: 0, wantWorkspaceID: "sub-1", wantOrgID: "org-1"},
+		{idx: 1, wantWorkspaceID: "sub-2", wantOrgID: "org-2"},
+	}
+	for _, tt := range tests {
+		got, err := resolveTargetSelection(sorted, tt.idx)
+		if err != nil {
+			t.Fatalf("resolveTargetSelection(_, %d) error = %v", tt.idx, err)
+		}
+		if got.WorkspaceID != tt.wantWorkspaceID || got.OrganizationID != tt.wantOrgID {
+			t.Errorf("selecting row %d returned WorkspaceID=%q OrganizationID=%q, want %q/%q",
+				tt.idx, got.WorkspaceID, got.OrganizationID, tt.wantWorkspaceID, tt.wantOrgID)
+		}
+	}
+}
+
+func TestResolveTargetSelection_OutOfRange(t *testing.T) {
+	t.Parallel()
+	targets := []models.EligibleTarget{
+		{WorkspaceID: "sub-1", WorkspaceName: "Shared", WorkspaceType: models.WorkspaceTypeSubscription, RoleInfo: models.RoleInfo{Name: "Owner"}},
+	}
+	for _, idx := range []int{-1, 1} {
+		if _, err := resolveTargetSelection(targets, idx); err == nil {
+			t.Errorf("resolveTargetSelection(_, %d) = nil error, want out-of-range error", idx)
+		}
+	}
+}
+
 // Not parallel: mutates the package-global IsTerminalFunc.
 func TestSelectTarget_EmptyList(t *testing.T) {
 	original := IsTerminalFunc
diff --git a/internal/ui/session_selector.go b/internal/ui/session_selector.go
index 89f16c6..5002a6f 100644
--- a/internal/ui/session_selector.go
+++ b/internal/ui/session_selector.go
@@ -104,6 +104,10 @@ func FindSessionByDisplay(
 }
 
 // SelectSessions presents a multi-select prompt for choosing sessions to revoke.
+// It resolves the answer by display text rather than by index, unlike the target,
+// group, role and request selectors. That is safe here and deliberately left alone:
+// every option string embeds session.SessionID, so two sessions can never render
+// identically and FindSessionByDisplay cannot resolve to the wrong row.
 func SelectSessions(sessions []models.SessionInfo, nameMap map[string]string) ([]models.SessionInfo, error) {
 	if !IsInteractive() {
 		return nil, fmt.Errorf("%w; use --all or provide session IDs as arguments", ErrNotInteractive)

From 64481c65571013f1c1e9c96683aabc012e74581d Mon Sep 17 00:00:00 2001
From: Tim Schindler <tim@iosharp.com>
Date: Sun, 16 Aug 2026 09:21:52 +0200
Subject: [PATCH 4/8] fix(cmd): resolve unified selection by index, not display
 text
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The unified selector behind plain `grant`, `grant --groups` and
`grant favorites add` still bound survey.Select to a string and looked the
answer back up with findItemByDisplay. formatSelectionItem carries no ID, so
two Entra ID groups with the same name in different directories render
identically and the lookup returned the first match regardless of which row the
user highlighted — elevating into the wrong group.

Bind the selected index instead and recover the item with a bounds-checked
resolveSelectionItem, mirroring resolveGroupSelection/resolveTargetSelection.
Out-of-range indexes error rather than clamp. buildUnifiedOptions now sorts
stably so the options slice and the sorted items slice stay index-aligned.
---
 cmd/root.go           | 13 +++++---
 cmd/selection.go      | 21 +++++++-----
 cmd/selection_test.go | 78 +++++++++++++++++++++++++++----------------
 3 files changed, 71 insertions(+), 41 deletions(-)

diff --git a/cmd/root.go b/cmd/root.go
index 729903f..1abd49f 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -971,7 +971,12 @@ func (s *uiSelector) SelectTarget(targets []models.EligibleTarget) (*models.Elig
 	return ui.SelectTarget(targets)
 }
 
-// uiUnifiedSelector implements unifiedSelector using survey.Select
+// uiUnifiedSelector implements unifiedSelector using survey.Select. This is the
+// selector plain `grant`, `grant --groups` and `grant favorites add` all reach.
+// It resolves the answer by the selected index rather than by display text, and
+// renders from the same sorted slice it resolves against, so duplicate display
+// strings — two Entra ID groups with the same name in different directories, say —
+// cannot elevate into the wrong one.
 type uiUnifiedSelector struct{}
 
 func (s *uiUnifiedSelector) SelectItem(items []selectionItem) (*selectionItem, error) {
@@ -985,16 +990,16 @@ func (s *uiUnifiedSelector) SelectItem(items []selectionItem) (*selectionItem, e
 
 	options, sorted := buildUnifiedOptions(items)
 
-	var selected string
+	var selectedIdx int
 	prompt := &survey.Select{
 		Message: "Select a target:",
 		Options: options,
 		Filter:  nil,
 	}
 
-	if err := survey.AskOne(prompt, &selected, survey.WithStdio(os.Stdin, os.Stderr, os.Stderr)); err != nil {
+	if err := survey.AskOne(prompt, &selectedIdx, survey.WithStdio(os.Stdin, os.Stderr, os.Stderr)); err != nil {
 		return nil, fmt.Errorf("selection failed: %w", err)
 	}
 
-	return findItemByDisplay(sorted, selected)
+	return resolveSelectionItem(sorted, selectedIdx)
 }
diff --git a/cmd/selection.go b/cmd/selection.go
index 2ad7d81..b0533db 100644
--- a/cmd/selection.go
+++ b/cmd/selection.go
@@ -57,7 +57,9 @@ func buildUnifiedOptions(items []selectionItem) ([]string, []selectionItem) {
 		pairs[i] = indexed{display: formatSelectionItem(item), item: item}
 	}
 
-	sort.Slice(pairs, func(i, j int) bool {
+	// Stable so that items which render identically keep their input order, and the
+	// options slice and the sorted items slice stay index-for-index aligned.
+	sort.SliceStable(pairs, func(i, j int) bool {
 		return pairs[i].display < pairs[j].display
 	})
 
@@ -71,12 +73,15 @@ func buildUnifiedOptions(items []selectionItem) ([]string, []selectionItem) {
 	return options, sorted
 }
 
-// findItemByDisplay finds a selectionItem by its formatted display string.
-func findItemByDisplay(items []selectionItem, display string) (*selectionItem, error) {
-	for i := range items {
-		if formatSelectionItem(items[i]) == display {
-			return &items[i], nil
-		}
+// resolveSelectionItem recovers the item at the index survey returned. Resolving by
+// index rather than by display text is what makes duplicate display strings safe:
+// formatSelectionItem carries no ID, so the same group name in two directories — or
+// the same workspace name and role in two subscriptions — renders identically, and a
+// text lookup would return the first match no matter which row the user highlighted.
+// Out-of-range indexes are an error, never clamped: guessing a row is the bug.
+func resolveSelectionItem(sorted []selectionItem, idx int) (*selectionItem, error) {
+	if idx < 0 || idx >= len(sorted) {
+		return nil, fmt.Errorf("invalid selection index %d", idx)
 	}
-	return nil, fmt.Errorf("item not found: %s", display)
+	return &sorted[idx], nil
 }
diff --git a/cmd/selection_test.go b/cmd/selection_test.go
index 554c79b..a7c1986 100644
--- a/cmd/selection_test.go
+++ b/cmd/selection_test.go
@@ -145,59 +145,79 @@ func TestBuildUnifiedOptions(t *testing.T) {
 	}
 }
 
-func TestFindItemByDisplay(t *testing.T) {
-	cloudTarget := &scamodels.EligibleTarget{
+func TestResolveSelectionItem(t *testing.T) {
+	first := &scamodels.GroupsEligibleTarget{
+		DirectoryID: "dir-first",
+		GroupID:     "group-first",
+		GroupName:   "Cloud Admins",
+	}
+	second := &scamodels.GroupsEligibleTarget{
+		DirectoryID: "dir-second",
+		GroupID:     "group-second",
+		GroupName:   "Cloud Admins",
+	}
+	cloud := &scamodels.EligibleTarget{
+		WorkspaceID:   "sub-1",
 		WorkspaceName: "Prod-EastUS",
 		WorkspaceType: scamodels.WorkspaceTypeSubscription,
 		RoleInfo:      scamodels.RoleInfo{Name: "Contributor"},
 	}
-	groupTarget := &scamodels.GroupsEligibleTarget{
-		DirectoryName: "Contoso",
-		GroupName:     "Engineering",
-	}
 
+	// The two groups render identically, which is exactly the case a display-string
+	// lookup gets wrong.
 	items := []selectionItem{
-		{kind: selectionCloud, cloud: cloudTarget},
-		{kind: selectionGroup, group: groupTarget},
+		{kind: selectionGroup, group: first},
+		{kind: selectionGroup, group: second},
+		{kind: selectionCloud, cloud: cloud},
 	}
 
 	tests := []struct {
 		name    string
-		display string
+		idx     int
 		wantErr bool
+		wantID  string
 	}{
-		{
-			name:    "finds cloud by display",
-			display: "Subscription: Prod-EastUS / Role: Contributor",
-			wantErr: false,
-		},
-		{
-			name:    "finds group by display",
-			display: "Directory: Contoso / Group: Engineering (azure)",
-			wantErr: false,
-		},
-		{
-			name:    "returns error on mismatch",
-			display: "NonExistent Display String",
-			wantErr: true,
-		},
+		{name: "first of two identical displays", idx: 0, wantID: "group-first"},
+		{name: "second of two identical displays", idx: 1, wantID: "group-second"},
+		{name: "cloud item", idx: 2, wantID: "sub-1"},
+		{name: "negative index errors", idx: -1, wantErr: true},
+		{name: "index past end errors", idx: 3, wantErr: true},
+		{name: "far out of range errors", idx: 99, wantErr: true},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			item, err := findItemByDisplay(items, tt.display)
+			item, err := resolveSelectionItem(items, tt.idx)
 			if tt.wantErr {
 				if err == nil {
-					t.Error("expected error but got none")
+					t.Fatalf("resolveSelectionItem(%d) = %v, want error", tt.idx, item)
+				}
+				if item != nil {
+					t.Errorf("resolveSelectionItem(%d) item = %v, want nil", tt.idx, item)
 				}
 				return
 			}
 			if err != nil {
-				t.Errorf("unexpected error: %v", err)
+				t.Fatalf("unexpected error: %v", err)
+			}
+			var gotID string
+			switch item.kind {
+			case selectionGroup:
+				gotID = item.group.GroupID
+			case selectionCloud:
+				gotID = item.cloud.WorkspaceID
 			}
-			if item == nil {
-				t.Fatal("expected non-nil item")
+			if gotID != tt.wantID {
+				t.Errorf("resolveSelectionItem(%d) id = %q, want %q", tt.idx, gotID, tt.wantID)
 			}
 		})
 	}
 }
+
+// TestResolveSelectionItem_EmptySlice guards the degenerate case: with nothing to
+// select from, every index must be rejected rather than panicking.
+func TestResolveSelectionItem_EmptySlice(t *testing.T) {
+	if item, err := resolveSelectionItem(nil, 0); err == nil {
+		t.Errorf("resolveSelectionItem(nil, 0) = %v, want error", item)
+	}
+}

From d52f3d1f3e86c9a4e48d640ad04a78207dd3d350 Mon Sep 17 00:00:00 2001
From: Tim Schindler <tim@iosharp.com>
Date: Sun, 16 Aug 2026 09:21:59 +0200
Subject: [PATCH 5/8] test(ui): pin the selector wiring with pty-driven tests

The index fixes were only pinned by unit tests over the resolve* helpers, so
reverting SelectTarget/SelectGroup/SelectItem to display-string resolution left
the whole suite green. These tests drive the real survey prompt over a real
pseudo-terminal and assert the highlighted row is the one that comes back,
including after the user types a filter.

The pty is allocated with raw /dev/ptmx plus the TIOCSPTLCK/TIOCGPTN ioctls so
no new Go module dependency is needed. Linux-only by file name; CI's Windows
leg never compiles these files.
---
 cmd/pty_linux_test.go                  | 147 +++++++++++++++++++++++++
 cmd/selection_pty_linux_test.go        | 109 ++++++++++++++++++
 internal/ui/pty_linux_test.go          | 145 ++++++++++++++++++++++++
 internal/ui/selector_pty_linux_test.go |  98 +++++++++++++++++
 4 files changed, 499 insertions(+)
 create mode 100644 cmd/pty_linux_test.go
 create mode 100644 cmd/selection_pty_linux_test.go
 create mode 100644 internal/ui/pty_linux_test.go
 create mode 100644 internal/ui/selector_pty_linux_test.go

diff --git a/cmd/pty_linux_test.go b/cmd/pty_linux_test.go
new file mode 100644
index 0000000..612e600
--- /dev/null
+++ b/cmd/pty_linux_test.go
@@ -0,0 +1,147 @@
+package cmd
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+	"sync"
+	"syscall"
+	"testing"
+	"time"
+	"unsafe"
+
+	"github.com/aaearon/grant-cli/internal/ui"
+)
+
+// ptySession drives a survey prompt over a real pseudo-terminal.
+//
+// survey refuses to render unless its input is a tty and it can put that tty into
+// raw mode, so the only way to exercise the real SelectTarget/SelectGroup wiring —
+// as opposed to the pure resolve* helpers — is to give it one. This uses the raw
+// /dev/ptmx + TIOCSPTLCK/TIOCGPTN ioctls rather than a pty module so the repo keeps
+// its zero-new-dependency goal. Linux-only by file name; the ioctl constants do not
+// exist on other platforms and CI's Windows leg simply never compiles this file.
+type ptySession struct {
+	master *os.File
+	slave  *os.File
+
+	mu  sync.Mutex
+	out bytes.Buffer
+}
+
+// newPTYSession allocates a pty, points os.Stdin and os.Stderr at the slave and
+// starts draining the master. Everything is restored via t.Cleanup.
+//
+// Not parallel-safe: it mutates os.Stdin/os.Stderr. Callers must not call t.Parallel().
+func newPTYSession(t *testing.T) *ptySession {
+	t.Helper()
+
+	master, err := os.OpenFile("/dev/ptmx", os.O_RDWR, 0)
+	if err != nil {
+		t.Skipf("cannot open /dev/ptmx: %v", err)
+	}
+
+	// unlockpt(3): TIOCSPTLCK with a zero value.
+	var unlock int32
+	if _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, master.Fd(),
+		syscall.TIOCSPTLCK, uintptr(unsafe.Pointer(&unlock))); errno != 0 {
+		_ = master.Close()
+		t.Skipf("TIOCSPTLCK failed: %v", errno)
+	}
+
+	// ptsname(3): TIOCGPTN yields the slave index.
+	var ptn uint32
+	if _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, master.Fd(),
+		syscall.TIOCGPTN, uintptr(unsafe.Pointer(&ptn))); errno != 0 {
+		_ = master.Close()
+		t.Skipf("TIOCGPTN failed: %v", errno)
+	}
+
+	slave, err := os.OpenFile(fmt.Sprintf("/dev/pts/%d", ptn), os.O_RDWR|syscall.O_NOCTTY, 0)
+	if err != nil {
+		_ = master.Close()
+		t.Skipf("cannot open pty slave: %v", err)
+	}
+
+	p := &ptySession{master: master, slave: slave}
+
+	origStdin, origStderr := os.Stdin, os.Stderr
+	os.Stdin, os.Stderr = slave, slave
+
+	drained := make(chan struct{})
+	go func() {
+		defer close(drained)
+		buf := make([]byte, 4096)
+		for {
+			n, err := master.Read(buf)
+			if n > 0 {
+				p.mu.Lock()
+				p.out.Write(buf[:n])
+				p.mu.Unlock()
+			}
+			if err != nil {
+				return
+			}
+		}
+	}()
+
+	t.Cleanup(func() {
+		os.Stdin, os.Stderr = origStdin, origStderr
+		_ = slave.Close()
+		_ = master.Close()
+		select {
+		case <-drained:
+		case <-time.After(2 * time.Second):
+		}
+	})
+
+	return p
+}
+
+// screen returns everything the prompt has written so far.
+func (p *ptySession) screen() string {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	return p.out.String()
+}
+
+// waitFor blocks until the prompt has rendered want. Waiting for the prompt text is
+// what guarantees survey has already switched the tty into raw mode, so the keys sent
+// afterwards are not mangled by the canonical line discipline.
+func (p *ptySession) waitFor(t *testing.T, want string) {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for time.Now().Before(deadline) {
+		if strings.Contains(p.screen(), want) {
+			return
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+	t.Fatalf("timed out waiting for %q; screen so far:\n%s", want, p.screen())
+}
+
+// send writes keys in a single Write so multi-byte escape sequences cannot be split.
+func (p *ptySession) send(t *testing.T, keys string) {
+	t.Helper()
+	if _, err := p.master.Write([]byte(keys)); err != nil && !errors.Is(err, io.EOF) {
+		t.Fatalf("writing %q to pty: %v", keys, err)
+	}
+}
+
+const (
+	keyDown  = "\x1b[B"
+	keyEnter = "\r"
+)
+
+// forceInteractive makes ui.IsInteractive() report true for the duration of the test.
+//
+// Not parallel: mutates the package-global ui.IsTerminalFunc.
+func forceInteractive(t *testing.T) {
+	t.Helper()
+	orig := ui.IsTerminalFunc
+	ui.IsTerminalFunc = func(uintptr) bool { return true }
+	t.Cleanup(func() { ui.IsTerminalFunc = orig })
+}
diff --git a/cmd/selection_pty_linux_test.go b/cmd/selection_pty_linux_test.go
new file mode 100644
index 0000000..b2f176e
--- /dev/null
+++ b/cmd/selection_pty_linux_test.go
@@ -0,0 +1,109 @@
+package cmd
+
+import (
+	"testing"
+
+	scamodels "github.com/aaearon/grant-cli/internal/sca/models"
+)
+
+// duplicateGroupItems returns three selection items, the first two of which are
+// distinct Entra ID groups that render identically: FormatGroupOption falls back to
+// "Group: <name>" when no directory name has been cross-referenced, so the same group
+// name in two different directories is indistinguishable on screen.
+func duplicateGroupItems() []selectionItem {
+	first := &scamodels.GroupsEligibleTarget{
+		DirectoryID: "dir-first",
+		GroupID:     "group-first",
+		GroupName:   "Cloud Admins",
+	}
+	second := &scamodels.GroupsEligibleTarget{
+		DirectoryID: "dir-second",
+		GroupID:     "group-second",
+		GroupName:   "Cloud Admins",
+	}
+	zebra := &scamodels.GroupsEligibleTarget{
+		DirectoryID: "dir-zebra",
+		GroupID:     "group-zebra",
+		GroupName:   "Zebra Admins",
+	}
+	return []selectionItem{
+		{kind: selectionGroup, group: first},
+		{kind: selectionGroup, group: second},
+		{kind: selectionGroup, group: zebra},
+	}
+}
+
+// TestUIUnifiedSelector_PTY_DuplicateGroupDisplay pins the wiring of the selector that
+// plain `grant`, `grant --groups` and `grant favorites add` actually reach. It drives
+// the live survey prompt over a pty and asserts that highlighting the second of two
+// identically rendered groups elevates into that group. Resolving by display string
+// instead of by index must fail this test.
+//
+// Not parallel: newPTYSession swaps os.Stdin/os.Stderr and forceInteractive swaps
+// the package-global ui.IsTerminalFunc.
+func TestUIUnifiedSelector_PTY_DuplicateGroupDisplay(t *testing.T) {
+	tests := []struct {
+		name        string
+		keys        string
+		wantGroupID string
+		wantDirID   string
+	}{
+		{
+			name:        "arrow to second duplicate",
+			keys:        keyDown + keyEnter,
+			wantGroupID: "group-second",
+			wantDirID:   "dir-second",
+		},
+		{
+			name: "filter first, then arrow to second duplicate",
+			// Typing a filter is the dangerous path: the visible rows are a subset,
+			// yet survey still reports an index into the original option slice.
+			keys:        "Cloud" + keyDown + keyEnter,
+			wantGroupID: "group-second",
+			wantDirID:   "dir-second",
+		},
+		{
+			name:        "accept the highlighted first duplicate",
+			keys:        keyEnter,
+			wantGroupID: "group-first",
+			wantDirID:   "dir-first",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			forceInteractive(t)
+			p := newPTYSession(t)
+
+			type result struct {
+				item *selectionItem
+				err  error
+			}
+			done := make(chan result, 1)
+			go func() {
+				s := &uiUnifiedSelector{}
+				item, err := s.SelectItem(duplicateGroupItems())
+				done <- result{item, err}
+			}()
+
+			p.waitFor(t, "Select a target:")
+			p.send(t, tt.keys)
+
+			got := <-done
+			if got.err != nil {
+				t.Fatalf("SelectItem() error = %v; screen:\n%s", got.err, p.screen())
+			}
+			if got.item.kind != selectionGroup {
+				t.Fatalf("SelectItem() kind = %v, want selectionGroup", got.item.kind)
+			}
+			if got.item.group.GroupID != tt.wantGroupID {
+				t.Errorf("SelectItem() GroupID = %q, want %q (wrong group selected)",
+					got.item.group.GroupID, tt.wantGroupID)
+			}
+			if got.item.group.DirectoryID != tt.wantDirID {
+				t.Errorf("SelectItem() DirectoryID = %q, want %q (wrong directory)",
+					got.item.group.DirectoryID, tt.wantDirID)
+			}
+		})
+	}
+}
diff --git a/internal/ui/pty_linux_test.go b/internal/ui/pty_linux_test.go
new file mode 100644
index 0000000..ca894c7
--- /dev/null
+++ b/internal/ui/pty_linux_test.go
@@ -0,0 +1,145 @@
+package ui
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+	"sync"
+	"syscall"
+	"testing"
+	"time"
+	"unsafe"
+)
+
+// ptySession drives a survey prompt over a real pseudo-terminal.
+//
+// survey refuses to render unless its input is a tty and it can put that tty into
+// raw mode, so the only way to exercise the real SelectTarget/SelectGroup wiring —
+// as opposed to the pure resolve* helpers — is to give it one. This uses the raw
+// /dev/ptmx + TIOCSPTLCK/TIOCGPTN ioctls rather than a pty module so the repo keeps
+// its zero-new-dependency goal. Linux-only by file name; the ioctl constants do not
+// exist on other platforms and CI's Windows leg simply never compiles this file.
+type ptySession struct {
+	master *os.File
+	slave  *os.File
+
+	mu  sync.Mutex
+	out bytes.Buffer
+}
+
+// newPTYSession allocates a pty, points os.Stdin and os.Stderr at the slave and
+// starts draining the master. Everything is restored via t.Cleanup.
+//
+// Not parallel-safe: it mutates os.Stdin/os.Stderr. Callers must not call t.Parallel().
+func newPTYSession(t *testing.T) *ptySession {
+	t.Helper()
+
+	master, err := os.OpenFile("/dev/ptmx", os.O_RDWR, 0)
+	if err != nil {
+		t.Skipf("cannot open /dev/ptmx: %v", err)
+	}
+
+	// unlockpt(3): TIOCSPTLCK with a zero value.
+	var unlock int32
+	if _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, master.Fd(),
+		syscall.TIOCSPTLCK, uintptr(unsafe.Pointer(&unlock))); errno != 0 {
+		_ = master.Close()
+		t.Skipf("TIOCSPTLCK failed: %v", errno)
+	}
+
+	// ptsname(3): TIOCGPTN yields the slave index.
+	var ptn uint32
+	if _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, master.Fd(),
+		syscall.TIOCGPTN, uintptr(unsafe.Pointer(&ptn))); errno != 0 {
+		_ = master.Close()
+		t.Skipf("TIOCGPTN failed: %v", errno)
+	}
+
+	slave, err := os.OpenFile(fmt.Sprintf("/dev/pts/%d", ptn), os.O_RDWR|syscall.O_NOCTTY, 0)
+	if err != nil {
+		_ = master.Close()
+		t.Skipf("cannot open pty slave: %v", err)
+	}
+
+	p := &ptySession{master: master, slave: slave}
+
+	origStdin, origStderr := os.Stdin, os.Stderr
+	os.Stdin, os.Stderr = slave, slave
+
+	drained := make(chan struct{})
+	go func() {
+		defer close(drained)
+		buf := make([]byte, 4096)
+		for {
+			n, err := master.Read(buf)
+			if n > 0 {
+				p.mu.Lock()
+				p.out.Write(buf[:n])
+				p.mu.Unlock()
+			}
+			if err != nil {
+				return
+			}
+		}
+	}()
+
+	t.Cleanup(func() {
+		os.Stdin, os.Stderr = origStdin, origStderr
+		_ = slave.Close()
+		_ = master.Close()
+		select {
+		case <-drained:
+		case <-time.After(2 * time.Second):
+		}
+	})
+
+	return p
+}
+
+// screen returns everything the prompt has written so far.
+func (p *ptySession) screen() string {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	return p.out.String()
+}
+
+// waitFor blocks until the prompt has rendered want. Waiting for the prompt text is
+// what guarantees survey has already switched the tty into raw mode, so the keys sent
+// afterwards are not mangled by the canonical line discipline.
+func (p *ptySession) waitFor(t *testing.T, want string) {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for time.Now().Before(deadline) {
+		if strings.Contains(p.screen(), want) {
+			return
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+	t.Fatalf("timed out waiting for %q; screen so far:\n%s", want, p.screen())
+}
+
+// send writes keys in a single Write so multi-byte escape sequences cannot be split.
+func (p *ptySession) send(t *testing.T, keys string) {
+	t.Helper()
+	if _, err := p.master.Write([]byte(keys)); err != nil && !errors.Is(err, io.EOF) {
+		t.Fatalf("writing %q to pty: %v", keys, err)
+	}
+}
+
+const (
+	keyDown  = "\x1b[B"
+	keyEnter = "\r"
+)
+
+// forceInteractive makes IsInteractive() report true for the duration of the test.
+//
+// Not parallel: mutates the package-global ui.IsTerminalFunc.
+func forceInteractive(t *testing.T) {
+	t.Helper()
+	orig := IsTerminalFunc
+	IsTerminalFunc = func(uintptr) bool { return true }
+	t.Cleanup(func() { IsTerminalFunc = orig })
+}
diff --git a/internal/ui/selector_pty_linux_test.go b/internal/ui/selector_pty_linux_test.go
new file mode 100644
index 0000000..9623f4c
--- /dev/null
+++ b/internal/ui/selector_pty_linux_test.go
@@ -0,0 +1,98 @@
+package ui
+
+import (
+	"testing"
+
+	"github.com/aaearon/grant-cli/internal/sca/models"
+)
+
+// duplicateTargets returns three targets, the first two of which render identically:
+// FormatTargetOption carries no workspace ID, so the same workspace name and role in
+// two different subscriptions is indistinguishable on screen.
+func duplicateTargets() []models.EligibleTarget {
+	return []models.EligibleTarget{
+		{
+			CSP:           models.CSPAzure,
+			WorkspaceID:   "sub-first",
+			WorkspaceName: "Production",
+			WorkspaceType: "SUBSCRIPTION",
+			RoleInfo:      models.RoleInfo{Name: "Owner"},
+		},
+		{
+			CSP:           models.CSPAzure,
+			WorkspaceID:   "sub-second",
+			WorkspaceName: "Production",
+			WorkspaceType: "SUBSCRIPTION",
+			RoleInfo:      models.RoleInfo{Name: "Owner"},
+		},
+		{
+			CSP:           models.CSPAzure,
+			WorkspaceID:   "sub-zebra",
+			WorkspaceName: "Zebra",
+			WorkspaceType: "SUBSCRIPTION",
+			RoleInfo:      models.RoleInfo{Name: "Reader"},
+		},
+	}
+}
+
+// TestSelectTarget_PTY_DuplicateDisplay pins the real SelectTarget wiring, not just
+// resolveTargetSelection: it drives the live survey prompt over a pty and asserts that
+// highlighting the second of two identically rendered targets returns that second
+// target. Reverting SelectTarget to display-string resolution must fail this test.
+//
+// Not parallel: newPTYSession swaps os.Stdin/os.Stderr and forceInteractive swaps
+// the package-global IsTerminalFunc.
+func TestSelectTarget_PTY_DuplicateDisplay(t *testing.T) {
+	tests := []struct {
+		name  string
+		keys  string
+		wants string
+	}{
+		{
+			name:  "arrow to second duplicate",
+			keys:  keyDown + keyEnter,
+			wants: "sub-second",
+		},
+		{
+			name: "filter first, then arrow to second duplicate",
+			// Typing a filter is the dangerous path: the visible rows are a subset,
+			// yet survey still reports an index into the original option slice.
+			keys:  "Production" + keyDown + keyEnter,
+			wants: "sub-second",
+		},
+		{
+			name:  "accept the highlighted first duplicate",
+			keys:  keyEnter,
+			wants: "sub-first",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			forceInteractive(t)
+			p := newPTYSession(t)
+
+			type result struct {
+				target *models.EligibleTarget
+				err    error
+			}
+			done := make(chan result, 1)
+			go func() {
+				target, err := SelectTarget(duplicateTargets())
+				done <- result{target, err}
+			}()
+
+			p.waitFor(t, "Select a target:")
+			p.send(t, tt.keys)
+
+			got := <-done
+			if got.err != nil {
+				t.Fatalf("SelectTarget() error = %v; screen:\n%s", got.err, p.screen())
+			}
+			if got.target.WorkspaceID != tt.wants {
+				t.Errorf("SelectTarget() WorkspaceID = %q, want %q (wrong row selected)",
+					got.target.WorkspaceID, tt.wants)
+			}
+		})
+	}
+}

From 3eb128aeaf095382fc901c244c020636994312fd Mon Sep 17 00:00:00 2001
From: Tim Schindler <tim@iosharp.com>
Date: Sun, 16 Aug 2026 09:23:21 +0200
Subject: [PATCH 6/8] refactor(ui): delete the dead group selector and
 display-lookup helpers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ui.SelectGroup, ui.FindGroupByDisplay and ui.FindTargetByDisplay had zero
production callers: the group path users actually reach (`grant --groups` and
plain `grant`) goes through cmd's unified selector, and both Find* helpers were
superseded by index resolution. Removing SelectGroup orphans
sortGroupsForDisplay and resolveGroupSelection, which existed only to serve it,
so they go too, along with the tests that only covered the deleted code.

ui.SelectTarget stays — it is live via `grant env`. FormatGroupOption and
BuildGroupOptions stay — cmd/list.go uses them.
---
 internal/ui/group_selector.go      |  71 ---------
 internal/ui/group_selector_test.go | 231 -----------------------------
 internal/ui/selector.go            |  12 --
 internal/ui/selector_test.go       |  81 ----------
 4 files changed, 395 deletions(-)

diff --git a/internal/ui/group_selector.go b/internal/ui/group_selector.go
index 04cacf8..16f91b3 100644
--- a/internal/ui/group_selector.go
+++ b/internal/ui/group_selector.go
@@ -1,12 +1,9 @@
 package ui
 
 import (
-	"errors"
 	"fmt"
-	"os"
 	"sort"
 
-	"github.com/Iilun/survey/v2"
 	"github.com/aaearon/grant-cli/internal/sca/models"
 )
 
@@ -32,71 +29,3 @@ func BuildGroupOptions(groups []models.GroupsEligibleTarget) []string {
 	sort.Strings(options)
 	return options
 }
-
-// FindGroupByDisplay finds a group by its formatted display string. SelectGroup no
-// longer uses it — it resolves by index — so this has no production caller today.
-// On a display collision it returns the first match in the slice it is given.
-func FindGroupByDisplay(groups []models.GroupsEligibleTarget, display string) (*models.GroupsEligibleTarget, error) {
-	for i := range groups {
-		if FormatGroupOption(groups[i]) == display {
-			return &groups[i], nil
-		}
-	}
-	return nil, fmt.Errorf("group not found: %s", display)
-}
-
-// sortGroupsForDisplay returns a copy of groups ordered by display string, leaving
-// the caller's slice untouched. It only fixes the order the options are rendered in;
-// which group a selection denotes is decided by index in resolveGroupSelection.
-// The sort is stable, so groups that render identically keep their input order.
-func sortGroupsForDisplay(groups []models.GroupsEligibleTarget) []models.GroupsEligibleTarget {
-	sorted := make([]models.GroupsEligibleTarget, len(groups))
-	copy(sorted, groups)
-	sort.SliceStable(sorted, func(i, j int) bool {
-		return FormatGroupOption(sorted[i]) < FormatGroupOption(sorted[j])
-	})
-	return sorted
-}
-
-// resolveGroupSelection recovers the group at the index survey returned. Resolving by
-// index rather than by display text is what makes duplicate display strings safe: the
-// same group name in two directories renders identically, and a text lookup would
-// return the first match no matter which row the user highlighted.
-func resolveGroupSelection(sorted []models.GroupsEligibleTarget, idx int) (*models.GroupsEligibleTarget, error) {
-	if idx < 0 || idx >= len(sorted) {
-		return nil, fmt.Errorf("invalid group selection index %d", idx)
-	}
-	return &sorted[idx], nil
-}
-
-// SelectGroup presents an interactive selector for choosing a group. Uses the selected
-// index (not display text) to recover the group, so duplicate display strings are safe.
-func SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTarget, error) {
-	if !IsInteractive() {
-		return nil, fmt.Errorf("%w; use --group flag for non-interactive mode", ErrNotInteractive)
-	}
-
-	if len(groups) == 0 {
-		return nil, errors.New("no eligible groups available")
-	}
-
-	sorted := sortGroupsForDisplay(groups)
-
-	options := make([]string, len(sorted))
-	for i := range sorted {
-		options[i] = FormatGroupOption(sorted[i])
-	}
-
-	var selectedIdx int
-	prompt := &survey.Select{
-		Message: "Select a group:",
-		Options: options,
-		Filter:  nil,
-	}
-
-	if err := survey.AskOne(prompt, &selectedIdx, survey.WithStdio(os.Stdin, os.Stderr, os.Stderr)); err != nil {
-		return nil, fmt.Errorf("group selection failed: %w", err)
-	}
-
-	return resolveGroupSelection(sorted, selectedIdx)
-}
diff --git a/internal/ui/group_selector_test.go b/internal/ui/group_selector_test.go
index fce433b..32f7d89 100644
--- a/internal/ui/group_selector_test.go
+++ b/internal/ui/group_selector_test.go
@@ -1,10 +1,6 @@
 package ui
 
 import (
-	"errors"
-	"reflect"
-	"sort"
-	"strings"
 	"testing"
 
 	"github.com/aaearon/grant-cli/internal/sca/models"
@@ -124,230 +120,3 @@ func TestBuildGroupOptions_DuplicateDisplayStrings(t *testing.T) {
 		}
 	}
 }
-
-func TestFindGroupByDisplay_DuplicateDisplayStrings(t *testing.T) {
-	t.Parallel()
-	// When display strings collide, FindGroupByDisplay returns the first match
-	// in the slice it's given. SelectGroup sorts a copy, so the caller controls order.
-	groups := []models.GroupsEligibleTarget{
-		{DirectoryID: "dir2", GroupID: "grp2", GroupName: "Engineering"},
-		{DirectoryID: "dir1", GroupID: "grp1", GroupName: "Engineering"},
-	}
-	got, err := FindGroupByDisplay(groups, "Group: Engineering")
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-	// Should return first in slice (grp2 since dir2 is first)
-	if got.GroupID != "grp2" {
-		t.Errorf("expected grp2 (first in slice), got %q", got.GroupID)
-	}
-}
-
-func TestFindGroupByDisplay(t *testing.T) {
-	t.Parallel()
-	groups := []models.GroupsEligibleTarget{
-		{DirectoryID: "dir1", DirectoryName: "Contoso", GroupID: "grp1", GroupName: "Engineering"},
-		{DirectoryID: "dir1", DirectoryName: "Contoso", GroupID: "grp2", GroupName: "DevOps"},
-	}
-
-	tests := []struct {
-		name    string
-		groups  []models.GroupsEligibleTarget
-		display string
-		wantID  string
-		wantErr bool
-	}{
-		{
-			name:    "found engineering",
-			groups:  groups,
-			display: "Directory: Contoso / Group: Engineering",
-			wantID:  "grp1",
-		},
-		{
-			name:    "found devops",
-			groups:  groups,
-			display: "Directory: Contoso / Group: DevOps",
-			wantID:  "grp2",
-		},
-		{
-			name:    "not found",
-			groups:  groups,
-			display: "Directory: Contoso / Group: NonExistent",
-			wantErr: true,
-		},
-		{
-			name:    "empty groups",
-			groups:  []models.GroupsEligibleTarget{},
-			display: "Group: Test",
-			wantErr: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			got, err := FindGroupByDisplay(tt.groups, tt.display)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("FindGroupByDisplay() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if tt.wantErr {
-				return
-			}
-			if got.GroupID != tt.wantID {
-				t.Errorf("FindGroupByDisplay().GroupID = %q, want %q", got.GroupID, tt.wantID)
-			}
-		})
-	}
-}
-
-// TestSortGroupsForDisplay_Ordering pins exactly two properties of the helper, and no
-// more: the options rendered from its result are in display order, and the caller's
-// slice is not reordered underneath it. It says nothing about which group a selection
-// denotes — that is resolveGroupSelection's job and is pinned by
-// TestResolveGroupSelection_DuplicateDisplayStrings below.
-func TestSortGroupsForDisplay_Ordering(t *testing.T) {
-	t.Parallel()
-	// Deliberately unsorted input containing a display collision: the two
-	// "Engineering" groups have no DirectoryName, so both render identically.
-	groups := []models.GroupsEligibleTarget{
-		{DirectoryID: "dir-z", GroupID: "grp-zebra", GroupName: "Zebra Team"},
-		{DirectoryID: "dir-1", GroupID: "grp-eng-1", GroupName: "Engineering"},
-		{DirectoryID: "dir-2", GroupID: "grp-eng-2", GroupName: "Engineering"},
-		{DirectoryID: "dir-a", GroupID: "grp-alpha", GroupName: "Alpha Team"},
-	}
-	before := append([]models.GroupsEligibleTarget(nil), groups...)
-
-	sorted := sortGroupsForDisplay(groups)
-
-	if len(sorted) != len(groups) {
-		t.Fatalf("sortGroupsForDisplay() length = %d, want %d", len(sorted), len(groups))
-	}
-
-	options := make([]string, len(sorted))
-	for i := range sorted {
-		options[i] = FormatGroupOption(sorted[i])
-	}
-	if !sort.StringsAreSorted(options) {
-		t.Errorf("rendered options are not in display order: %q", options)
-	}
-
-	// The caller's slice must not be reordered underneath it — full snapshot, not
-	// just the endpoints, so an in-place sort cannot hide in the middle.
-	if !reflect.DeepEqual(groups, before) {
-		t.Errorf("sortGroupsForDisplay() mutated the caller's slice:\n got %+v\nwant %+v", groups, before)
-	}
-}
-
-// TestResolveGroupSelection_DuplicateDisplayStrings pins the wrong-group fix. Two
-// groups with the same name in different directories render to the same display
-// string, so recovering the answer by text returns the first match regardless of
-// which row the user highlighted — highlight the second, elevate into the first.
-// survey.Select cannot be driven from a test, so the index path is asserted on the
-// extracted resolver, mirroring SelectRole/SelectRequest.
-func TestResolveGroupSelection_DuplicateDisplayStrings(t *testing.T) {
-	t.Parallel()
-	groups := []models.GroupsEligibleTarget{
-		{DirectoryID: "dir-1", GroupID: "grp-eng-1", GroupName: "Engineering"},
-		{DirectoryID: "dir-2", GroupID: "grp-eng-2", GroupName: "Engineering"},
-	}
-	sorted := sortGroupsForDisplay(groups)
-	if len(sorted) != 2 {
-		t.Fatalf("sortGroupsForDisplay() length = %d, want 2", len(sorted))
-	}
-	if FormatGroupOption(sorted[0]) != FormatGroupOption(sorted[1]) {
-		t.Fatalf("fixture no longer collides: %q vs %q", FormatGroupOption(sorted[0]), FormatGroupOption(sorted[1]))
-	}
-	if sorted[0].GroupID == sorted[1].GroupID {
-		t.Fatalf("fixture groups are indistinguishable: %+v", sorted)
-	}
-
-	// The sort is stable and the two entries compare equal, so the rendered order is
-	// the input order: row 0 is grp-eng-1, row 1 is grp-eng-2.
-	tests := []struct {
-		idx         int
-		wantGroupID string
-		wantDirID   string
-	}{
-		{idx: 0, wantGroupID: "grp-eng-1", wantDirID: "dir-1"},
-		{idx: 1, wantGroupID: "grp-eng-2", wantDirID: "dir-2"},
-	}
-	for _, tt := range tests {
-		got, err := resolveGroupSelection(sorted, tt.idx)
-		if err != nil {
-			t.Fatalf("resolveGroupSelection(_, %d) error = %v", tt.idx, err)
-		}
-		if got.GroupID != tt.wantGroupID || got.DirectoryID != tt.wantDirID {
-			t.Errorf("selecting row %d returned GroupID=%q DirectoryID=%q, want %q/%q",
-				tt.idx, got.GroupID, got.DirectoryID, tt.wantGroupID, tt.wantDirID)
-		}
-	}
-}
-
-func TestResolveGroupSelection_OutOfRange(t *testing.T) {
-	t.Parallel()
-	groups := []models.GroupsEligibleTarget{{DirectoryID: "dir-1", GroupID: "grp1", GroupName: "Engineering"}}
-	for _, idx := range []int{-1, 1} {
-		if _, err := resolveGroupSelection(groups, idx); err == nil {
-			t.Errorf("resolveGroupSelection(_, %d) = nil error, want out-of-range error", idx)
-		}
-	}
-}
-
-// Not parallel: mutates the package-global IsTerminalFunc.
-func TestSelectGroup_EmptyList(t *testing.T) {
-	original := IsTerminalFunc
-	defer func() { IsTerminalFunc = original }()
-	IsTerminalFunc = func(fd uintptr) bool { return true }
-
-	_, err := SelectGroup(nil)
-	if err == nil {
-		t.Fatal("expected error for empty list")
-	}
-	if !strings.Contains(err.Error(), "no eligible groups available") {
-		t.Errorf("unexpected error: %v", err)
-	}
-}
-
-// Not parallel: mutates the package-global IsTerminalFunc.
-func TestSelectGroup_NonTTY(t *testing.T) {
-	original := IsTerminalFunc
-	defer func() { IsTerminalFunc = original }()
-	IsTerminalFunc = func(fd uintptr) bool { return false }
-
-	groups := []models.GroupsEligibleTarget{
-		{DirectoryID: "dir1", GroupID: "grp1", GroupName: "Engineering"},
-	}
-
-	_, err := SelectGroup(groups)
-	if err == nil {
-		t.Fatal("expected error for non-TTY")
-	}
-	if !errors.Is(err, ErrNotInteractive) {
-		t.Errorf("expected ErrNotInteractive, got: %v", err)
-	}
-	if !strings.Contains(err.Error(), "--group") {
-		t.Errorf("error should mention --group, got: %v", err)
-	}
-}
-
-// TestSelectGroup_NonTTYEmptyList pins the order of the two guards. _NonTTY passes a
-// non-empty list and _EmptyList forces a TTY, so their inputs never intersect and
-// swapping the guards survives both. This case satisfies both conditions at once and
-// demands the non-interactive error.
-// Not parallel: mutates the package-global IsTerminalFunc.
-// The package restores globals with defer rather than t.Cleanup — deliberate, it is
-// the convention every other test in internal/ui already follows.
-func TestSelectGroup_NonTTYEmptyList(t *testing.T) {
-	original := IsTerminalFunc
-	defer func() { IsTerminalFunc = original }()
-	IsTerminalFunc = func(fd uintptr) bool { return false }
-
-	_, err := SelectGroup(nil)
-	if err == nil {
-		t.Fatal("expected error for non-TTY with an empty list")
-	}
-	if !errors.Is(err, ErrNotInteractive) {
-		t.Errorf("expected ErrNotInteractive to win over the empty-list guard, got: %v", err)
-	}
-}
diff --git a/internal/ui/selector.go b/internal/ui/selector.go
index 14f9157..6f9ee3e 100644
--- a/internal/ui/selector.go
+++ b/internal/ui/selector.go
@@ -59,18 +59,6 @@ func BuildOptions(targets []models.EligibleTarget) []string {
 	return options
 }
 
-// FindTargetByDisplay finds a target by its formatted display string. SelectTarget no
-// longer uses it — it resolves by index — so this has no production caller today.
-// On a display collision it returns the first match in the slice it is given.
-func FindTargetByDisplay(targets []models.EligibleTarget, display string) (*models.EligibleTarget, error) {
-	for i := range targets {
-		if FormatTargetOption(targets[i]) == display {
-			return &targets[i], nil
-		}
-	}
-	return nil, fmt.Errorf("target not found: %s", display)
-}
-
 // sortTargetsForDisplay returns a copy of targets ordered by display string, leaving
 // the caller's slice untouched. It only fixes the order the options are rendered in;
 // which target a selection denotes is decided by index in resolveTargetSelection.
diff --git a/internal/ui/selector_test.go b/internal/ui/selector_test.go
index 77831f5..17a81c5 100644
--- a/internal/ui/selector_test.go
+++ b/internal/ui/selector_test.go
@@ -257,87 +257,6 @@ func TestSelectTarget_NonTTYEmptyList(t *testing.T) {
 	}
 }
 
-func TestFindTargetByDisplay(t *testing.T) {
-	t.Parallel()
-	targets := []models.EligibleTarget{
-		{
-			OrganizationID: "org1",
-			WorkspaceID:    "sub1",
-			WorkspaceName:  "Production",
-			WorkspaceType:  models.WorkspaceTypeSubscription,
-			RoleInfo:       models.RoleInfo{ID: "role1", Name: "Owner"},
-		},
-		{
-			OrganizationID: "org1",
-			WorkspaceID:    "rg1",
-			WorkspaceName:  "rg-web",
-			WorkspaceType:  models.WorkspaceTypeResourceGroup,
-			RoleInfo:       models.RoleInfo{ID: "role2", Name: "Contributor"},
-		},
-	}
-
-	tests := []struct {
-		name    string
-		targets []models.EligibleTarget
-		display string
-		want    *models.EligibleTarget
-		wantErr bool
-	}{
-		{
-			name:    "found subscription",
-			targets: targets,
-			display: "Subscription: Production / Role: Owner",
-			want:    &targets[0],
-			wantErr: false,
-		},
-		{
-			name:    "found resource group",
-			targets: targets,
-			display: "Resource Group: rg-web / Role: Contributor",
-			want:    &targets[1],
-			wantErr: false,
-		},
-		{
-			name:    "not found",
-			targets: targets,
-			display: "Subscription: NonExistent / Role: Reader",
-			want:    nil,
-			wantErr: true,
-		},
-		{
-			name:    "empty targets",
-			targets: []models.EligibleTarget{},
-			display: "Subscription: Test / Role: Owner",
-			want:    nil,
-			wantErr: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			got, err := FindTargetByDisplay(tt.targets, tt.display)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("FindTargetByDisplay() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if tt.want == nil && got != nil {
-				t.Errorf("FindTargetByDisplay() = %v, want nil", got)
-				return
-			}
-			if tt.want != nil {
-				if got == nil {
-					t.Errorf("FindTargetByDisplay() = nil, want %v", tt.want)
-					return
-				}
-				if got.WorkspaceID != tt.want.WorkspaceID || got.RoleInfo.ID != tt.want.RoleInfo.ID {
-					t.Errorf("FindTargetByDisplay() = %v, want %v", got, tt.want)
-				}
-			}
-		})
-	}
-}
-
 // TestSortTargetsForDisplay_Ordering pins the order options are rendered in, and
 // nothing else. Which target a selection denotes is resolveTargetSelection's job.
 func TestSortTargetsForDisplay_Ordering(t *testing.T) {

From 1d1417eaeca14ecbd0c436bfab26484759af1a3d Mon Sep 17 00:00:00 2001
From: Tim Schindler <tim@iosharp.com>
Date: Sun, 16 Aug 2026 09:24:32 +0200
Subject: [PATCH 7/8] test: widen the sort fixtures so instability is actually
 detectable

Both sorted selectors were only exercised with 4-element fixtures. Go's pdqsort
delegates to insertion sort below n=12, which is incidentally stable, so
swapping sort.SliceStable for sort.Slice passed the whole suite. The new
fixtures use 15 entries in 5 colliding groups and assert that items rendering
identically keep their input order, which kills that mutation in both places.
---
 cmd/selection_test.go        | 105 +++++++++++++++++++++++++++++++++++
 internal/ui/selector_test.go |  71 +++++++++++++++++++++++
 2 files changed, 176 insertions(+)

diff --git a/cmd/selection_test.go b/cmd/selection_test.go
index a7c1986..0ed0cf2 100644
--- a/cmd/selection_test.go
+++ b/cmd/selection_test.go
@@ -1,6 +1,8 @@
 package cmd
 
 import (
+	"fmt"
+	"reflect"
 	"testing"
 
 	scamodels "github.com/aaearon/grant-cli/internal/sca/models"
@@ -221,3 +223,106 @@ func TestResolveSelectionItem_EmptySlice(t *testing.T) {
 		t.Errorf("resolveSelectionItem(nil, 0) = %v, want error", item)
 	}
 }
+
+// collidingSelectionItems returns 15 items in 5 groups of 3 that render identically
+// within each group, mixing cloud targets and groups. The size matters: Go's pdqsort
+// delegates to insertion sort below n=12, which happens to be stable, so a small
+// fixture cannot tell sort.Slice and sort.SliceStable apart. The ID encodes the input
+// position within the group.
+func collidingSelectionItems() []selectionItem {
+	names := []string{"Alpha", "Bravo", "Charlie", "Delta", "Echo"}
+	var items []selectionItem
+	// Interleaved and reversed by name so an unstable sort has plenty to scramble.
+	for copyNum := 1; copyNum <= 3; copyNum++ {
+		for n := len(names) - 1; n >= 0; n-- {
+			id := fmt.Sprintf("%s-%d", names[n], copyNum)
+			if n%2 == 0 {
+				items = append(items, selectionItem{
+					kind: selectionGroup,
+					group: &scamodels.GroupsEligibleTarget{
+						DirectoryID: "dir-" + id,
+						GroupID:     id,
+						GroupName:   names[n],
+					},
+				})
+				continue
+			}
+			items = append(items, selectionItem{
+				kind: selectionCloud,
+				cloud: &scamodels.EligibleTarget{
+					WorkspaceID:   id,
+					WorkspaceName: names[n],
+					WorkspaceType: scamodels.WorkspaceTypeSubscription,
+					RoleInfo:      scamodels.RoleInfo{Name: "Owner"},
+				},
+			})
+		}
+	}
+	return items
+}
+
+// selectionItemID returns the identity that formatSelectionItem does not render.
+func selectionItemID(item selectionItem) string {
+	switch item.kind {
+	case selectionGroup:
+		return item.group.GroupID
+	case selectionCloud:
+		return item.cloud.WorkspaceID
+	default:
+		return ""
+	}
+}
+
+// TestBuildUnifiedOptions_StableAmongCollisions pins the sort as *stable*, not merely
+// ordered. The options slice and the returned items slice must stay index-for-index
+// aligned, and two items that render identically must keep their input order —
+// otherwise the same keystrokes elevate into a different target run to run.
+func TestBuildUnifiedOptions_StableAmongCollisions(t *testing.T) {
+	items := collidingSelectionItems()
+	if len(items) < 13 {
+		t.Fatalf("fixture has %d items; needs >= 13 to distinguish sort.Slice from sort.SliceStable", len(items))
+	}
+
+	options, sorted := buildUnifiedOptions(items)
+	if len(options) != len(items) || len(sorted) != len(items) {
+		t.Fatalf("buildUnifiedOptions() = %d options / %d items, want %d each", len(options), len(sorted), len(items))
+	}
+
+	// options[i] must be the rendering of sorted[i]; index resolution depends on it.
+	for i := range sorted {
+		if options[i] != formatSelectionItem(sorted[i]) {
+			t.Fatalf("options[%d] = %q, but sorted[%d] renders as %q", i, options[i], i, formatSelectionItem(sorted[i]))
+		}
+	}
+
+	// Displays must be non-decreasing (it is still a sort).
+	for i := 1; i < len(options); i++ {
+		if options[i-1] > options[i] {
+			t.Fatalf("not sorted at %d: %q > %q", i, options[i-1], options[i])
+		}
+	}
+
+	// Within each colliding display, input order must be preserved.
+	wantOrder := map[string][]string{}
+	for _, item := range items {
+		d := formatSelectionItem(item)
+		wantOrder[d] = append(wantOrder[d], selectionItemID(item))
+	}
+	gotOrder := map[string][]string{}
+	for _, item := range sorted {
+		d := formatSelectionItem(item)
+		gotOrder[d] = append(gotOrder[d], selectionItemID(item))
+	}
+	if len(wantOrder) < 2 {
+		t.Fatalf("fixture no longer collides: %d distinct displays", len(wantOrder))
+	}
+	for display, want := range wantOrder {
+		if len(want) < 2 {
+			t.Fatalf("display %q does not collide; fixture is broken", display)
+		}
+		if !reflect.DeepEqual(gotOrder[display], want) {
+			t.Errorf("display %q: colliding items reordered\n got %v\nwant %v (input order)",
+				display, gotOrder[display], want)
+		}
+	}
+}
diff --git a/internal/ui/selector_test.go b/internal/ui/selector_test.go
index 17a81c5..75737c3 100644
--- a/internal/ui/selector_test.go
+++ b/internal/ui/selector_test.go
@@ -2,6 +2,7 @@ package ui
 
 import (
 	"errors"
+	"fmt"
 	"reflect"
 	"sort"
 	"strings"
@@ -368,3 +369,73 @@ func TestSelectTarget_EmptyList(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}
 }
+
+// collidingTargets returns 15 targets in 5 groups of 3 that render identically within
+// each group. The size matters: Go's pdqsort delegates to insertion sort below n=12,
+// which happens to be stable, so a 4-element fixture cannot tell sort.Slice and
+// sort.SliceStable apart. WorkspaceID encodes the input position within its group.
+func collidingTargets() []models.EligibleTarget {
+	names := []string{"Alpha", "Bravo", "Charlie", "Delta", "Echo"}
+	var targets []models.EligibleTarget
+	// Interleaved and reversed by name so an unstable sort has plenty to scramble.
+	for copyNum := 1; copyNum <= 3; copyNum++ {
+		for n := len(names) - 1; n >= 0; n-- {
+			targets = append(targets, models.EligibleTarget{
+				WorkspaceID:   fmt.Sprintf("%s-%d", names[n], copyNum),
+				WorkspaceName: names[n],
+				WorkspaceType: models.WorkspaceTypeSubscription,
+				RoleInfo:      models.RoleInfo{Name: "Owner"},
+			})
+		}
+	}
+	return targets
+}
+
+// TestSortTargetsForDisplay_StableAmongCollisions pins the sort as *stable*, not merely
+// ordered. Rendering order is what the user sees and index resolution is resolved
+// against the same slice, so two targets that render identically must keep their input
+// order — otherwise the same keystrokes pick a different target run to run.
+func TestSortTargetsForDisplay_StableAmongCollisions(t *testing.T) {
+	t.Parallel()
+
+	targets := collidingTargets()
+	if len(targets) < 13 {
+		t.Fatalf("fixture has %d targets; needs >= 13 to distinguish sort.Slice from sort.SliceStable", len(targets))
+	}
+
+	sorted := sortTargetsForDisplay(targets)
+	if len(sorted) != len(targets) {
+		t.Fatalf("sortTargetsForDisplay() length = %d, want %d", len(sorted), len(targets))
+	}
+
+	// Displays must be non-decreasing (it is still a sort).
+	for i := 1; i < len(sorted); i++ {
+		if FormatTargetOption(sorted[i-1]) > FormatTargetOption(sorted[i]) {
+			t.Fatalf("not sorted at %d: %q > %q", i, FormatTargetOption(sorted[i-1]), FormatTargetOption(sorted[i]))
+		}
+	}
+
+	// Within each colliding display, input order must be preserved.
+	wantOrder := map[string][]string{}
+	for _, target := range targets {
+		d := FormatTargetOption(target)
+		wantOrder[d] = append(wantOrder[d], target.WorkspaceID)
+	}
+	gotOrder := map[string][]string{}
+	for _, target := range sorted {
+		d := FormatTargetOption(target)
+		gotOrder[d] = append(gotOrder[d], target.WorkspaceID)
+	}
+	if len(wantOrder) < 2 {
+		t.Fatalf("fixture no longer collides: %d distinct displays", len(wantOrder))
+	}
+	for display, want := range wantOrder {
+		if len(want) < 2 {
+			t.Fatalf("display %q does not collide; fixture is broken", display)
+		}
+		if !reflect.DeepEqual(gotOrder[display], want) {
+			t.Errorf("display %q: colliding targets reordered\n got %v\nwant %v (input order)",
+				display, gotOrder[display], want)
+		}
+	}
+}

From f33a246722adff09eec73b5cdffaf9834dbd49c1 Mon Sep 17 00:00:00 2001
From: Tim Schindler <tim@iosharp.com>
Date: Sun, 16 Aug 2026 09:27:17 +0200
Subject: [PATCH 8/8] docs: correct the pinning claims for the selector fixes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The ledger credited the two index fixes to tests over the extracted resolve*
helpers, which do not pin the wiring — reverting the selectors to
display-string resolution left the suite green. Name the pty tests instead, and
add rows for the unified-selector fix, the dead-code deletion and the widened
sort fixtures.

A test comment claimed survey.Select cannot be driven from a test; it can, and
now is. The SelectSessions comment asserted collisions are impossible; that is
an assumption about the SCA API, so state it as one and note that
FindSessionByDisplay fails open if it breaks.

CHANGELOG: collapse the two selector lines into one that covers both the
unified selector and grant env's target selector.
---
 CHANGELOG.md                    |  3 +--
 cmd/pty_linux_test.go           |  2 +-
 cmd/request_submit.go           |  6 ++++++
 docs/mutation-ledger.md         | 20 +++++++++++++-------
 internal/ui/pty_linux_test.go   |  2 +-
 internal/ui/selector_test.go    |  5 +++--
 internal/ui/session_selector.go | 12 +++++++++---
 7 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8ddc366..b977db2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,8 +8,7 @@ All notable changes to this project will be documented in this file.
 
 - `grant favorites add` now fails immediately without a terminal instead of authenticating first
 - `grant favorites add`'s non-interactive error now mentions the required favorite name, not only the flags
-- Picking one of two identically named Entra ID groups in the interactive selector no longer elevates into the other
-- The interactive target selector now elevates the row you picked, not another target that renders the same way
+- Interactive selectors now elevate the row you picked, not another target or Entra ID group that happens to render the same way
 
 ## [0.9.0] - 2026-08-14
 
diff --git a/cmd/pty_linux_test.go b/cmd/pty_linux_test.go
index 612e600..4862427 100644
--- a/cmd/pty_linux_test.go
+++ b/cmd/pty_linux_test.go
@@ -126,7 +126,7 @@ func (p *ptySession) waitFor(t *testing.T, want string) {
 // send writes keys in a single Write so multi-byte escape sequences cannot be split.
 func (p *ptySession) send(t *testing.T, keys string) {
 	t.Helper()
-	if _, err := p.master.Write([]byte(keys)); err != nil && !errors.Is(err, io.EOF) {
+	if _, err := p.master.WriteString(keys); err != nil && !errors.Is(err, io.EOF) {
 		t.Fatalf("writing %q to pty: %v", keys, err)
 	}
 }
diff --git a/cmd/request_submit.go b/cmd/request_submit.go
index f01e95f..4360485 100644
--- a/cmd/request_submit.go
+++ b/cmd/request_submit.go
@@ -446,6 +446,12 @@ func selectSubmitWorkspace(workspaces []submitWorkspace) (*submitWorkspace, erro
 	if err != nil {
 		return nil, err
 	}
+	// Bounds-checked rather than trusted: survey reports an index into Options, which
+	// is built one-for-one from workspaces, so this should always be in range — but a
+	// panic here would be a poor way to find out otherwise.
+	if selected < 0 || selected >= len(workspaces) {
+		return nil, fmt.Errorf("invalid workspace selection index %d", selected)
+	}
 	return &workspaces[selected], nil
 }
 
diff --git a/docs/mutation-ledger.md b/docs/mutation-ledger.md
index 847f8d3..46bb994 100644
--- a/docs/mutation-ledger.md
+++ b/docs/mutation-ledger.md
@@ -248,14 +248,20 @@ The seven production changes from the plan's table, plus two added by the PR7 re
 | OUT-26 | `favorites add` early non-interactive guard, favorites-specific message | PR1 | `### Fixed` |
 | CFG-02 | `ParseCacheTTL` errors on any explicitly-invalid value | PR6 | `### Changed` |
 | CACHE-07 | `maxSessionAge` → `sessionTimestampRetention` | PR6 | no (internal) |
-| UI-02 | `sortGroupsForDisplay` extraction | PR7 | no (refactor) |
+| UI-02 | `sortGroupsForDisplay` extraction | PR7 | no (refactor; since removed with `SelectGroup`) |
 | SFU-10 | `syncStagedFileFn` seam | PR3 | no (test seam; **not** a production gap) |
-| — | `SelectGroup` resolves the answer by **index** (`resolveGroupSelection`) instead of display text, matching `SelectRole`/`SelectRequest`. Two groups with the same name in different directories render identically, so the old text lookup returned the first match whatever the user highlighted. Not an audit row — found by the PR7 review. Pinned by `TestResolveGroupSelection_DuplicateDisplayStrings` | PR7 | `### Fixed` |
-| — | `SelectTarget` resolves the answer by **index** (`resolveTargetSelection`) against the same sorted copy it renders (`sortTargetsForDisplay`), instead of looking the display text up in the caller's **unsorted** slice. `FormatTargetOption` carries no ID, so two targets with the same workspace name and role in different subscriptions/accounts render identically; worse than the group case, because the rendered and searched slices were in different orders. Not an audit row — found by the PR7 review. Pinned by `TestResolveTargetSelection_DuplicateDisplayStrings` and `TestSortTargetsForDisplay_Ordering`. `SelectSessions` still resolves by text and is deliberately left alone — its option strings embed the session ID, so collisions are unreachable | PR7 | `### Fixed` |
-
-`FindGroupByDisplay` and `FindTargetByDisplay` are exported and still covered by tests,
-but neither has a production caller after the two index fixes. Both are follow-up
-deletion candidates; they were kept here rather than removed in a fix commit.
+| — | `SelectTarget` resolves the answer by **index** (`resolveTargetSelection`) against the same sorted copy it renders (`sortTargetsForDisplay`), instead of looking the display text up in the caller's **unsorted** slice. `FormatTargetOption` carries no ID, so two targets with the same workspace name and role in different subscriptions/accounts render identically, and the rendered and searched slices were in different orders besides. Not an audit row — found by the PR7 review. Pinned by `TestSelectTarget_PTY_DuplicateDisplay`, which drives the live prompt over a pty and covers the filtered path; `TestResolveTargetSelection_DuplicateDisplayStrings` and `TestSortTargetsForDisplay_Ordering` cover the extracted helpers only and do **not** pin the wiring | PR7 | `### Fixed` |
+| — | `uiUnifiedSelector.SelectItem` resolves the answer by **index** (`resolveSelectionItem`) instead of display text, and `buildUnifiedOptions` sorts stably so options and items stay index-aligned. This is the selector plain `grant`, `grant --groups` and `grant favorites add` actually reach, and it was still bug-for-bug identical to the pre-fix `SelectGroup`: `formatSelectionItem` carries no ID, so two Entra ID groups with the same name in different directories render identically and the text lookup elevated into the first. Not an audit row — found by the PR7 adversarial re-review, which showed the original PR7 fix missed the live caller. Pinned by `TestUIUnifiedSelector_PTY_DuplicateGroupDisplay` (wiring, incl. the filtered path), `TestResolveSelectionItem` (bounds) and `TestBuildUnifiedOptions_StableAmongCollisions` (stability) | PR7 | `### Fixed` |
+| — | `SelectGroup`, `FindGroupByDisplay` and `FindTargetByDisplay` deleted — zero production callers. `sortGroupsForDisplay` and `resolveGroupSelection` existed only to serve `SelectGroup` and went with it; the group path in production is the unified selector above. Closes the deletion-candidate note that used to sit under this table | PR7 | no (dead code) |
+| — | Sort fixtures for `sortTargetsForDisplay` and `buildUnifiedOptions` widened to 15 colliding entries. At n=4 Go's pdqsort delegates to insertion sort, which is incidentally stable, so `sort.SliceStable` → `sort.Slice` survived the suite in both places. Both mutants are now killed | PR7 | no (test-only) |
+| — | `SelectSessions` still resolves by display text, deliberately. Its option strings embed `session.SessionID` and the SCA API is **assumed** to return distinct IDs, so collisions should be unreachable; `FindSessionByDisplay` fails *open* if that assumption breaks. Assumption now stated at the call site rather than asserted as fact | PR7 | no (comment only) |
+
+The two index fixes above are pinned by pty-driven tests that drive the real
+`survey.Select` prompt (`internal/ui/pty_linux_test.go`, `cmd/pty_linux_test.go`;
+raw `/dev/ptmx` + `TIOCSPTLCK`/`TIOCGPTN`, no new module dependency, Linux-only by
+file name). This matters: tests over the extracted `resolve*` helpers alone left the
+whole suite green when the selectors were reverted to display-string resolution, so
+they pinned the helpers and not the behaviour.
 
 ### Total CONFIRMED
 
diff --git a/internal/ui/pty_linux_test.go b/internal/ui/pty_linux_test.go
index ca894c7..cc2d3d1 100644
--- a/internal/ui/pty_linux_test.go
+++ b/internal/ui/pty_linux_test.go
@@ -124,7 +124,7 @@ func (p *ptySession) waitFor(t *testing.T, want string) {
 // send writes keys in a single Write so multi-byte escape sequences cannot be split.
 func (p *ptySession) send(t *testing.T, keys string) {
 	t.Helper()
-	if _, err := p.master.Write([]byte(keys)); err != nil && !errors.Is(err, io.EOF) {
+	if _, err := p.master.WriteString(keys); err != nil && !errors.Is(err, io.EOF) {
 		t.Fatalf("writing %q to pty: %v", keys, err)
 	}
 }
diff --git a/internal/ui/selector_test.go b/internal/ui/selector_test.go
index 75737c3..d9b8aaa 100644
--- a/internal/ui/selector_test.go
+++ b/internal/ui/selector_test.go
@@ -302,8 +302,9 @@ func TestSortTargetsForDisplay_Ordering(t *testing.T) {
 // Recovering the answer by text returns the first match regardless of which row the
 // user highlighted — and SelectTarget searched the caller's *unsorted* slice while
 // rendering a sorted one, so the two could disagree even without a collision.
-// survey.Select cannot be driven from a test, so the index path is asserted on the
-// extracted resolver, mirroring SelectGroup/SelectRole/SelectRequest.
+// This covers the extracted resolver in isolation. The real SelectTarget wiring is
+// pinned separately by TestSelectTarget_PTY_DuplicateDisplay, which drives the live
+// survey prompt over a pseudo-terminal.
 func TestResolveTargetSelection_DuplicateDisplayStrings(t *testing.T) {
 	t.Parallel()
 	targets := []models.EligibleTarget{
diff --git a/internal/ui/session_selector.go b/internal/ui/session_selector.go
index 5002a6f..a7d4846 100644
--- a/internal/ui/session_selector.go
+++ b/internal/ui/session_selector.go
@@ -105,9 +105,15 @@ func FindSessionByDisplay(
 
 // SelectSessions presents a multi-select prompt for choosing sessions to revoke.
 // It resolves the answer by display text rather than by index, unlike the target,
-// group, role and request selectors. That is safe here and deliberately left alone:
-// every option string embeds session.SessionID, so two sessions can never render
-// identically and FindSessionByDisplay cannot resolve to the wrong row.
+// role and request selectors.
+//
+// That rests on an assumption about the SCA API, stated here rather than left
+// implicit: every option string embeds session.SessionID, and the API is assumed to
+// return distinct session IDs, so no two options can render identically. If that ever
+// stops holding, FindSessionByDisplay fails *open* — it returns the first match — and
+// the wrong session would be revoked. Revoking is far less dangerous than elevating
+// into the wrong role, which is why this is documented rather than converted to index
+// binding, but the assumption is load-bearing.
 func SelectSessions(sessions []models.SessionInfo, nameMap map[string]string) ([]models.SessionInfo, error) {
 	if !IsInteractive() {
 		return nil, fmt.Errorf("%w; use --all or provide session IDs as arguments", ErrNotInteractive)