feat: add TTY detection and fail-fast for non-interactive mode

aaearon · aaearon · commit f2c8362345b0 · 2026-02-21T09:19:34.000+01:00
Introduce internal/ui/tty.go with IsInteractive() and ErrNotInteractive
so all interactive prompts fail fast with helpful error messages when
stdin is not a terminal, preventing hangs in LLM/CI/pipe contexts.

Guarded functions: SelectTarget, SelectSessions, ConfirmRevocation,
SelectGroup, uiUnifiedSelector.SelectItem, surveyNamePrompter.PromptName.

Promoted go-isatty v0.0.20 from indirect to direct dependency.
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -64,6 +64,12 @@ Custom `SCAAccessService` follows SDK conventions:
 - `--refresh` bypasses eligibility cache on `grant` and `grant env`
 - `fetchEligibility()` and `resolveTargetCSP()` in `cmd/root.go` — shared by root, env, and favorites
 
+## TTY Detection
+- `internal/ui/tty.go` — `IsTerminalFunc` (overridable), `IsInteractive()`, `ErrNotInteractive`
+- All interactive prompts (`SelectTarget`, `SelectSessions`, `ConfirmRevocation`, `SelectGroup`, `uiUnifiedSelector.SelectItem`, `surveyNamePrompter.PromptName`) fail fast with `ErrNotInteractive` when stdin is not a TTY
+- Error messages suggest the appropriate non-interactive flag (e.g., `--target/--role`, `--all`, `--yes`, `--group`, `--favorite`)
+- `go-isatty` v0.0.20 is a direct dependency (promoted from indirect via survey)
+
 ## Cache
 - Eligibility responses cached in `~/.grant/cache/` as JSON files (e.g., `eligibility_azure.json`, `groups_eligibility_azure.json`)
 - Default TTL: 4 hours, configurable via `cache_ttl` in `~/.grant/config.yaml` (Go duration syntax: `2h`, `30m`)
diff --git a/cmd/favorites.go b/cmd/favorites.go
@@ -8,6 +8,7 @@ import (
 
 	survey "github.com/Iilun/survey/v2"
 	"github.com/aaearon/grant-cli/internal/config"
+	"github.com/aaearon/grant-cli/internal/ui"
 	"github.com/spf13/cobra"
 )
 
@@ -95,6 +96,10 @@ func newFavoritesAddCommand() *cobra.Command {
 type surveyNamePrompter struct{}
 
 func (s *surveyNamePrompter) PromptName() (string, error) {
+	if !ui.IsInteractive() {
+		return "", fmt.Errorf("%w; provide the name as an argument", ui.ErrNotInteractive)
+	}
+
 	var name string
 	if err := survey.AskOne(&survey.Input{
 		Message: "Favorite name:",
diff --git a/cmd/favorites_test.go b/cmd/favorites_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/aaearon/grant-cli/internal/cache"
 	"github.com/aaearon/grant-cli/internal/config"
 	"github.com/aaearon/grant-cli/internal/sca/models"
+	"github.com/aaearon/grant-cli/internal/ui"
 )
 
 func TestFavoritesListCommand(t *testing.T) {
@@ -1183,3 +1184,21 @@ func TestFavoritesListWithGroupFavorites(t *testing.T) {
 		})
 	}
 }
+
+func TestSurveyNamePrompter_NonTTY(t *testing.T) {
+	original := ui.IsTerminalFunc
+	defer func() { ui.IsTerminalFunc = original }()
+	ui.IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	prompter := &surveyNamePrompter{}
+	_, err := prompter.PromptName()
+	if err == nil {
+		t.Fatal("expected error for non-TTY")
+	}
+	if !errors.Is(err, ui.ErrNotInteractive) {
+		t.Errorf("expected ErrNotInteractive, got: %v", err)
+	}
+	if !strings.Contains(err.Error(), "argument") {
+		t.Errorf("error should suggest providing name as argument, got: %v", err)
+	}
+}
diff --git a/cmd/root.go b/cmd/root.go
@@ -830,6 +830,10 @@ func (s *uiSelector) SelectTarget(targets []models.EligibleTarget) (*models.Elig
 type uiUnifiedSelector struct{}
 
 func (s *uiUnifiedSelector) SelectItem(items []selectionItem) (*selectionItem, error) {
+	if !ui.IsInteractive() {
+		return nil, fmt.Errorf("%w; use --target/--role, --group, or --favorite flags for non-interactive mode", ui.ErrNotInteractive)
+	}
+
 	if len(items) == 0 {
 		return nil, errors.New("no eligible targets or groups available")
 	}
diff --git a/cmd/root_test.go b/cmd/root_test.go
@@ -6,6 +6,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/aaearon/grant-cli/internal/sca/models"
+	"github.com/aaearon/grant-cli/internal/ui"
 	"github.com/cyberark/idsec-sdk-golang/pkg/config"
 	"github.com/spf13/cobra"
 )
@@ -219,3 +221,30 @@ func TestExecuteHintOutput(t *testing.T) {
 		})
 	}
 }
+
+func TestUnifiedSelector_NonTTY(t *testing.T) {
+	original := ui.IsTerminalFunc
+	defer func() { ui.IsTerminalFunc = original }()
+	ui.IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	selector := &uiUnifiedSelector{}
+	items := []selectionItem{
+		{kind: selectionCloud, cloud: &models.EligibleTarget{
+			WorkspaceName: "Sub A",
+			WorkspaceType: models.WorkspaceTypeSubscription,
+			RoleInfo:      models.RoleInfo{Name: "Owner"},
+		}},
+	}
+
+	_, err := selector.SelectItem(items)
+	if err == nil {
+		t.Fatal("expected error for non-TTY")
+	}
+	if !errors.Is(err, ui.ErrNotInteractive) {
+		t.Errorf("expected ErrNotInteractive, got: %v", err)
+	}
+	errMsg := err.Error()
+	if !strings.Contains(errMsg, "--target") || !strings.Contains(errMsg, "--group") || !strings.Contains(errMsg, "--favorite") {
+		t.Errorf("error should mention --target/--role, --group, and --favorite, got: %v", err)
+	}
+}
diff --git a/go.mod b/go.mod
@@ -6,6 +6,7 @@ require (
 	github.com/Iilun/survey/v2 v2.5.3
 	github.com/blang/semver v3.5.1+incompatible
 	github.com/cyberark/idsec-sdk-golang v0.1.14
+	github.com/mattn/go-isatty v0.0.20
 	github.com/rhysd/go-github-selfupdate v1.2.3
 	github.com/spf13/cobra v1.9.1
 	gopkg.in/yaml.v3 v3.0.1
@@ -36,7 +37,6 @@ require (
 	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
 	github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
 	github.com/mattn/go-colorable v0.1.14 // indirect
-	github.com/mattn/go-isatty v0.0.20 // indirect
 	github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect
 	github.com/mtibben/percent v0.2.1 // indirect
 	github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect
diff --git a/internal/ui/group_selector.go b/internal/ui/group_selector.go
@@ -47,6 +47,10 @@ func FindGroupByDisplay(groups []models.GroupsEligibleTarget, display string) (*
 // 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.
 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")
 	}
diff --git a/internal/ui/group_selector_test.go b/internal/ui/group_selector_test.go
@@ -1,6 +1,8 @@
 package ui
 
 import (
+	"errors"
+	"strings"
 	"testing"
 
 	"github.com/aaearon/grant-cli/internal/sca/models"
@@ -196,3 +198,25 @@ func TestFindGroupByDisplay(t *testing.T) {
 		})
 	}
 }
+
+func TestSelectGroup_NonTTY(t *testing.T) {
+	t.Parallel()
+	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)
+	}
+}
diff --git a/internal/ui/selector.go b/internal/ui/selector.go
@@ -65,6 +65,10 @@ func FindTargetByDisplay(targets []models.EligibleTarget, display string) (*mode
 
 // SelectTarget presents an interactive selector for choosing a target.
 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)
+	}
+
 	if len(targets) == 0 {
 		return nil, errors.New("no eligible targets available")
 	}
diff --git a/internal/ui/selector_test.go b/internal/ui/selector_test.go
@@ -1,6 +1,8 @@
 package ui
 
 import (
+	"errors"
+	"strings"
 	"testing"
 
 	"github.com/aaearon/grant-cli/internal/sca/models"
@@ -185,6 +187,28 @@ func TestBuildOptions(t *testing.T) {
 	}
 }
 
+func TestSelectTarget_NonTTY(t *testing.T) {
+	t.Parallel()
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+	IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	targets := []models.EligibleTarget{
+		{WorkspaceName: "Sub A", WorkspaceType: models.WorkspaceTypeSubscription, RoleInfo: models.RoleInfo{Name: "Owner"}},
+	}
+
+	_, err := SelectTarget(targets)
+	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(), "--target") {
+		t.Errorf("error should mention --target, got: %v", err)
+	}
+}
+
 func TestFindTargetByDisplay(t *testing.T) {
 	t.Parallel()
 	targets := []models.EligibleTarget{
diff --git a/internal/ui/session_selector.go b/internal/ui/session_selector.go
@@ -62,6 +62,10 @@ func FindSessionByDisplay(sessions []models.SessionInfo, nameMap map[string]stri
 
 // SelectSessions presents a multi-select prompt for choosing sessions to revoke.
 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)
+	}
+
 	if len(sessions) == 0 {
 		return nil, errors.New("no sessions available")
 	}
@@ -96,6 +100,10 @@ func SelectSessions(sessions []models.SessionInfo, nameMap map[string]string) ([
 
 // ConfirmRevocation prompts the user to confirm session revocation.
 func ConfirmRevocation(count int) (bool, error) {
+	if !IsInteractive() {
+		return false, fmt.Errorf("%w; use --yes to skip confirmation", ErrNotInteractive)
+	}
+
 	var confirmed bool
 	prompt := &survey.Confirm{
 		Message: fmt.Sprintf("Revoke %d session(s)?", count),
diff --git a/internal/ui/session_selector_test.go b/internal/ui/session_selector_test.go
@@ -1,6 +1,8 @@
 package ui
 
 import (
+	"errors"
+	"strings"
 	"testing"
 
 	"github.com/aaearon/grant-cli/internal/sca/models"
@@ -165,3 +167,43 @@ func TestFindSessionByDisplay(t *testing.T) {
 		}
 	})
 }
+
+func TestSelectSessions_NonTTY(t *testing.T) {
+	t.Parallel()
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+	IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	sessions := []models.SessionInfo{
+		{SessionID: "s1", RoleID: "Admin", WorkspaceID: "ws-a", SessionDuration: 3600},
+	}
+
+	_, err := SelectSessions(sessions, nil)
+	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(), "--all") {
+		t.Errorf("error should mention --all, got: %v", err)
+	}
+}
+
+func TestConfirmRevocation_NonTTY(t *testing.T) {
+	t.Parallel()
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+	IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	_, err := ConfirmRevocation(3)
+	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(), "--yes") {
+		t.Errorf("error should mention --yes, got: %v", err)
+	}
+}
diff --git a/internal/ui/tty.go b/internal/ui/tty.go
@@ -0,0 +1,23 @@
+package ui
+
+import (
+	"errors"
+	"os"
+
+	"github.com/mattn/go-isatty"
+)
+
+// IsTerminalFunc checks whether the given file descriptor is a terminal.
+// It is a variable so tests can override it.
+var IsTerminalFunc = func(fd uintptr) bool {
+	return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd)
+}
+
+// IsInteractive reports whether stdin is connected to a terminal.
+func IsInteractive() bool {
+	return IsTerminalFunc(os.Stdin.Fd())
+}
+
+// ErrNotInteractive is returned when an interactive prompt is attempted
+// without a terminal attached to stdin.
+var ErrNotInteractive = errors.New("interactive selection requires a terminal")
diff --git a/internal/ui/tty_test.go b/internal/ui/tty_test.go
@@ -0,0 +1,27 @@
+package ui
+
+import "testing"
+
+func TestIsInteractive_WhenTerminal(t *testing.T) {
+	t.Parallel()
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+
+	IsTerminalFunc = func(fd uintptr) bool { return true }
+
+	if !IsInteractive() {
+		t.Error("IsInteractive() = false, want true when terminal")
+	}
+}
+
+func TestIsInteractive_WhenNotTerminal(t *testing.T) {
+	t.Parallel()
+	original := IsTerminalFunc
+	defer func() { IsTerminalFunc = original }()
+
+	IsTerminalFunc = func(fd uintptr) bool { return false }
+
+	if IsInteractive() {
+		t.Error("IsInteractive() = true, want false when not terminal")
+	}
+}
