diff --git a/internal/app/app.go b/internal/app/app.go index 9e0d5fb9b..802057bdd 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -28,6 +28,24 @@ import ( // Version is set from main via ldflags at build time. var Version = "dev" +// ErrNoTTYForTUI is returned when the TUI is requested from a non-interactive +// environment (scripts, CI, non-interactive SSH). Lists available non-TUI commands. +var ErrNoTTYForTUI = errors.New( + "gentle-ai: no TTY available — the interactive TUI cannot launch from scripts, CI, or non-interactive SSH.\n" + + "Available non-interactive commands:\n" + + " gentle-ai --version Print version\n" + + " gentle-ai --help Show all commands\n" + + " gentle-ai update Check for available updates\n" + + " gentle-ai install Configure AI coding agents\n" + + " gentle-ai sync Sync agent configs to current version\n" + + " gentle-ai doctor Run ecosystem health diagnostics") + +// stdinIsTerminal reports whether stdin is connected to a terminal. +// Uses the existing isattyFn seam from selfupdate.go for testability. +var stdinIsTerminal = func() bool { + return isattyFn(os.Stdin.Fd()) +} + var ( updateCheckAll = update.CheckAll updateCheckFiltered = update.CheckFiltered @@ -125,6 +143,13 @@ func RunArgs(args []string, stdout io.Writer) error { } } + // TTY guard: the interactive TUI requires a terminal. Non-interactive + // environments (scripts, CI, non-interactive SSH) get a clear error + // listing available non-TUI commands instead of a Bubbletea panic (#95). + if isTUIFlow && !stdinIsTerminal() { + return ErrNoTTYForTUI + } + if len(args) == 0 { homeDir, err := os.UserHomeDir() if err != nil { diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 9d0ed7aad..bd10fd2dc 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -3,6 +3,7 @@ package app import ( "bytes" "context" + "errors" "fmt" "io" "os" @@ -1135,11 +1136,13 @@ func TestRunArgs_TUISkipsSelfUpdate(t *testing.T) { origDetect := detectSystem origEnsure := ensureCurrentOSSupported origRunTUI := runTUI + origStdinIsTerminal := stdinIsTerminal t.Cleanup(func() { selfUpdateFn = origSelfUpdate detectSystem = origDetect ensureCurrentOSSupported = origEnsure runTUI = origRunTUI + stdinIsTerminal = origStdinIsTerminal }) ensureCurrentOSSupported = func() error { return nil } @@ -1147,6 +1150,9 @@ func TestRunArgs_TUISkipsSelfUpdate(t *testing.T) { return system.DetectionResult{System: system.SystemInfo{Supported: true}}, nil } + // Simulate a TTY so the TTY guard doesn't block the TUI flow. + stdinIsTerminal = func() bool { return true } + // Return the same model to avoid nil dereference if RunArgs inspects it. tuiCalled := 0 runTUI = func(m tea.Model, _ ...tea.ProgramOption) (tea.Model, error) { @@ -1173,6 +1179,91 @@ func TestRunArgs_TUISkipsSelfUpdate(t *testing.T) { } } +func TestRunArgs_NoTTY_ReturnsErrNoTTYForTUI(t *testing.T) { + // NOTE: modifies package-level vars; must not run in parallel. + origStdinIsTerminal := stdinIsTerminal + origEnsure := ensureCurrentOSSupported + origDetect := detectSystem + t.Cleanup(func() { + stdinIsTerminal = origStdinIsTerminal + ensureCurrentOSSupported = origEnsure + detectSystem = origDetect + }) + + // Stub OS/support and system-detection seams to isolate TTY behavior. + ensureCurrentOSSupported = func() error { return nil } + detectSystem = func(context.Context) (system.DetectionResult, error) { + return system.DetectionResult{System: system.SystemInfo{Supported: true}}, nil + } + + // Simulate a non-interactive environment (no TTY). + stdinIsTerminal = func() bool { return false } + + var buf bytes.Buffer + err := RunArgs([]string{}, &buf) + if err == nil { + t.Fatal("RunArgs(empty args) with no TTY: expected error, got nil") + } + if !errors.Is(err, ErrNoTTYForTUI) { + t.Fatalf("RunArgs(empty args) with no TTY: error = %v; want ErrNoTTYForTUI", err) + } + if !strings.Contains(err.Error(), "no TTY available") { + t.Fatalf("RunArgs(empty args) with no TTY: error = %v; want error containing 'no TTY available'", err) + } + if !strings.Contains(err.Error(), "--version") { + t.Fatalf("RunArgs(empty args) with no TTY: error should list --version command; got %v", err) + } + if !strings.Contains(err.Error(), "--help") { + t.Fatalf("RunArgs(empty args) with no TTY: error should list --help command; got %v", err) + } +} + +func TestRunArgs_NoTTY_StillAllowsVersionAndHelp(t *testing.T) { + // NOTE: modifies package-level vars; must not run in parallel. + origStdinIsTerminal := stdinIsTerminal + origEnsure := ensureCurrentOSSupported + origDetect := detectSystem + t.Cleanup(func() { + stdinIsTerminal = origStdinIsTerminal + ensureCurrentOSSupported = origEnsure + detectSystem = origDetect + }) + + // Stub OS/support and system-detection seams to isolate TTY behavior. + ensureCurrentOSSupported = func() error { return nil } + detectSystem = func(context.Context) (system.DetectionResult, error) { + return system.DetectionResult{System: system.SystemInfo{Supported: true}}, nil + } + + // Simulate a non-interactive environment (no TTY). + stdinIsTerminal = func() bool { return false } + + tests := []struct { + name string + args []string + }{ + {name: "version", args: []string{"version"}}, + {name: "--version", args: []string{"--version"}}, + {name: "-v", args: []string{"-v"}}, + {name: "help", args: []string{"help"}}, + {name: "--help", args: []string{"--help"}}, + {name: "-h", args: []string{"-h"}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + err := RunArgs(tc.args, &buf) + if err != nil { + t.Fatalf("RunArgs(%v) with no TTY: unexpected error = %v", tc.args, err) + } + if buf.Len() == 0 { + t.Fatalf("RunArgs(%v) with no TTY: expected output, got empty buffer", tc.args) + } + }) + } +} + func TestIsExplicitUpdateFlow(t *testing.T) { tests := []struct { name string @@ -1485,10 +1576,12 @@ func TestRunArgs_TUIRestartsAfterGentleAIUpgradeResult(t *testing.T) { origDetect := detectSystem origEnsure := ensureCurrentOSSupported origRunTUI := runTUI + origStdinIsTerminal := stdinIsTerminal t.Cleanup(func() { detectSystem = origDetect ensureCurrentOSSupported = origEnsure runTUI = origRunTUI + stdinIsTerminal = origStdinIsTerminal unsetEnv(t, envSelfUpdateDone) }) unsetEnv(t, envSelfUpdateDone) @@ -1498,6 +1591,9 @@ func TestRunArgs_TUIRestartsAfterGentleAIUpgradeResult(t *testing.T) { return system.DetectionResult{System: system.SystemInfo{Supported: true, Profile: system.PlatformProfile{OS: "darwin", PackageManager: "brew", Supported: true}}}, nil } + // Simulate a TTY so the TTY guard doesn't block the TUI flow. + stdinIsTerminal = func() bool { return true } + report := upgrade.UpgradeReport{Results: []upgrade.ToolUpgradeResult{ {ToolName: "gentle-ai", Status: upgrade.UpgradeSucceeded, NewVersion: "v1.40.0"}, }} @@ -1539,12 +1635,14 @@ func TestRunArgs_PendingSync_RunsSyncAndClearsFlag(t *testing.T) { origDetect := detectSystem origRunTUI := runTUI origDeferredSync := deferredSyncFn + origStdinIsTerminal := stdinIsTerminal t.Cleanup(func() { selfUpdateFn = origSelf ensureCurrentOSSupported = origEnsure detectSystem = origDetect runTUI = origRunTUI deferredSyncFn = origDeferredSync + stdinIsTerminal = origStdinIsTerminal }) selfUpdateFn = func(_ context.Context, _ string, _ system.PlatformProfile, _ io.Writer) error { @@ -1554,6 +1652,8 @@ func TestRunArgs_PendingSync_RunsSyncAndClearsFlag(t *testing.T) { detectSystem = func(context.Context) (system.DetectionResult, error) { return system.DetectionResult{System: system.SystemInfo{Supported: true, Profile: system.PlatformProfile{OS: "darwin", PackageManager: "brew", Supported: true}}}, nil } + // Simulate a TTY so the TTY guard doesn't block the TUI flow. + stdinIsTerminal = func() bool { return true } runTUI = func(m tea.Model, _ ...tea.ProgramOption) (tea.Model, error) { return m, nil } @@ -1601,12 +1701,14 @@ func TestRunArgs_PendingSync_LeavesSetOnFailure(t *testing.T) { origDetect := detectSystem origRunTUI := runTUI origDeferredSync := deferredSyncFn + origStdinIsTerminal := stdinIsTerminal t.Cleanup(func() { selfUpdateFn = origSelf ensureCurrentOSSupported = origEnsure detectSystem = origDetect runTUI = origRunTUI deferredSyncFn = origDeferredSync + stdinIsTerminal = origStdinIsTerminal }) selfUpdateFn = func(_ context.Context, _ string, _ system.PlatformProfile, _ io.Writer) error { @@ -1616,6 +1718,8 @@ func TestRunArgs_PendingSync_LeavesSetOnFailure(t *testing.T) { detectSystem = func(context.Context) (system.DetectionResult, error) { return system.DetectionResult{System: system.SystemInfo{Supported: true, Profile: system.PlatformProfile{OS: "darwin", PackageManager: "brew", Supported: true}}}, nil } + // Simulate a TTY so the TTY guard doesn't block the TUI flow. + stdinIsTerminal = func() bool { return true } runTUI = func(m tea.Model, _ ...tea.ProgramOption) (tea.Model, error) { return m, nil } @@ -1677,12 +1781,14 @@ func TestRunArgs_PendingSync_ClearWriteFailureIsLogged(t *testing.T) { origDetect := detectSystem origRunTUI := runTUI origDeferredSync := deferredSyncFn + origStdinIsTerminal := stdinIsTerminal t.Cleanup(func() { selfUpdateFn = origSelf ensureCurrentOSSupported = origEnsure detectSystem = origDetect runTUI = origRunTUI deferredSyncFn = origDeferredSync + stdinIsTerminal = origStdinIsTerminal }) selfUpdateFn = func(_ context.Context, _ string, _ system.PlatformProfile, _ io.Writer) error { @@ -1692,6 +1798,8 @@ func TestRunArgs_PendingSync_ClearWriteFailureIsLogged(t *testing.T) { detectSystem = func(context.Context) (system.DetectionResult, error) { return system.DetectionResult{System: system.SystemInfo{Supported: true, Profile: system.PlatformProfile{OS: "darwin", PackageManager: "brew", Supported: true}}}, nil } + // Simulate a TTY so the TTY guard doesn't block the TUI flow. + stdinIsTerminal = func() bool { return true } runTUI = func(m tea.Model, _ ...tea.ProgramOption) (tea.Model, error) { return m, nil } @@ -1730,12 +1838,14 @@ func TestRunArgs_NoPendingSync_NoSyncCall(t *testing.T) { origDetect := detectSystem origRunTUI := runTUI origDeferredSync := deferredSyncFn + origStdinIsTerminal := stdinIsTerminal t.Cleanup(func() { selfUpdateFn = origSelf ensureCurrentOSSupported = origEnsure detectSystem = origDetect runTUI = origRunTUI deferredSyncFn = origDeferredSync + stdinIsTerminal = origStdinIsTerminal }) selfUpdateFn = func(_ context.Context, _ string, _ system.PlatformProfile, _ io.Writer) error { @@ -1745,6 +1855,8 @@ func TestRunArgs_NoPendingSync_NoSyncCall(t *testing.T) { detectSystem = func(context.Context) (system.DetectionResult, error) { return system.DetectionResult{System: system.SystemInfo{Supported: true, Profile: system.PlatformProfile{OS: "darwin", PackageManager: "brew", Supported: true}}}, nil } + // Simulate a TTY so the TTY guard doesn't block the TUI flow. + stdinIsTerminal = func() bool { return true } runTUI = func(m tea.Model, _ ...tea.ProgramOption) (tea.Model, error) { return m, nil } diff --git a/internal/app/parity_test.go b/internal/app/parity_test.go index 18fed5fd1..8405c939f 100644 --- a/internal/app/parity_test.go +++ b/internal/app/parity_test.go @@ -163,7 +163,15 @@ func TestGuardFlowLinuxDryRunPropagatesDecision(t *testing.T) { func TestRunArgsNoCommandLaunchesTUI(t *testing.T) { origRunTUI := runTUI - t.Cleanup(func() { runTUI = origRunTUI }) + origStdinIsTerminal := stdinIsTerminal + t.Cleanup(func() { + runTUI = origRunTUI + stdinIsTerminal = origStdinIsTerminal + }) + + // Simulate a TTY so the TTY guard doesn't block the TUI flow. + stdinIsTerminal = func() bool { return true } + runTUI = func(m tea.Model, opts ...tea.ProgramOption) (tea.Model, error) { return nil, errors.New("mock TUI error: no TTY") } diff --git a/internal/update/upgrade/executor.go b/internal/update/upgrade/executor.go index d9583d47d..3e391db9d 100644 --- a/internal/update/upgrade/executor.go +++ b/internal/update/upgrade/executor.go @@ -517,10 +517,12 @@ func ExecuteWithOptions(ctx context.Context, results []update.UpdateResult, prof // Executable tools: run upgrade strategy. for _, r := range executable { + // Resolve method once per tool — effectiveMethod may run an external brew probe, + // so calling it multiple times adds process overhead and risks inconsistent routing. method := effectiveMethod(r.Tool, profile) msg := fmt.Sprintf("Upgrading %s via %s (%s → %s)", r.Tool.Name, method, r.InstalledVersion, r.LatestVersion) sp := NewSpinner(pw, msg) - toolResult := executeOne(ctx, r, profile, dryRun) + toolResult := executeOne(ctx, r, profile, dryRun, method) // Check if the upgrade succeeded but requires immediate exit (Windows self-replace). // This must be handled BEFORE calling sp.Finish() so the spinner can terminate properly. @@ -567,12 +569,13 @@ func detectCommandHint(tool update.ToolInfo) string { } // executeOne runs the upgrade for a single tool. -func executeOne(ctx context.Context, r update.UpdateResult, profile system.PlatformProfile, dryRun bool) ToolUpgradeResult { +// method must be pre-resolved by the caller to avoid redundant effectiveMethod calls. +func executeOne(ctx context.Context, r update.UpdateResult, profile system.PlatformProfile, dryRun bool, method update.InstallMethod) ToolUpgradeResult { base := ToolUpgradeResult{ ToolName: r.Tool.Name, OldVersion: r.InstalledVersion, NewVersion: r.LatestVersion, - Method: effectiveMethod(r.Tool, profile), + Method: method, } if dryRun { @@ -580,7 +583,7 @@ func executeOne(ctx context.Context, r update.UpdateResult, profile system.Platf return base } - exitReq, err := runStrategy(ctx, r, profile) + exitReq, err := runStrategy(ctx, r, profile, method) if err != nil { // Distinguish manual fallback (informational skip) from real failures. if hint, ok := AsManualFallback(err); ok { diff --git a/internal/update/upgrade/strategy.go b/internal/update/upgrade/strategy.go index d14027922..c89030b21 100644 --- a/internal/update/upgrade/strategy.go +++ b/internal/update/upgrade/strategy.go @@ -64,13 +64,11 @@ const maxScriptSize = 1 * 1024 * 1024 // 1 MB // - script method + windows → manualFallback // - OpenCode plugin method → update materialized package in ~/.config/opencode when possible // - unknown method → manualFallback with explicit message -func runStrategy(ctx context.Context, r update.UpdateResult, profile system.PlatformProfile) (bool, error) { +func runStrategy(ctx context.Context, r update.UpdateResult, profile system.PlatformProfile, method update.InstallMethod) (bool, error) { if isBetaGentleAIUpgrade(r) && profile.OS != "windows" { return false, goInstallMainUpgrade(r.Tool) } - method := effectiveMethod(r.Tool, profile) - switch method { case update.InstallBrew: return false, brewUpgrade(ctx, r.Tool.Name) diff --git a/internal/update/upgrade/strategy_test.go b/internal/update/upgrade/strategy_test.go index b844123e4..333d37d9a 100644 --- a/internal/update/upgrade/strategy_test.go +++ b/internal/update/upgrade/strategy_test.go @@ -38,7 +38,8 @@ func TestRunStrategy_BrewUpgrade(t *testing.T) { } profile := system.PlatformProfile{OS: "darwin", PackageManager: "brew"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err != nil { t.Fatalf("runStrategy brew: unexpected error: %v", err) } @@ -75,7 +76,8 @@ func TestRunStrategy_GoInstallUpgrade(t *testing.T) { } profile := system.PlatformProfile{OS: "linux", PackageManager: "apt"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err != nil { t.Fatalf("runStrategy go-install: unexpected error: %v", err) } @@ -116,7 +118,8 @@ func TestRunStrategy_BetaGentleAISelfUpgradeUsesGoInstallMain(t *testing.T) { } profile := system.PlatformProfile{OS: "linux", PackageManager: "apt", Supported: true} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err != nil { t.Fatalf("runStrategy beta gentle-ai: unexpected error: %v", err) } @@ -182,7 +185,8 @@ func TestRunStrategy_GoInstallMissingImportPath(t *testing.T) { } profile := system.PlatformProfile{OS: "linux", PackageManager: "apt"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err == nil { t.Errorf("expected error when GoImportPath is empty, got nil") } @@ -200,7 +204,8 @@ func TestRunStrategy_UnsupportedMethodManualFallback(t *testing.T) { } profile := system.PlatformProfile{OS: "linux", PackageManager: "apt"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) // Unsupported method → manual fallback error. if err == nil { t.Errorf("expected error for unsupported install method, got nil") @@ -226,7 +231,8 @@ func TestRunStrategy_BrewUpgradeFailure(t *testing.T) { } profile := system.PlatformProfile{OS: "darwin", PackageManager: "brew"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err == nil { t.Errorf("expected error when brew upgrade fails, got nil") } @@ -252,7 +258,8 @@ func TestRunStrategy_GoInstallFailure(t *testing.T) { } profile := system.PlatformProfile{OS: "linux", PackageManager: "apt"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err == nil { t.Errorf("expected error when go install fails, got nil") } @@ -451,14 +458,17 @@ func TestRunStrategyOpenCodePluginManualFallback(t *testing.T) { return mockCmd("echo", "should not run") } - _, err := runStrategy(context.Background(), update.UpdateResult{ + _r := update.UpdateResult{ Tool: update.ToolInfo{ Name: "opencode-subagent-statusline", InstallMethod: update.InstallOpenCodePlugin, NpmPackage: "opencode-subagent-statusline", }, UpdateHint: "restart OpenCode", - }, system.PlatformProfile{PackageManager: "brew"}) + } + _profile := system.PlatformProfile{PackageManager: "brew"} + _method := effectiveMethod(_r.Tool, _profile) + _, err := runStrategy(context.Background(), _r, _profile, _method) if err == nil { t.Fatal("expected manual fallback error, got nil") @@ -524,7 +534,7 @@ func TestRunStrategyOpenCodePluginUpgradesMaterializedPackage(t *testing.T) { return cmd } - _, err := runStrategy(context.Background(), update.UpdateResult{ + _r2 := update.UpdateResult{ Tool: update.ToolInfo{ Name: pkg, InstallMethod: update.InstallOpenCodePlugin, @@ -532,7 +542,10 @@ func TestRunStrategyOpenCodePluginUpgradesMaterializedPackage(t *testing.T) { }, InstalledVersion: "0.1.0", LatestVersion: "0.2.0", - }, system.PlatformProfile{PackageManager: "brew"}) + } + _profile2 := system.PlatformProfile{PackageManager: "brew"} + _method2 := effectiveMethod(_r2.Tool, _profile2) + _, err := runStrategy(context.Background(), _r2, _profile2, _method2) if err != nil { t.Fatalf("runStrategy OpenCode plugin: unexpected error: %v", err) } @@ -604,14 +617,17 @@ func TestRunStrategyOpenCodePluginRegisteredPendingRunsPackageManager(t *testing return mockCmd("true") } - _, err := runStrategy(context.Background(), update.UpdateResult{ + _r3 := update.UpdateResult{ Tool: update.ToolInfo{ Name: pkg, InstallMethod: update.InstallOpenCodePlugin, NpmPackage: pkg, }, Status: update.RegisteredNotMaterialized, - }, system.PlatformProfile{}) + } + _profile3 := system.PlatformProfile{} + _method3 := effectiveMethod(_r3.Tool, _profile3) + _, err := runStrategy(context.Background(), _r3, _profile3, _method3) if err != nil { t.Fatalf("registered OpenCode plugin should be npm-managed during upgrade, got: %v", err) } @@ -649,13 +665,16 @@ func TestRunStrategyOpenCodePluginFallsBackWithoutPackageManager(t *testing.T) { return mockCmd("echo", "should not run") } - _, err := runStrategy(context.Background(), update.UpdateResult{ + _r4 := update.UpdateResult{ Tool: update.ToolInfo{ Name: pkg, InstallMethod: update.InstallOpenCodePlugin, NpmPackage: pkg, }, - }, system.PlatformProfile{}) + } + _profile4 := system.PlatformProfile{} + _method4 := effectiveMethod(_r4.Tool, _profile4) + _, err := runStrategy(context.Background(), _r4, _profile4, _method4) if err == nil { t.Fatal("expected manual fallback when bun/npm are unavailable, got nil") } @@ -950,7 +969,8 @@ func TestRunStrategy_ExecErrorWrapped(t *testing.T) { } profile := system.PlatformProfile{OS: "darwin", PackageManager: "brew"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err == nil { t.Fatal("expected error, got nil") } @@ -1258,7 +1278,8 @@ func TestRunStrategy_GGAUsesGitClone(t *testing.T) { } profile := system.PlatformProfile{OS: "linux", PackageManager: "apt"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err != nil { t.Fatalf("runStrategy GGA: unexpected error: %v", err) } @@ -1375,7 +1396,8 @@ func TestEngramUpgradeUsesDownloadNotGoInstall(t *testing.T) { } profile := system.PlatformProfile{OS: "windows", PackageManager: "winget"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err != nil { t.Fatalf("runStrategy engram windows: unexpected error: %v", err) } @@ -1424,7 +1446,8 @@ func TestEngramUpgradeLinuxUsesDownload(t *testing.T) { } profile := system.PlatformProfile{OS: "linux", PackageManager: "apt"} - _, err := runStrategy(context.Background(), r, profile) + method := effectiveMethod(r.Tool, profile) + _, err := runStrategy(context.Background(), r, profile, method) if err != nil { t.Fatalf("runStrategy engram linux: unexpected error: %v", err) }