From 7f70c4d59d9b04bfbb960ea5a05869c78f44b862 Mon Sep 17 00:00:00 2001 From: Owais Jamil Date: Sat, 8 Nov 2025 10:39:25 -0600 Subject: [PATCH] feat(tty): implement TTY detection and error handling for non-interactive environments --- ROADMAP.md | 8 +- cmd/diff.go | 44 ++++++ cmd/generate.go | 6 +- cmd/unreleased.go | 8 ++ go.mod | 5 +- internal/diff/format.go | 4 + internal/docs/README.md | 111 ++++++++++++++- internal/tty/tty.go | 110 +++++++++++++++ internal/tty/tty_test.go | 288 +++++++++++++++++++++++++++++++++++++++ 9 files changed, 573 insertions(+), 11 deletions(-) create mode 100644 internal/tty/tty.go create mode 100644 internal/tty/tty_test.go diff --git a/ROADMAP.md b/ROADMAP.md index 0a0b692..f435057 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -108,10 +108,10 @@ Repository tagging and automation-friendly features. - [x] `release --dry-run` - [x] Show what would be written without writing - [x] Display preview of CHANGELOG changes with styled output -- [ ] Non-TTY environment handling - - [ ] Detect TTY availability - - [ ] Fallback to non-interactive mode - - [ ] CI-friendly error messages +- [x] Non-TTY environment handling + - [x] Detect TTY availability + - [x] Fallback to non-interactive mode + - [x] CI-friendly error messages - [ ] Add pre-commit hook examples - [ ] Validate commit message format - [ ] Ensure `.changes/` entries exist for features diff --git a/cmd/diff.go b/cmd/diff.go index d4c0837..aae0e5f 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -34,6 +34,7 @@ import ( "github.com/spf13/cobra" "github.com/stormlightlabs/git-storm/internal/diff" "github.com/stormlightlabs/git-storm/internal/gitlog" + "github.com/stormlightlabs/git-storm/internal/tty" "github.com/stormlightlabs/git-storm/internal/ui" ) @@ -124,6 +125,10 @@ func runDiff(fromRef, toRef, filePath string, expanded bool, view diff.DiffViewK }) } + if !tty.IsInteractive() { + return outputPlainDiff(allDiffs, expanded, view) + } + model := ui.NewMultiFileDiffModel(allDiffs, expanded, view) p := tea.NewProgram(model, tea.WithAltScreen()) @@ -144,3 +149,42 @@ func parseDiffView(viewName string) (diff.DiffViewKind, error) { return 0, fmt.Errorf("invalid view %q: expected one of split, unified", viewName) } } + +// outputPlainDiff outputs diffs in plain text format for non-interactive environments. +// +// TODO: move this to package [diff] +func outputPlainDiff(allDiffs []ui.FileDiff, expanded bool, view diff.DiffViewKind) error { + for i, fileDiff := range allDiffs { + fmt.Printf("=== File %d/%d ===\n", i+1, len(allDiffs)) + fmt.Printf("--- %s\n", fileDiff.OldPath) + fmt.Printf("+++ %s\n", fileDiff.NewPath) + fmt.Println() + + var formatter diff.Formatter + switch view { + case diff.ViewUnified: + formatter = &diff.UnifiedFormatter{ + TerminalWidth: 80, + ShowLineNumbers: true, + Expanded: expanded, + EnableWordWrap: false, + } + default: + formatter = &diff.SideBySideFormatter{ + TerminalWidth: 80, + ShowLineNumbers: true, + Expanded: expanded, + EnableWordWrap: false, + } + } + + output := formatter.Format(fileDiff.Edits) + fmt.Println(output) + + if i < len(allDiffs)-1 { + fmt.Println() + } + } + + return nil +} diff --git a/cmd/generate.go b/cmd/generate.go index 38a365c..700995e 100644 --- a/cmd/generate.go +++ b/cmd/generate.go @@ -22,6 +22,7 @@ import ( "github.com/stormlightlabs/git-storm/internal/changeset" "github.com/stormlightlabs/git-storm/internal/gitlog" "github.com/stormlightlabs/git-storm/internal/style" + "github.com/stormlightlabs/git-storm/internal/tty" "github.com/stormlightlabs/git-storm/internal/ui" ) @@ -74,6 +75,10 @@ entries in .changes/. Supports conventional commit parsing and interactive review mode.`, Args: cobra.MaximumNArgs(2), RunE: func(cmd *cobra.Command, args []string) error { + if interactive && !tty.IsInteractive() { + return tty.ErrorInteractiveFlag("--interactive") + } + var from, to string if sinceTag != "" { @@ -246,6 +251,5 @@ interactive review mode.`, c.Flags().BoolVarP(&interactive, "interactive", "i", false, "Review changes interactively in a TUI") c.Flags().StringVar(&sinceTag, "since", "", "Generate changes since the given tag") - return c } diff --git a/cmd/unreleased.go b/cmd/unreleased.go index 91cc8d5..27d95de 100644 --- a/cmd/unreleased.go +++ b/cmd/unreleased.go @@ -65,6 +65,7 @@ import ( "github.com/stormlightlabs/git-storm/internal/changeset" "github.com/stormlightlabs/git-storm/internal/gitlog" "github.com/stormlightlabs/git-storm/internal/style" + "github.com/stormlightlabs/git-storm/internal/tty" "github.com/stormlightlabs/git-storm/internal/ui" ) @@ -149,6 +150,13 @@ scope, and summary.`, Long: `Launches an interactive Bubble Tea TUI to review, edit, or categorize unreleased entries before final release.`, RunE: func(cmd *cobra.Command, args []string) error { + if !tty.IsInteractive() { + return tty.ErrorInteractiveRequired("storm unreleased review", []string{ + "Use 'storm unreleased list' to view entries in plain text", + "Use 'storm unreleased list --json' for JSON output", + }) + } + entries, err := changeset.List(changesDir) if err != nil { return fmt.Errorf("failed to list changelog entries: %w", err) diff --git a/go.mod b/go.mod index 7f7d428..bbac818 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,10 @@ require ( github.com/spf13/cobra v1.10.1 ) -require github.com/goccy/go-yaml v1.18.0 +require ( + github.com/goccy/go-yaml v1.18.0 + golang.org/x/term v0.36.0 +) require github.com/atotto/clipboard v0.1.4 // indirect diff --git a/internal/diff/format.go b/internal/diff/format.go index 82bccf2..17501d3 100644 --- a/internal/diff/format.go +++ b/internal/diff/format.go @@ -32,6 +32,10 @@ const ( compressedIndicator = "⋮" ) +type Formatter interface { + Format(edits []Edit) string +} + // SideBySideFormatter renders diff edits in a split-pane layout with syntax highlighting. type SideBySideFormatter struct { // TerminalWidth is the total available width for rendering diff --git a/internal/docs/README.md b/internal/docs/README.md index 6d79e46..14a63b4 100644 --- a/internal/docs/README.md +++ b/internal/docs/README.md @@ -1,13 +1,14 @@ --- title: Testing Workflow updated: 2025-11-08 -version: 2 +version: 3 --- "Ride the lightning." This document provides a comprehensive testing workflow for the `storm` changelog manager. -All tests should be run within this repository to validate functionality against real Git history. +All tests should be run within this repository to validate functionality against real Git +history. ## Setup @@ -16,6 +17,99 @@ All tests should be run within this repository to validate functionality against task build ``` +## Non-TTY Environment Handling + +Storm automatically detects whether it's running in an interactive terminal (TTY) or a +non-interactive environment (CI pipelines, scripts, pipes). Commands gracefully degrade +or provide helpful error messages. + +### TTY Detection + +The CLI checks for: + +- Terminal availability on stdin/stdout +- Common CI environment variables (GITHUB_ACTIONS, GITLAB_CI, CIRCLECI, etc.) + +### Command Behavior + +#### `generate --interactive` + +**Interactive (TTY):** Launches TUI for commit selection +**Non-Interactive:** Returns error with suggestion to use non-interactive mode + +```bash +# CI/Non-TTY +storm generate HEAD~5 HEAD --interactive +# Error: flag '--interactive' requires an interactive terminal (detected GitHub Actions environment) +``` + +**Workaround:** + +```bash +# Use without --interactive flag for automatic processing +storm generate HEAD~5 HEAD +``` + +#### `unreleased review` + +**Interactive (TTY):** Launches TUI for reviewing entries +**Non-Interactive:** Returns error with alternatives + +```bash +# CI/Non-TTY +storm unreleased review +# Error: command 'storm unreleased review' requires an interactive terminal (detected CI environment) +# +# Alternatives: +# - Use 'storm unreleased list' to view entries in plain text +# - Use 'storm unreleased list --json' for JSON output +``` + +#### `diff` + +**Interactive (TTY):** Launches TUI for navigating diffs +**Non-Interactive:** Outputs plain text diff to stdout + +```bash +# CI/Non-TTY - automatically outputs plain text +storm diff HEAD~1 HEAD +# === File 1/3 === +# --- HEAD~1:file.go +# +++ HEAD:file.go +# [plain text diff output] +``` + +### Testing Non-TTY Behavior + +#### Simulate CI environment + +```bash +CI=true storm unreleased review +# Should error with CI-friendly message +``` + +#### Pipe output + +```bash +storm diff HEAD~1 HEAD | less +# Should output plain text diff (not TUI) +``` + +#### Redirect to file + +```bash +storm diff HEAD~1 HEAD > changes.diff +# Should write plain text to file +``` + +**Expected Behaviors:** + +- Clear error messages indicating TTY requirement +- Suggestions for alternative commands +- CI system name detection (e.g., "detected GitHub Actions environment") +- Automatic fallback to plain text for `diff` command +- No ANSI escape codes in piped/redirected output + ## Core Workflow ### Manual Entry Creation (`unreleased add`) @@ -195,6 +289,7 @@ storm generate HEAD~10 HEAD --interactive - Allows selection/deselection - Creates only selected entries - Handles cancellation (Ctrl+C) +- Errors gracefully in non-TTY with helpful message #### Since tag @@ -323,7 +418,8 @@ storm unreleased review - Empty changes directory (should show message, not crash) - Corrupted entry file (should handle gracefully) -- Non-TTY environment (should detect and warn) +- Non-TTY environment (detects and errors with alternatives) +- CI environment (detects CI system name in error message) - Cancel review (Esc/q) - no changes applied - Delete file that no longer exists (should error gracefully) - Edit with empty fields (fields preserve original if empty) @@ -463,8 +559,10 @@ storm diff HEAD~1 HEAD **Expected:** -- Shows unified diff with syntax highlighting -- Iceberg theme colors +- TTY: Launches interactive TUI with navigation +- Non-TTY: Outputs plain text diff to stdout +- Shows diff with syntax highlighting (TTY only) +- Iceberg theme colors (TTY only) - Context lines displayed - File headers shown @@ -484,3 +582,6 @@ storm diff HEAD~1 HEAD -- "*.go" - No changes between refs - Binary files (should indicate) - Large diffs (should handle gracefully) +- Non-TTY environment (automatic plain text output) +- Piped output (plain text format) +- Redirected to file (plain text format) diff --git a/internal/tty/tty.go b/internal/tty/tty.go new file mode 100644 index 0000000..82e9554 --- /dev/null +++ b/internal/tty/tty.go @@ -0,0 +1,110 @@ +// package tty provides utilities for detecting terminal (TTY) availability and +// generating appropriate fallback behavior for non-interactive environments. +package tty + +import ( + "errors" + "fmt" + "os" + + "golang.org/x/term" +) + +// IsTTY checks if the given file descriptor is a terminal. +func IsTTY(fd uintptr) bool { + return term.IsTerminal(int(fd)) +} + +// IsInteractive checks if both stdin and stdout are connected to a terminal. +// This is the primary check for determining if TUI applications can run. +func IsInteractive() bool { + return IsTTY(os.Stdin.Fd()) && IsTTY(os.Stdout.Fd()) +} + +// IsCI detects if the current environment is a CI system by checking for common +// CI environment variables. +func IsCI() bool { + ciEnvVars := []string{ + "CI", // Generic CI indicator + "CONTINUOUS_INTEGRATION", + "GITHUB_ACTIONS", + "GITLAB_CI", + "CIRCLECI", + "TRAVIS", + "JENKINS_URL", + "BUILDKITE", + "DRONE", + "TEAMCITY_VERSION", + } + + for _, envVar := range ciEnvVars { + if os.Getenv(envVar) != "" { + return true + } + } + + return false +} + +// GetCIName attempts to identify the specific CI system being used. +func GetCIName() string { + ciMap := map[string]string{ + "GITHUB_ACTIONS": "GitHub Actions", + "GITLAB_CI": "GitLab CI", + "CIRCLECI": "CircleCI", + "TRAVIS": "Travis CI", + "JENKINS_URL": "Jenkins", + "BUILDKITE": "Buildkite", + "DRONE": "Drone CI", + "TEAMCITY_VERSION": "TeamCity", + } + + for envVar, name := range ciMap { + if os.Getenv(envVar) != "" { + return name + } + } + + if IsCI() { + return "CI" + } + + return "" +} + +// ErrorInteractiveRequired returns a formatted error message indicating that the +// command requires an interactive terminal, with suggestions for alternatives. +func ErrorInteractiveRequired(commandName string, alternatives []string) error { + msg := fmt.Sprintf("command '%s' requires an interactive terminal", commandName) + + if IsCI() { + ciName := GetCIName() + msg += fmt.Sprintf(" (detected %s environment)", ciName) + } else { + msg += " (stdin is not a TTY)" + } + + if len(alternatives) > 0 { + msg += "\n\nAlternatives:" + for _, alt := range alternatives { + msg += fmt.Sprintf("\n - %s", alt) + } + } + + return errors.New(msg) +} + +// ErrorInteractiveFlag returns a formatted error message indicating that an +// interactive flag cannot be used in a non-TTY environment. +func ErrorInteractiveFlag(flagName string) error { + msg := fmt.Sprintf("flag '%s' requires an interactive terminal", flagName) + + if IsCI() { + ciName := GetCIName() + msg += fmt.Sprintf(" (detected %s environment)", ciName) + } else { + msg += " (stdin is not a TTY)" + } + + return errors.New(msg) +} diff --git a/internal/tty/tty_test.go b/internal/tty/tty_test.go new file mode 100644 index 0000000..2dc1c97 --- /dev/null +++ b/internal/tty/tty_test.go @@ -0,0 +1,288 @@ +package tty + +import ( + "os" + "strings" + "testing" +) + +func TestIsCI(t *testing.T) { + tests := []struct { + name string + envVars map[string]string + expected bool + }{ + { + name: "no CI vars", + envVars: map[string]string{}, + expected: false, + }, + { + name: "generic CI var", + envVars: map[string]string{"CI": "true"}, + expected: true, + }, + { + name: "GitHub Actions", + envVars: map[string]string{"GITHUB_ACTIONS": "true"}, + expected: true, + }, + { + name: "GitLab CI", + envVars: map[string]string{"GITLAB_CI": "true"}, + expected: true, + }, + { + name: "CircleCI", + envVars: map[string]string{"CIRCLECI": "true"}, + expected: true, + }, + { + name: "multiple CI vars", + envVars: map[string]string{"CI": "true", "TRAVIS": "true"}, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ciEnvVars := []string{ + "CI", "CONTINUOUS_INTEGRATION", "GITHUB_ACTIONS", + "GITLAB_CI", "CIRCLECI", "TRAVIS", "JENKINS_URL", + "BUILDKITE", "DRONE", "TEAMCITY_VERSION", + } + for _, v := range ciEnvVars { + os.Unsetenv(v) + } + + for k, v := range tt.envVars { + os.Setenv(k, v) + } + + defer func() { + for k := range tt.envVars { + os.Unsetenv(k) + } + }() + + result := IsCI() + if result != tt.expected { + t.Errorf("IsCI() = %v, expected %v", result, tt.expected) + } + }) + } +} + +func TestGetCIName(t *testing.T) { + tests := []struct { + name string + envVar string + expected string + }{ + { + name: "GitHub Actions", + envVar: "GITHUB_ACTIONS", + expected: "GitHub Actions", + }, + { + name: "GitLab CI", + envVar: "GITLAB_CI", + expected: "GitLab CI", + }, + { + name: "CircleCI", + envVar: "CIRCLECI", + expected: "CircleCI", + }, + { + name: "Travis CI", + envVar: "TRAVIS", + expected: "Travis CI", + }, + { + name: "Jenkins", + envVar: "JENKINS_URL", + expected: "Jenkins", + }, + { + name: "Buildkite", + envVar: "BUILDKITE", + expected: "Buildkite", + }, + { + name: "Drone CI", + envVar: "DRONE", + expected: "Drone CI", + }, + { + name: "TeamCity", + envVar: "TEAMCITY_VERSION", + expected: "TeamCity", + }, + { + name: "Generic CI", + envVar: "CI", + expected: "CI", + }, + { + name: "No CI", + envVar: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ciEnvVars := []string{ + "CI", "CONTINUOUS_INTEGRATION", "GITHUB_ACTIONS", + "GITLAB_CI", "CIRCLECI", "TRAVIS", "JENKINS_URL", + "BUILDKITE", "DRONE", "TEAMCITY_VERSION", + } + for _, v := range ciEnvVars { + os.Unsetenv(v) + } + + if tt.envVar != "" { + os.Setenv(tt.envVar, "true") + } + + defer func() { + if tt.envVar != "" { + os.Unsetenv(tt.envVar) + } + }() + + result := GetCIName() + if result != tt.expected { + t.Errorf("GetCIName() = %q, expected %q", result, tt.expected) + } + }) + } +} + +func TestErrorInteractiveRequired(t *testing.T) { + tests := []struct { + name string + commandName string + alternatives []string + ciEnv string + wantContains []string + }{ + { + name: "basic error", + commandName: "review", + wantContains: []string{ + "command 'review' requires an interactive terminal", + }, + }, + { + name: "with alternatives", + commandName: "review", + alternatives: []string{ + "Use 'storm unreleased list' to view entries", + "Use 'storm unreleased list --json' for JSON output", + }, + wantContains: []string{ + "command 'review' requires an interactive terminal", + "Alternatives:", + "Use 'storm unreleased list' to view entries", + "Use 'storm unreleased list --json' for JSON output", + }, + }, + { + name: "CI environment", + commandName: "diff", + ciEnv: "GITHUB_ACTIONS", + wantContains: []string{ + "command 'diff' requires an interactive terminal", + "detected GitHub Actions environment", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ciEnvVars := []string{ + "CI", "CONTINUOUS_INTEGRATION", "GITHUB_ACTIONS", + "GITLAB_CI", "CIRCLECI", "TRAVIS", "JENKINS_URL", + "BUILDKITE", "DRONE", "TEAMCITY_VERSION", + } + for _, v := range ciEnvVars { + os.Unsetenv(v) + } + + if tt.ciEnv != "" { + os.Setenv(tt.ciEnv, "true") + defer os.Unsetenv(tt.ciEnv) + } + + err := ErrorInteractiveRequired(tt.commandName, tt.alternatives) + if err == nil { + t.Fatal("ErrorInteractiveRequired() returned nil, expected error") + } + + errMsg := err.Error() + for _, want := range tt.wantContains { + if !strings.Contains(errMsg, want) { + t.Errorf("ErrorInteractiveRequired() error message missing %q\nGot: %s", want, errMsg) + } + } + }) + } +} + +func TestErrorInteractiveFlag(t *testing.T) { + tests := []struct { + name string + flagName string + ciEnv string + wantContains []string + }{ + { + name: "basic error", + flagName: "--interactive", + wantContains: []string{ + "flag '--interactive' requires an interactive terminal", + }, + }, + { + name: "CI environment", + flagName: "--interactive", + ciEnv: "GITLAB_CI", + wantContains: []string{ + "flag '--interactive' requires an interactive terminal", + "detected GitLab CI environment", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ciEnvVars := []string{ + "CI", "CONTINUOUS_INTEGRATION", "GITHUB_ACTIONS", + "GITLAB_CI", "CIRCLECI", "TRAVIS", "JENKINS_URL", + "BUILDKITE", "DRONE", "TEAMCITY_VERSION", + } + for _, v := range ciEnvVars { + os.Unsetenv(v) + } + + if tt.ciEnv != "" { + os.Setenv(tt.ciEnv, "true") + defer os.Unsetenv(tt.ciEnv) + } + + err := ErrorInteractiveFlag(tt.flagName) + if err == nil { + t.Fatal("ErrorInteractiveFlag() returned nil, expected error") + } + + errMsg := err.Error() + for _, want := range tt.wantContains { + if !strings.Contains(errMsg, want) { + t.Errorf("ErrorInteractiveFlag() error message missing %q\nGot: %s", want, errMsg) + } + } + }) + } +} -- 2.51.2