From f722ba2bc525102a8d27b21ff5fb3da61de59394 Mon Sep 17 00:00:00 2001 From: Rafael Garcia Date: Fri, 24 Jul 2026 11:26:55 -0400 Subject: [PATCH 1/5] Fail fast on interactive prompts in non-interactive shells Interactive pterm prompts (confirm/select/text input) never return when stdin is not a terminal: the underlying keyboard listener swallows read errors and spins forever, re-rendering the prompt in a tight loop. AI agents and CI scripts that invoke a prompting command hang until killed (verified: 'kernel create < /dev/null' produced ~13MB of ANSI output in 10s and never exited). Every prompt call site is now gated on a stdin TTY check and returns a specific, actionable error instead: - Confirmation prompts (all delete commands, the browsers-create pool conflict confirm, and the create overwrite confirm) direct the caller to re-run with --yes. - Text/select prompts (kernel create name/language/template, browsers create --viewport-interactive) name the exact flag and valid values to pass instead. New in this change: - pkg/interactive: IsInteractive() plus the two shared error builders. - kernel create --yes/-y: skip the overwrite-existing-directory confirm (the only confirm that had no --yes bypass, along with the pool conflict confirm in browsers create, which also gains --yes). - Invalid --name/--language/--template values now error in non-interactive shells instead of silently falling back to a prompt. kernel login, browsers ssh, and upgrade are intentionally left alone; they are usable by agents as-is. --- cmd/api_keys.go | 4 ++ cmd/app.go | 4 ++ cmd/auth_connections.go | 4 ++ cmd/browsers.go | 20 +++++-- cmd/create.go | 31 +++++++---- cmd/credential_providers.go | 4 ++ cmd/credentials.go | 4 ++ cmd/deploy.go | 4 ++ cmd/extensions.go | 4 ++ cmd/noninteractive_test.go | 47 ++++++++++++++++ cmd/profiles.go | 4 ++ cmd/proxies/delete.go | 4 ++ pkg/create/prompts.go | 59 +++++++++++++++++++- pkg/create/prompts_test.go | 85 +++++++++++++++++++++++++++++ pkg/create/types.go | 3 + pkg/interactive/interactive.go | 38 +++++++++++++ pkg/interactive/interactive_test.go | 28 ++++++++++ 17 files changed, 330 insertions(+), 17 deletions(-) create mode 100644 cmd/noninteractive_test.go create mode 100644 pkg/create/prompts_test.go create mode 100644 pkg/interactive/interactive.go create mode 100644 pkg/interactive/interactive_test.go diff --git a/cmd/api_keys.go b/cmd/api_keys.go index 47cc8544..535622dc 100644 --- a/cmd/api_keys.go +++ b/cmd/api_keys.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/util" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" @@ -181,6 +182,9 @@ func (c APIKeysCmd) Update(ctx context.Context, in APIKeysUpdateInput) error { func (c APIKeysCmd) Delete(ctx context.Context, in APIKeysDeleteInput) error { if !in.SkipConfirm { + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("delete API key '%s'", in.ID)) + } msg := fmt.Sprintf("Are you sure you want to delete API key '%s'?", in.ID) pterm.DefaultInteractiveConfirm.DefaultText = msg ok, _ := pterm.DefaultInteractiveConfirm.Show() diff --git a/cmd/app.go b/cmd/app.go index 7271dcc5..b665c29f 100644 --- a/cmd/app.go +++ b/cmd/app.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/util" "github.com/kernel/kernel-go-sdk" "github.com/pterm/pterm" @@ -256,6 +257,9 @@ func runAppDelete(cmd *cobra.Command, args []string) error { if version != "" { scope = fmt.Sprintf("version '%s'", version) } + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("delete all deployments for app '%s' (%s)", appName, scope)) + } msg := fmt.Sprintf("Delete all deployments for app '%s' (%s)? This cannot be undone.", appName, scope) pterm.DefaultInteractiveConfirm.DefaultText = msg ok, _ := pterm.DefaultInteractiveConfirm.Show() diff --git a/cmd/auth_connections.go b/cmd/auth_connections.go index 153a4981..58aac050 100644 --- a/cmd/auth_connections.go +++ b/cmd/auth_connections.go @@ -6,6 +6,7 @@ import ( "strings" "time" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/util" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" @@ -491,6 +492,9 @@ func (c AuthConnectionCmd) List(ctx context.Context, in AuthConnectionListInput) func (c AuthConnectionCmd) Delete(ctx context.Context, in AuthConnectionDeleteInput) error { if !in.SkipConfirm { + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("delete managed auth '%s'", in.ID)) + } msg := fmt.Sprintf("Are you sure you want to delete managed auth '%s'?", in.ID) pterm.DefaultInteractiveConfirm.DefaultText = msg ok, _ := pterm.DefaultInteractiveConfirm.Show() diff --git a/cmd/browsers.go b/cmd/browsers.go index 28f1aae3..ab825022 100644 --- a/cmd/browsers.go +++ b/cmd/browsers.go @@ -17,6 +17,7 @@ import ( "strings" "time" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/table" "github.com/kernel/cli/pkg/util" "github.com/kernel/kernel-go-sdk" @@ -2713,6 +2714,7 @@ func init() { browsersCreateCmd.Flags().String("telemetry", "", "Configure telemetry (opt-in): --telemetry=all (default set), --telemetry=off (disable), or --telemetry=console,network (capture exactly those categories)") browsersCreateCmd.Flags().String("name", "", "Optional unique name for the browser session (used to find it later; can be changed with 'browsers update --name')") browsersCreateCmd.Flags().StringArray("tag", nil, "Set a tag KEY=VALUE on the session (repeatable; up to 50 pairs)") + browsersCreateCmd.Flags().BoolP("yes", "y", false, "Skip confirmation prompts") browsersCreateCmd.Flags().String("chrome-policy", "", "Custom Chrome enterprise policy as a JSON object") browsersCreateCmd.Flags().String("chrome-policy-file", "", "Read Chrome enterprise policy (JSON object) from a file (use '-' for stdin)") browsersCreateCmd.MarkFlagsMutuallyExclusive("chrome-policy", "chrome-policy-file") @@ -2803,6 +2805,7 @@ func poolLeaseAllowedFlags() map[string]bool { "tag": true, "telemetry": true, "output": true, + "yes": true, // Global persistent flags that don't configure browsers "no-color": true, "log-level": true, @@ -2835,6 +2838,7 @@ func runBrowsersCreate(cmd *cobra.Command, args []string) error { chromePolicy, _ := cmd.Flags().GetString("chrome-policy") chromePolicyFile, _ := cmd.Flags().GetString("chrome-policy-file") output, _ := cmd.Flags().GetString("output") + skipConfirm, _ := cmd.Flags().GetBool("yes") if poolID != "" && poolName != "" { pterm.Error.Println("must specify at most one of --pool-id or --pool-name") @@ -2863,10 +2867,15 @@ func runBrowsersCreate(cmd *cobra.Command, args []string) error { pterm.Info.Println("When using a pool, all browser configuration comes from the pool itself.") pterm.Info.Println("The conflicting flags will be ignored.") - result, _ := pterm.DefaultInteractiveConfirm.Show("Continue with pool configuration?") - if !result { - pterm.Info.Println("Cancelled. Remove conflicting flags or omit the pool flag.") - return nil + if !skipConfirm { + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("ignore conflicting browser configuration flags (%s) and continue with %s", strings.Join(conflicts, ", "), flagLabel)) + } + result, _ := pterm.DefaultInteractiveConfirm.Show("Continue with pool configuration?") + if !result { + pterm.Info.Println("Cancelled. Remove conflicting flags or omit the pool flag.") + return nil + } } pterm.Success.Println("Proceeding with pool configuration...") } @@ -2911,6 +2920,9 @@ func runBrowsersCreate(cmd *cobra.Command, args []string) error { // Handle interactive viewport selection if viewportInteractive { + if !interactive.IsInteractive() { + return interactive.ErrInputRequired("viewport selection", "pass --viewport instead of --viewport-interactive (e.g. --viewport 1920x1080@25)") + } if viewport != "" { pterm.Warning.Println("Both --viewport and --viewport-interactive specified; using interactive mode") } diff --git a/cmd/create.go b/cmd/create.go index bf101ef3..9e948c54 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -25,14 +25,16 @@ func (c CreateCmd) Create(ctx context.Context, ci create.CreateInput) error { // Check if directory already exists and prompt for overwrite if _, err := os.Stat(appPath); err == nil { - overwrite, err := create.PromptForOverwrite(ci.Name) - if err != nil { - return fmt.Errorf("failed to prompt for overwrite: %w", err) - } - - if !overwrite { - pterm.Warning.Println("Operation cancelled.") - return nil + if !ci.SkipConfirm { + overwrite, err := create.PromptForOverwrite(ci.Name) + if err != nil { + return err + } + + if !overwrite { + pterm.Warning.Println("Operation cancelled.") + return nil + } } // Remove existing directory @@ -81,6 +83,7 @@ func init() { createCmd.Flags().StringP("name", "n", "", "Name of the application") createCmd.Flags().StringP("language", "l", "", fmt.Sprintf("Language of the application (%s)", strings.Join(supportedLanguageDisplay(), ", "))) createCmd.Flags().StringP("template", "t", "", "Template to use for the application (see 'kernel create --help' for the full list)") + createCmd.Flags().BoolP("yes", "y", false, "Skip confirmation prompts (overwrite an existing directory without asking)") } // supportedLanguageDisplay returns each supported language with its shorthand, @@ -104,7 +107,9 @@ func buildCreateLongHelp() string { var b strings.Builder b.WriteString("Commands for creating new Kernel applications.\n\n") b.WriteString("Pass --name, --language and --template to scaffold non-interactively;\n") - b.WriteString("any omitted flag falls back to an interactive prompt.\n\n") + b.WriteString("any omitted flag falls back to an interactive prompt. In a\n") + b.WriteString("non-interactive shell the command fails fast instead of prompting.\n") + b.WriteString("Pass --yes to overwrite an existing directory without confirmation.\n\n") b.WriteString("Languages:\n") for _, l := range create.SupportedLanguages { @@ -143,6 +148,7 @@ func runCreateApp(cmd *cobra.Command, args []string) error { appName, _ := cmd.Flags().GetString("name") language, _ := cmd.Flags().GetString("language") template, _ := cmd.Flags().GetString("template") + skipConfirm, _ := cmd.Flags().GetBool("yes") appName, err := create.PromptForAppName(appName) if err != nil { @@ -161,8 +167,9 @@ func runCreateApp(cmd *cobra.Command, args []string) error { c := CreateCmd{} return c.Create(cmd.Context(), create.CreateInput{ - Name: appName, - Language: language, - Template: template, + Name: appName, + Language: language, + Template: template, + SkipConfirm: skipConfirm, }) } diff --git a/cmd/credential_providers.go b/cmd/credential_providers.go index 5eca81ed..49d96225 100644 --- a/cmd/credential_providers.go +++ b/cmd/credential_providers.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/util" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" @@ -254,6 +255,9 @@ func (c CredentialProvidersCmd) Update(ctx context.Context, in CredentialProvide func (c CredentialProvidersCmd) Delete(ctx context.Context, in CredentialProvidersDeleteInput) error { if !in.SkipConfirm { + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("delete credential provider '%s'", in.ID)) + } msg := fmt.Sprintf("Are you sure you want to delete credential provider '%s'?", in.ID) pterm.DefaultInteractiveConfirm.DefaultText = msg ok, _ := pterm.DefaultInteractiveConfirm.Show() diff --git a/cmd/credentials.go b/cmd/credentials.go index b17a37e7..dbb9455a 100644 --- a/cmd/credentials.go +++ b/cmd/credentials.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/util" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" @@ -281,6 +282,9 @@ func (c CredentialsCmd) Update(ctx context.Context, in CredentialsUpdateInput) e func (c CredentialsCmd) Delete(ctx context.Context, in CredentialsDeleteInput) error { if !in.SkipConfirm { + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("delete credential '%s'", in.Identifier)) + } msg := fmt.Sprintf("Are you sure you want to delete credential '%s'?", in.Identifier) pterm.DefaultInteractiveConfirm.DefaultText = msg ok, _ := pterm.DefaultInteractiveConfirm.Show() diff --git a/cmd/deploy.go b/cmd/deploy.go index f399d6c3..d7430646 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -15,6 +15,7 @@ import ( "time" "github.com/joho/godotenv" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/util" kernel "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" @@ -388,6 +389,9 @@ func runDeployDelete(cmd *cobra.Command, args []string) error { skipConfirm, _ := cmd.Flags().GetBool("yes") if !skipConfirm { + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("delete deployment '%s'", deploymentID)) + } msg := fmt.Sprintf("Are you sure you want to delete deployment '%s'? This cannot be undone.", deploymentID) pterm.DefaultInteractiveConfirm.DefaultText = msg ok, _ := pterm.DefaultInteractiveConfirm.Show() diff --git a/cmd/extensions.go b/cmd/extensions.go index da99041f..3fef666e 100644 --- a/cmd/extensions.go +++ b/cmd/extensions.go @@ -11,6 +11,7 @@ import ( "time" "github.com/kernel/cli/pkg/extensions" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/util" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" @@ -184,6 +185,9 @@ func (e ExtensionsCmd) Delete(ctx context.Context, in ExtensionsDeleteInput) err } if !in.SkipConfirm { + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("delete extension '%s'", in.Identifier)) + } msg := fmt.Sprintf("Are you sure you want to delete extension '%s'?", in.Identifier) pterm.DefaultInteractiveConfirm.DefaultText = msg ok, _ := pterm.DefaultInteractiveConfirm.Show() diff --git a/cmd/noninteractive_test.go b/cmd/noninteractive_test.go new file mode 100644 index 00000000..d5d07eb7 --- /dev/null +++ b/cmd/noninteractive_test.go @@ -0,0 +1,47 @@ +package cmd + +import ( + "context" + "testing" + + "github.com/kernel/kernel-go-sdk/option" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Under `go test` stdin is not a terminal, so any command path that would +// show an interactive confirmation must fail fast with a --yes hint instead +// of prompting (which would otherwise hang forever in agent/CI shells). + +func TestAPIKeysDeleteFailsFastWhenNonInteractive(t *testing.T) { + _ = capturePtermOutput(t) + fake := &FakeAPIKeysService{ + DeleteFunc: func(ctx context.Context, id string, opts ...option.RequestOption) error { + t.Fatal("delete must not be called without confirmation") + return nil + }, + } + c := APIKeysCmd{apiKeys: fake} + + err := c.Delete(context.Background(), APIKeysDeleteInput{ID: "key_123"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "delete API key 'key_123'") + assert.Contains(t, err.Error(), "--yes") + assert.Contains(t, err.Error(), "not an interactive terminal") +} + +func TestExtensionsDeleteFailsFastWhenNonInteractive(t *testing.T) { + _ = capturePtermOutput(t) + fake := &FakeExtensionsService{ + DeleteFunc: func(ctx context.Context, idOrName string, opts ...option.RequestOption) error { + t.Fatal("delete must not be called without confirmation") + return nil + }, + } + e := ExtensionsCmd{extensions: fake} + + err := e.Delete(context.Background(), ExtensionsDeleteInput{Identifier: "e1"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "delete extension 'e1'") + assert.Contains(t, err.Error(), "--yes") +} diff --git a/cmd/profiles.go b/cmd/profiles.go index bfc055c7..bb94c259 100644 --- a/cmd/profiles.go +++ b/cmd/profiles.go @@ -11,6 +11,7 @@ import ( "path/filepath" "strings" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/util" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" @@ -227,6 +228,9 @@ func (p ProfilesCmd) Delete(ctx context.Context, in ProfilesDeleteInput) error { } if !in.SkipConfirm { + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("delete profile '%s'", in.Identifier)) + } msg := fmt.Sprintf("Are you sure you want to delete profile '%s'?", in.Identifier) pterm.DefaultInteractiveConfirm.DefaultText = msg ok, _ := pterm.DefaultInteractiveConfirm.Show() diff --git a/cmd/proxies/delete.go b/cmd/proxies/delete.go index 73abeec7..2bd3af21 100644 --- a/cmd/proxies/delete.go +++ b/cmd/proxies/delete.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/util" "github.com/pterm/pterm" "github.com/spf13/cobra" @@ -11,6 +12,9 @@ import ( func (p ProxyCmd) Delete(ctx context.Context, in ProxyDeleteInput) error { if !in.SkipConfirm { + if !interactive.IsInteractive() { + return interactive.ErrConfirmationRequired(fmt.Sprintf("delete proxy '%s'", in.ID)) + } // Try to get the proxy details for better confirmation message proxy, err := p.proxies.Get(ctx, in.ID) if err != nil { diff --git a/pkg/create/prompts.go b/pkg/create/prompts.go index dc0abe53..5303c691 100644 --- a/pkg/create/prompts.go +++ b/pkg/create/prompts.go @@ -4,7 +4,9 @@ import ( "fmt" "regexp" "slices" + "strings" + "github.com/kernel/cli/pkg/interactive" "github.com/pterm/pterm" ) @@ -31,8 +33,35 @@ func validateAppName(val any) error { return nil } +// languageOptionsHint renders the supported languages (with shorthands) for +// use in flag-usage hints, e.g. "typescript (ts), python (py)". +func languageOptionsHint() string { + opts := make([]string, 0, len(SupportedLanguages)) + for _, l := range SupportedLanguages { + if s := LanguageShorthand(l); s != "" { + opts = append(opts, fmt.Sprintf("%s (%s)", l, s)) + } else { + opts = append(opts, l) + } + } + return strings.Join(opts, ", ") +} + +// templateOptionsHint renders the template keys for use in flag-usage hints. +func templateOptionsHint(templateKVs TemplateKeyValues) string { + keys := make([]string, 0, len(templateKVs)) + for _, kv := range templateKVs { + keys = append(keys, kv.Key) + } + return strings.Join(keys, ", ") +} + // handleAppNamePrompt prompts the user for an app name interactively. func handleAppNamePrompt() (string, error) { + if !interactive.IsInteractive() { + return "", interactive.ErrInputRequired("app name", "pass --name to set the app name (e.g. --name "+DefaultAppName+")") + } + promptText := fmt.Sprintf("%s (%s)", AppNamePrompt, DefaultAppName) appName, err := pterm.DefaultInteractiveTextInput. WithDefaultText(promptText). @@ -56,6 +85,7 @@ func handleAppNamePrompt() (string, error) { // PromptForAppName validates the provided app name or prompts the user for one. // If the provided name is invalid, it shows a warning and prompts the user. +// In a non-interactive shell it fails fast instead of prompting. func PromptForAppName(providedAppName string) (string, error) { // If no app name was provided, prompt the user if providedAppName == "" { @@ -63,6 +93,9 @@ func PromptForAppName(providedAppName string) (string, error) { } if err := validateAppName(providedAppName); err != nil { + if !interactive.IsInteractive() { + return "", fmt.Errorf("invalid --name '%s': %w", providedAppName, err) + } pterm.Warning.Printf("Invalid app name '%s': %v\n", providedAppName, err) pterm.Info.Println("Please provide a valid app name.") return handleAppNamePrompt() @@ -72,6 +105,10 @@ func PromptForAppName(providedAppName string) (string, error) { } func handleLanguagePrompt() (string, error) { + if !interactive.IsInteractive() { + return "", interactive.ErrInputRequired("language selection", "pass --language with one of: "+languageOptionsHint()) + } + l, err := pterm.DefaultInteractiveSelect. WithOptions(SupportedLanguages). WithDefaultText(LanguagePrompt). @@ -82,6 +119,8 @@ func handleLanguagePrompt() (string, error) { return l, nil } +// PromptForLanguage validates the provided language or prompts the user for +// one. In a non-interactive shell it fails fast instead of prompting. func PromptForLanguage(providedLanguage string) (string, error) { if providedLanguage == "" { return handleLanguagePrompt() @@ -92,11 +131,18 @@ func PromptForLanguage(providedLanguage string) (string, error) { return l, nil } + if !interactive.IsInteractive() { + return "", fmt.Errorf("invalid --language '%s': must be one of: %s", providedLanguage, languageOptionsHint()) + } pterm.Warning.Printfln("Language '%s' not found. Please select from available languages.\n", providedLanguage) return handleLanguagePrompt() } func handleTemplatePrompt(templateKVs TemplateKeyValues) (string, error) { + if !interactive.IsInteractive() { + return "", interactive.ErrInputRequired("template selection", "pass --template with one of: "+templateOptionsHint(templateKVs)) + } + template, err := pterm.DefaultInteractiveSelect. WithOptions(templateKVs.GetTemplateDisplayValues()). WithDefaultText(TemplatePrompt). @@ -109,6 +155,8 @@ func handleTemplatePrompt(templateKVs TemplateKeyValues) (string, error) { return templateKVs.GetTemplateKeyFromValue(template) } +// PromptForTemplate validates the provided template or prompts the user for +// one. In a non-interactive shell it fails fast instead of prompting. func PromptForTemplate(providedTemplate string, providedLanguage string) (string, error) { templateKVs := GetSupportedTemplatesForLanguage(NormalizeLanguage(providedLanguage)) @@ -120,12 +168,21 @@ func PromptForTemplate(providedTemplate string, providedLanguage string) (string return providedTemplate, nil } + if !interactive.IsInteractive() { + return "", fmt.Errorf("invalid --template '%s' for language '%s': must be one of: %s", providedTemplate, NormalizeLanguage(providedLanguage), templateOptionsHint(templateKVs)) + } pterm.Warning.Printfln("Template '%s' not found. Please select from available templates.\n", providedTemplate) return handleTemplatePrompt(templateKVs) } -// PromptForOverwrite prompts the user to confirm overwriting an existing directory. +// PromptForOverwrite prompts the user to confirm overwriting an existing +// directory. In a non-interactive shell it fails fast instead of prompting; +// pass --yes to overwrite without confirmation. func PromptForOverwrite(dirName string) (bool, error) { + if !interactive.IsInteractive() { + return false, interactive.ErrConfirmationRequired(fmt.Sprintf("overwrite existing directory '%s'", dirName)) + } + overwrite, err := pterm.DefaultInteractiveConfirm. WithDefaultText(fmt.Sprintf("\nDirectory %s already exists. Overwrite?", dirName)). WithDefaultValue(false). diff --git a/pkg/create/prompts_test.go b/pkg/create/prompts_test.go new file mode 100644 index 00000000..c1b2a415 --- /dev/null +++ b/pkg/create/prompts_test.go @@ -0,0 +1,85 @@ +package create + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// These tests run under `go test`, where stdin is not a terminal, so every +// prompt path must fail fast with a flag-usage hint instead of prompting. + +func TestPromptForAppNameNonInteractive(t *testing.T) { + t.Run("missing name fails fast with --name hint", func(t *testing.T) { + _, err := PromptForAppName("") + require.Error(t, err) + assert.Contains(t, err.Error(), "--name") + assert.Contains(t, err.Error(), "not an interactive terminal") + }) + + t.Run("invalid name fails fast with validation error", func(t *testing.T) { + _, err := PromptForAppName("bad name!") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid --name 'bad name!'") + }) + + t.Run("valid name passes through", func(t *testing.T) { + name, err := PromptForAppName("my-app") + require.NoError(t, err) + assert.Equal(t, "my-app", name) + }) +} + +func TestPromptForLanguageNonInteractive(t *testing.T) { + t.Run("missing language fails fast with --language hint", func(t *testing.T) { + _, err := PromptForLanguage("") + require.Error(t, err) + assert.Contains(t, err.Error(), "--language") + assert.Contains(t, err.Error(), LanguageTypeScript) + assert.Contains(t, err.Error(), LanguagePython) + }) + + t.Run("invalid language fails fast listing options", func(t *testing.T) { + _, err := PromptForLanguage("ruby") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid --language 'ruby'") + assert.Contains(t, err.Error(), LanguageTypeScript) + }) + + t.Run("valid language passes through", func(t *testing.T) { + l, err := PromptForLanguage("ts") + require.NoError(t, err) + assert.Equal(t, LanguageTypeScript, l) + }) +} + +func TestPromptForTemplateNonInteractive(t *testing.T) { + t.Run("missing template fails fast with --template hint", func(t *testing.T) { + _, err := PromptForTemplate("", LanguageTypeScript) + require.Error(t, err) + assert.Contains(t, err.Error(), "--template") + assert.Contains(t, err.Error(), TemplateSampleApp) + }) + + t.Run("invalid template fails fast listing options", func(t *testing.T) { + _, err := PromptForTemplate("nope", LanguageTypeScript) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid --template 'nope'") + assert.Contains(t, err.Error(), TemplateSampleApp) + }) + + t.Run("valid template passes through", func(t *testing.T) { + tmpl, err := PromptForTemplate(TemplateSampleApp, LanguageTypeScript) + require.NoError(t, err) + assert.Equal(t, TemplateSampleApp, tmpl) + }) +} + +func TestPromptForOverwriteNonInteractive(t *testing.T) { + overwrite, err := PromptForOverwrite("existing-dir") + require.Error(t, err) + assert.False(t, overwrite) + assert.Contains(t, err.Error(), "overwrite existing directory 'existing-dir'") + assert.Contains(t, err.Error(), "--yes") +} diff --git a/pkg/create/types.go b/pkg/create/types.go index 35c32711..3f9b359d 100644 --- a/pkg/create/types.go +++ b/pkg/create/types.go @@ -6,6 +6,9 @@ type CreateInput struct { Name string Language string Template string + // SkipConfirm skips confirmation prompts (e.g. overwriting an existing + // directory). Set via the --yes flag. + SkipConfirm bool } const ( diff --git a/pkg/interactive/interactive.go b/pkg/interactive/interactive.go new file mode 100644 index 00000000..530cd0ac --- /dev/null +++ b/pkg/interactive/interactive.go @@ -0,0 +1,38 @@ +// Package interactive reports whether the CLI can show interactive prompts +// and builds the fail-fast errors used when it cannot. +// +// Interactive prompts (pterm confirm/select/text input) read keystrokes from +// the terminal. In a non-interactive shell — an AI agent's bash tool, CI, or +// any piped stdin — they never return (the underlying keyboard listener spins +// forever), so every prompt call site must gate on IsInteractive first and +// fail fast with instructions for avoiding the prompt. +package interactive + +import ( + "fmt" + "os" + + "golang.org/x/term" +) + +// IsInteractive reports whether stdin is attached to a terminal, i.e. whether +// interactive prompts can be shown. +func IsInteractive() bool { + return term.IsTerminal(int(os.Stdin.Fd())) +} + +// ErrConfirmationRequired builds the fail-fast error for confirmation +// prompts. action describes what would have been confirmed, e.g. +// "delete profile 'foo'". The resulting error tells the caller (often an AI +// agent) to re-run with --yes. +func ErrConfirmationRequired(action string) error { + return fmt.Errorf("cannot prompt for confirmation to %s: stdin is not an interactive terminal; re-run with --yes to skip the confirmation prompt", action) +} + +// ErrInputRequired builds the fail-fast error for text/select prompts. what +// describes the input that would have been prompted for, e.g. "app name"; +// hint names the flag(s) to pass instead, e.g. "pass --name to set the app +// name". +func ErrInputRequired(what, hint string) error { + return fmt.Errorf("cannot prompt for %s: stdin is not an interactive terminal; %s", what, hint) +} diff --git a/pkg/interactive/interactive_test.go b/pkg/interactive/interactive_test.go new file mode 100644 index 00000000..dab35859 --- /dev/null +++ b/pkg/interactive/interactive_test.go @@ -0,0 +1,28 @@ +package interactive + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// Under `go test` stdin is not attached to a terminal, so IsInteractive must +// report false. This is the same environment as CI pipelines and AI-agent +// shells — exactly the case the fail-fast gating exists for. +func TestIsInteractiveFalseWithoutTerminal(t *testing.T) { + assert.False(t, IsInteractive()) +} + +func TestErrConfirmationRequired(t *testing.T) { + err := ErrConfirmationRequired("delete profile 'foo'") + assert.Contains(t, err.Error(), "delete profile 'foo'") + assert.Contains(t, err.Error(), "--yes") + assert.Contains(t, err.Error(), "not an interactive terminal") +} + +func TestErrInputRequired(t *testing.T) { + err := ErrInputRequired("app name", "pass --name to set the app name") + assert.Contains(t, err.Error(), "app name") + assert.Contains(t, err.Error(), "pass --name") + assert.Contains(t, err.Error(), "not an interactive terminal") +} From 5a822aac51d8876d0a22679e7b2cbc52e711bc86 Mon Sep 17 00:00:00 2001 From: Rafael Garcia Date: Fri, 24 Jul 2026 14:08:16 -0400 Subject: [PATCH 2/5] Return create prompt errors unwrapped Addresses Bugbot review on #209: runCreateApp wrapped the fail-fast prompt errors with 'failed to get app name/language/template' prefixes, so CLI output didn't match the documented non-interactive messages and was inconsistent with the unwrapped overwrite-confirm error. Return them directly and add a regression test. --- cmd/create.go | 8 +++++--- cmd/noninteractive_test.go | 39 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/cmd/create.go b/cmd/create.go index 9e948c54..3bb1d643 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -150,19 +150,21 @@ func runCreateApp(cmd *cobra.Command, args []string) error { template, _ := cmd.Flags().GetString("template") skipConfirm, _ := cmd.Flags().GetBool("yes") + // Return prompt errors unwrapped: in non-interactive shells they are + // crafted fail-fast messages that name the exact flags to pass. appName, err := create.PromptForAppName(appName) if err != nil { - return fmt.Errorf("failed to get app name: %w", err) + return err } language, err = create.PromptForLanguage(language) if err != nil { - return fmt.Errorf("failed to get language: %w", err) + return err } template, err = create.PromptForTemplate(template, language) if err != nil { - return fmt.Errorf("failed to get template: %w", err) + return err } c := CreateCmd{} diff --git a/cmd/noninteractive_test.go b/cmd/noninteractive_test.go index d5d07eb7..ede41688 100644 --- a/cmd/noninteractive_test.go +++ b/cmd/noninteractive_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/kernel/kernel-go-sdk/option" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -30,6 +31,44 @@ func TestAPIKeysDeleteFailsFastWhenNonInteractive(t *testing.T) { assert.Contains(t, err.Error(), "not an interactive terminal") } +// runCreateApp must return prompt errors unwrapped so the CLI output matches +// the documented fail-fast messages ("cannot prompt for ...", "invalid --...") +// without "failed to get ..." prefixes. +func TestCreateFailsFastUnwrappedWhenNonInteractive(t *testing.T) { + newCreateCmd := func(flags map[string]string) *cobra.Command { + cmd := &cobra.Command{} + cmd.Flags().String("name", "", "") + cmd.Flags().String("language", "", "") + cmd.Flags().String("template", "", "") + cmd.Flags().Bool("yes", false, "") + for flag, value := range flags { + require.NoError(t, cmd.Flags().Set(flag, value)) + } + return cmd + } + + t.Run("missing app name", func(t *testing.T) { + err := runCreateApp(newCreateCmd(nil), nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot prompt for app name") + assert.NotContains(t, err.Error(), "failed to get") + }) + + t.Run("invalid language", func(t *testing.T) { + err := runCreateApp(newCreateCmd(map[string]string{"name": "my-app", "language": "ruby"}), nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid --language 'ruby'") + assert.NotContains(t, err.Error(), "failed to get") + }) + + t.Run("invalid template", func(t *testing.T) { + err := runCreateApp(newCreateCmd(map[string]string{"name": "my-app", "language": "typescript", "template": "nope"}), nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid --template 'nope'") + assert.NotContains(t, err.Error(), "failed to get") + }) +} + func TestExtensionsDeleteFailsFastWhenNonInteractive(t *testing.T) { _ = capturePtermOutput(t) fake := &FakeExtensionsService{ From f52565883760cc239d893b1f0df6ca869630d5a9 Mon Sep 17 00:00:00 2001 From: Rafael Garcia Date: Fri, 24 Jul 2026 14:14:33 -0400 Subject: [PATCH 3/5] Aggregate all missing create inputs into one fail-fast error Previously kernel create failed on the first promptable input it encountered (--name, then --language, then --template), so a non-interactive caller needed one retry per missing flag. In a non-interactive shell, all inputs are now validated up front via create.ValidateNonInteractive and every problem is reported in a single error, including an existing target directory that would need --yes: Error: cannot prompt for input: stdin is not an interactive terminal; fix all of the following and re-run: (1) --name is required (e.g. --name my-kernel-app); (2) --language is required: one of: typescript (ts), python (py); (3) --template is required (run 'kernel create --help' for the full list) Problems are numbered inline rather than newline-separated because the fang/lipgloss error renderer reflows text and collapses line breaks. A template that exists for no language is reported even when --language is also invalid, to avoid a second retry. The per-prompt guards remain as backstops for the interactive flow. --- cmd/create.go | 17 ++++++-- cmd/noninteractive_test.go | 29 ++++++++------ pkg/create/validate.go | 72 ++++++++++++++++++++++++++++++++++ pkg/create/validate_test.go | 61 ++++++++++++++++++++++++++++ pkg/interactive/interactive.go | 28 +++++++++++++ 5 files changed, 191 insertions(+), 16 deletions(-) create mode 100644 pkg/create/validate.go create mode 100644 pkg/create/validate_test.go diff --git a/cmd/create.go b/cmd/create.go index 3bb1d643..c3196f25 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/kernel/cli/pkg/create" + "github.com/kernel/cli/pkg/interactive" "github.com/pterm/pterm" "github.com/spf13/cobra" ) @@ -150,8 +151,19 @@ func runCreateApp(cmd *cobra.Command, args []string) error { template, _ := cmd.Flags().GetString("template") skipConfirm, _ := cmd.Flags().GetBool("yes") - // Return prompt errors unwrapped: in non-interactive shells they are - // crafted fail-fast messages that name the exact flags to pass. + c := CreateCmd{} + + // In a non-interactive shell, validate every input up front so a single + // error reports everything the caller must fix, instead of failing on one + // missing input per invocation. + if !interactive.IsInteractive() { + in, err := create.ValidateNonInteractive(appName, language, template, skipConfirm) + if err != nil { + return err + } + return c.Create(cmd.Context(), in) + } + appName, err := create.PromptForAppName(appName) if err != nil { return err @@ -167,7 +179,6 @@ func runCreateApp(cmd *cobra.Command, args []string) error { return err } - c := CreateCmd{} return c.Create(cmd.Context(), create.CreateInput{ Name: appName, Language: language, diff --git a/cmd/noninteractive_test.go b/cmd/noninteractive_test.go index ede41688..9cf7e7f5 100644 --- a/cmd/noninteractive_test.go +++ b/cmd/noninteractive_test.go @@ -31,10 +31,10 @@ func TestAPIKeysDeleteFailsFastWhenNonInteractive(t *testing.T) { assert.Contains(t, err.Error(), "not an interactive terminal") } -// runCreateApp must return prompt errors unwrapped so the CLI output matches -// the documented fail-fast messages ("cannot prompt for ...", "invalid --...") -// without "failed to get ..." prefixes. -func TestCreateFailsFastUnwrappedWhenNonInteractive(t *testing.T) { +// In a non-interactive shell, `kernel create` must report every missing or +// invalid input in a single unwrapped error (no "failed to get ..." +// prefixes), so one retry can fix everything. +func TestCreateFailsFastAggregatedWhenNonInteractive(t *testing.T) { newCreateCmd := func(flags map[string]string) *cobra.Command { cmd := &cobra.Command{} cmd.Flags().String("name", "", "") @@ -47,25 +47,28 @@ func TestCreateFailsFastUnwrappedWhenNonInteractive(t *testing.T) { return cmd } - t.Run("missing app name", func(t *testing.T) { + t.Run("no flags reports all three inputs in one error", func(t *testing.T) { err := runCreateApp(newCreateCmd(nil), nil) require.Error(t, err) - assert.Contains(t, err.Error(), "cannot prompt for app name") + assert.Contains(t, err.Error(), "--name is required") + assert.Contains(t, err.Error(), "--language is required") + assert.Contains(t, err.Error(), "--template is required") assert.NotContains(t, err.Error(), "failed to get") }) - t.Run("invalid language", func(t *testing.T) { - err := runCreateApp(newCreateCmd(map[string]string{"name": "my-app", "language": "ruby"}), nil) + t.Run("mixed missing and invalid flags aggregated", func(t *testing.T) { + err := runCreateApp(newCreateCmd(map[string]string{"language": "ruby", "template": "nope"}), nil) require.Error(t, err) - assert.Contains(t, err.Error(), "invalid --language 'ruby'") - assert.NotContains(t, err.Error(), "failed to get") + assert.Contains(t, err.Error(), "--name is required") + assert.Contains(t, err.Error(), "--language 'ruby' is invalid") + assert.Contains(t, err.Error(), "--template 'nope' is invalid") }) - t.Run("invalid template", func(t *testing.T) { + t.Run("single problem stays a single-line error", func(t *testing.T) { err := runCreateApp(newCreateCmd(map[string]string{"name": "my-app", "language": "typescript", "template": "nope"}), nil) require.Error(t, err) - assert.Contains(t, err.Error(), "invalid --template 'nope'") - assert.NotContains(t, err.Error(), "failed to get") + assert.Contains(t, err.Error(), "--template 'nope' is invalid for language 'typescript'") + assert.NotContains(t, err.Error(), "\n") }) } diff --git a/pkg/create/validate.go b/pkg/create/validate.go new file mode 100644 index 00000000..f7165cf8 --- /dev/null +++ b/pkg/create/validate.go @@ -0,0 +1,72 @@ +package create + +import ( + "fmt" + "os" + "slices" + + "github.com/kernel/cli/pkg/interactive" +) + +// ValidateNonInteractive validates every scaffolding input without prompting +// and aggregates all missing or invalid flags into a single error, so a +// non-interactive caller (usually an AI agent) can fix everything in one +// retry. It also flags an existing target directory when skipOverwriteConfirm +// is false, since that would otherwise require an interactive confirmation. +func ValidateNonInteractive(name, language, template string, skipOverwriteConfirm bool) (CreateInput, error) { + var problems []string + + // --name + nameValid := false + if name == "" { + problems = append(problems, "--name is required (e.g. --name "+DefaultAppName+")") + } else if err := validateAppName(name); err != nil { + problems = append(problems, fmt.Sprintf("--name '%s' is invalid: %v", name, err)) + } else { + nameValid = true + } + + // --language + lang := "" + if language == "" { + problems = append(problems, "--language is required: one of: "+languageOptionsHint()) + } else if l := NormalizeLanguage(language); slices.Contains(SupportedLanguages, l) { + lang = l + } else { + problems = append(problems, fmt.Sprintf("--language '%s' is invalid: must be one of: %s", language, languageOptionsHint())) + } + + // --template (valid values depend on --language) + if lang != "" { + templateKVs := GetSupportedTemplatesForLanguage(lang) + if template == "" { + problems = append(problems, "--template is required: one of: "+templateOptionsHint(templateKVs)) + } else if !templateKVs.ContainsKey(template) { + problems = append(problems, fmt.Sprintf("--template '%s' is invalid for language '%s': must be one of: %s", template, lang, templateOptionsHint(templateKVs))) + } + } else if template == "" { + problems = append(problems, "--template is required (run 'kernel create --help' for the full list)") + } else if _, ok := Templates[template]; !ok { + // Language is unknown, but the template doesn't exist for any + // language, so report it now rather than on the next retry. + problems = append(problems, fmt.Sprintf("--template '%s' is invalid (run 'kernel create --help' for the full list)", template)) + } + + // Overwriting the target directory would need a confirmation prompt. + if nameValid && !skipOverwriteConfirm { + if _, err := os.Stat(name); err == nil { + problems = append(problems, fmt.Sprintf("directory '%s' already exists: pass --yes to overwrite it, or choose a different --name", name)) + } + } + + if len(problems) > 0 { + return CreateInput{}, interactive.ErrInputsRequired(problems) + } + + return CreateInput{ + Name: name, + Language: lang, + Template: template, + SkipConfirm: skipOverwriteConfirm, + }, nil +} diff --git a/pkg/create/validate_test.go b/pkg/create/validate_test.go new file mode 100644 index 00000000..c531bf21 --- /dev/null +++ b/pkg/create/validate_test.go @@ -0,0 +1,61 @@ +package create + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateNonInteractiveAggregatesAllProblems(t *testing.T) { + _, err := ValidateNonInteractive("", "", "", false) + require.Error(t, err) + assert.Contains(t, err.Error(), "--name is required") + assert.Contains(t, err.Error(), "--language is required") + assert.Contains(t, err.Error(), "--template is required") + assert.Contains(t, err.Error(), "not an interactive terminal") +} + +func TestValidateNonInteractiveInvalidValues(t *testing.T) { + _, err := ValidateNonInteractive("bad name!", "ruby", "nope", false) + require.Error(t, err) + assert.Contains(t, err.Error(), "--name 'bad name!' is invalid") + assert.Contains(t, err.Error(), "--language 'ruby' is invalid") + // Language is unknown, but the template exists for no language at all, + // so it must still be reported in the same error. + assert.Contains(t, err.Error(), "--template 'nope' is invalid") +} + +func TestValidateNonInteractiveTemplateValidatedPerLanguage(t *testing.T) { + _, err := ValidateNonInteractive("my-app", "ts", "nope", false) + require.Error(t, err) + assert.Contains(t, err.Error(), "--template 'nope' is invalid for language 'typescript'") + assert.Contains(t, err.Error(), TemplateSampleApp) +} + +func TestValidateNonInteractiveExistingDirectory(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "my-app"), 0o755)) + + _, err := ValidateNonInteractive("my-app", "ts", TemplateSampleApp, false) + require.Error(t, err) + assert.Contains(t, err.Error(), "directory 'my-app' already exists") + assert.Contains(t, err.Error(), "--yes") + + // --yes skips the overwrite confirmation, so it is not a problem. + in, err := ValidateNonInteractive("my-app", "ts", TemplateSampleApp, true) + require.NoError(t, err) + assert.True(t, in.SkipConfirm) +} + +func TestValidateNonInteractiveValidInput(t *testing.T) { + in, err := ValidateNonInteractive("my-app", "py", TemplateSampleApp, false) + require.NoError(t, err) + assert.Equal(t, "my-app", in.Name) + assert.Equal(t, LanguagePython, in.Language) + assert.Equal(t, TemplateSampleApp, in.Template) + assert.False(t, in.SkipConfirm) +} diff --git a/pkg/interactive/interactive.go b/pkg/interactive/interactive.go index 530cd0ac..9d0c0bb2 100644 --- a/pkg/interactive/interactive.go +++ b/pkg/interactive/interactive.go @@ -11,6 +11,7 @@ package interactive import ( "fmt" "os" + "strings" "golang.org/x/term" ) @@ -36,3 +37,30 @@ func ErrConfirmationRequired(action string) error { func ErrInputRequired(what, hint string) error { return fmt.Errorf("cannot prompt for %s: stdin is not an interactive terminal; %s", what, hint) } + +// ErrInputsRequired builds the fail-fast error for one or more missing or +// invalid inputs. Commands with several promptable inputs should validate +// them all up front and report every problem in a single error, so a +// non-interactive caller can fix everything in one retry instead of +// discovering problems one invocation at a time. +// +// Problems are numbered inline — "(1) ...; (2) ..." — rather than +// newline-separated because the CLI error renderer reflows text and would +// collapse line breaks anyway. +func ErrInputsRequired(problems []string) error { + switch len(problems) { + case 0: + return nil + case 1: + return fmt.Errorf("cannot prompt for input: stdin is not an interactive terminal; %s", problems[0]) + default: + var b strings.Builder + for i, p := range problems { + if i > 0 { + b.WriteString("; ") + } + fmt.Fprintf(&b, "(%d) %s", i+1, p) + } + return fmt.Errorf("cannot prompt for input: stdin is not an interactive terminal; fix all of the following and re-run: %s", b.String()) + } +} From ba2884251beb9b54f702a61e8f3a3dcaeb055a3a Mon Sep 17 00:00:00 2001 From: Rafael Garcia Date: Fri, 24 Jul 2026 15:22:35 -0400 Subject: [PATCH 4/5] Restructure fail-fast interactivity per review Addresses masnwilliams' blocking review on #209: 1. pkg/interactive now owns prompt execution. Confirm/Select/TextInput run the pterm prompts behind the TTY gate; command code supplies only the action, hint, and options. No direct pterm interactive calls remain outside the package, so the no-hang invariant is enforced by the boundary rather than a per-call-site convention. Prompt errors are no longer silently dropped, and the confirm primitives stop mutating pterm's global DefaultInteractiveConfirm. 2. Typed PromptError with explicit rendering. Error() stays a single line (Go convention, numbered problems); Display() renders one problem per line. The root error handler renders PromptError with fang's ErrorText width and transform unset: the width-based word-wrap split flag tokens ('--' + newline + 'template') and the transform (strings.Fields + Join) collapsed newlines. Rendered stderr now contains every flag token intact. 3. One canonical resolver for kernel create. create.ResolveInput is the single path from raw flags to a normalized CreateInput plus typed Problems (name, language, template, overwrite). The interactive flow prompts to resolve one problem at a time and re-resolves; the non-interactive flow reports the same problem model in a single fail-fast error. The forked ValidateNonInteractive/PromptFor* pipelines are gone; prompts are thin per-field wrappers with no validation logic. 4. Test isolation. Gating tests inject the terminal capability via interactive.ForceTerminal instead of relying on ambient stdin, the detector is tested against an explicit pipe fd, and a subprocess e2e test (TestMain re-exec) runs the real CLI with stdin=/dev/null and asserts on final rendered stderr: exit 1, intact flag tokens, one problem per line. Interactive flows verified end-to-end under a real PTY (name -> language select -> template select -> scaffold; invalid-value warning before re-prompt). --- cmd/api_keys.go | 11 +- cmd/app.go | 11 +- cmd/auth_connections.go | 11 +- cmd/browsers.go | 28 ++-- cmd/create.go | 87 +++++++---- cmd/credential_providers.go | 11 +- cmd/credentials.go | 11 +- cmd/deploy.go | 11 +- cmd/extensions.go | 11 +- cmd/noninteractive_test.go | 115 ++++++++++++--- cmd/profiles.go | 11 +- cmd/proxies/delete.go | 8 +- cmd/root.go | 28 +++- pkg/create/prompts.go | 215 ++++++---------------------- pkg/create/prompts_test.go | 88 +++--------- pkg/create/resolve.go | 157 ++++++++++++++++++++ pkg/create/resolve_test.go | 79 ++++++++++ pkg/create/validate.go | 72 ---------- pkg/create/validate_test.go | 61 -------- pkg/interactive/interactive.go | 157 ++++++++++++++++---- pkg/interactive/interactive_test.go | 84 ++++++++++- 21 files changed, 751 insertions(+), 516 deletions(-) create mode 100644 pkg/create/resolve.go create mode 100644 pkg/create/resolve_test.go delete mode 100644 pkg/create/validate.go delete mode 100644 pkg/create/validate_test.go diff --git a/cmd/api_keys.go b/cmd/api_keys.go index 535622dc..c6b78baf 100644 --- a/cmd/api_keys.go +++ b/cmd/api_keys.go @@ -182,12 +182,13 @@ func (c APIKeysCmd) Update(ctx context.Context, in APIKeysUpdateInput) error { func (c APIKeysCmd) Delete(ctx context.Context, in APIKeysDeleteInput) error { if !in.SkipConfirm { - if !interactive.IsInteractive() { - return interactive.ErrConfirmationRequired(fmt.Sprintf("delete API key '%s'", in.ID)) + ok, err := interactive.Confirm( + fmt.Sprintf("delete API key '%s'", in.ID), + fmt.Sprintf("Are you sure you want to delete API key '%s'?", in.ID), + ) + if err != nil { + return err } - msg := fmt.Sprintf("Are you sure you want to delete API key '%s'?", in.ID) - pterm.DefaultInteractiveConfirm.DefaultText = msg - ok, _ := pterm.DefaultInteractiveConfirm.Show() if !ok { pterm.Info.Println("Deletion cancelled") return nil diff --git a/cmd/app.go b/cmd/app.go index b665c29f..51993e41 100644 --- a/cmd/app.go +++ b/cmd/app.go @@ -257,12 +257,13 @@ func runAppDelete(cmd *cobra.Command, args []string) error { if version != "" { scope = fmt.Sprintf("version '%s'", version) } - if !interactive.IsInteractive() { - return interactive.ErrConfirmationRequired(fmt.Sprintf("delete all deployments for app '%s' (%s)", appName, scope)) + ok, err := interactive.Confirm( + fmt.Sprintf("delete all deployments for app '%s' (%s)", appName, scope), + fmt.Sprintf("Delete all deployments for app '%s' (%s)? This cannot be undone.", appName, scope), + ) + if err != nil { + return err } - msg := fmt.Sprintf("Delete all deployments for app '%s' (%s)? This cannot be undone.", appName, scope) - pterm.DefaultInteractiveConfirm.DefaultText = msg - ok, _ := pterm.DefaultInteractiveConfirm.Show() if !ok { pterm.Info.Println("Deletion cancelled") return nil diff --git a/cmd/auth_connections.go b/cmd/auth_connections.go index 58aac050..c0780a75 100644 --- a/cmd/auth_connections.go +++ b/cmd/auth_connections.go @@ -492,12 +492,13 @@ func (c AuthConnectionCmd) List(ctx context.Context, in AuthConnectionListInput) func (c AuthConnectionCmd) Delete(ctx context.Context, in AuthConnectionDeleteInput) error { if !in.SkipConfirm { - if !interactive.IsInteractive() { - return interactive.ErrConfirmationRequired(fmt.Sprintf("delete managed auth '%s'", in.ID)) + ok, err := interactive.Confirm( + fmt.Sprintf("delete managed auth '%s'", in.ID), + fmt.Sprintf("Are you sure you want to delete managed auth '%s'?", in.ID), + ) + if err != nil { + return err } - msg := fmt.Sprintf("Are you sure you want to delete managed auth '%s'?", in.ID) - pterm.DefaultInteractiveConfirm.DefaultText = msg - ok, _ := pterm.DefaultInteractiveConfirm.Show() if !ok { pterm.Info.Println("Deletion cancelled") return nil diff --git a/cmd/browsers.go b/cmd/browsers.go index ab825022..cf910dd6 100644 --- a/cmd/browsers.go +++ b/cmd/browsers.go @@ -2868,10 +2868,13 @@ func runBrowsersCreate(cmd *cobra.Command, args []string) error { pterm.Info.Println("The conflicting flags will be ignored.") if !skipConfirm { - if !interactive.IsInteractive() { - return interactive.ErrConfirmationRequired(fmt.Sprintf("ignore conflicting browser configuration flags (%s) and continue with %s", strings.Join(conflicts, ", "), flagLabel)) + result, err := interactive.Confirm( + fmt.Sprintf("ignore conflicting browser configuration flags (%s) and continue with %s", strings.Join(conflicts, ", "), flagLabel), + "Continue with pool configuration?", + ) + if err != nil { + return err } - result, _ := pterm.DefaultInteractiveConfirm.Show("Continue with pool configuration?") if !result { pterm.Info.Println("Cancelled. Remove conflicting flags or omit the pool flag.") return nil @@ -2920,20 +2923,17 @@ func runBrowsersCreate(cmd *cobra.Command, args []string) error { // Handle interactive viewport selection if viewportInteractive { - if !interactive.IsInteractive() { - return interactive.ErrInputRequired("viewport selection", "pass --viewport instead of --viewport-interactive (e.g. --viewport 1920x1080@25)") - } - if viewport != "" { + if viewport != "" && interactive.IsInteractive() { pterm.Warning.Println("Both --viewport and --viewport-interactive specified; using interactive mode") } - options := getAvailableViewports() - selectedViewport, err := pterm.DefaultInteractiveSelect. - WithOptions(options). - WithDefaultText("Select a viewport size:"). - Show() + selectedViewport, err := interactive.Select( + "viewport selection", + "pass --viewport instead of --viewport-interactive (e.g. --viewport 1920x1080@25)", + "Select a viewport size:", + getAvailableViewports(), + ) if err != nil { - pterm.Error.Printf("Failed to select viewport: %v\n", err) - return nil + return err } viewport = selectedViewport } diff --git a/cmd/create.go b/cmd/create.go index c3196f25..f82610da 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -24,10 +24,12 @@ func (c CreateCmd) Create(ctx context.Context, ci create.CreateInput) error { return fmt.Errorf("failed to resolve app path: %w", err) } - // Check if directory already exists and prompt for overwrite + // Check if directory already exists and prompt for overwrite. This is a + // backstop for direct callers; runCreateApp resolves the overwrite + // confirmation before calling Create. if _, err := os.Stat(appPath); err == nil { if !ci.SkipConfirm { - overwrite, err := create.PromptForOverwrite(ci.Name) + overwrite, err := create.PromptOverwrite(ci.Name) if err != nil { return err } @@ -153,36 +155,59 @@ func runCreateApp(cmd *cobra.Command, args []string) error { c := CreateCmd{} - // In a non-interactive shell, validate every input up front so a single - // error reports everything the caller must fix, instead of failing on one - // missing input per invocation. - if !interactive.IsInteractive() { - in, err := create.ValidateNonInteractive(appName, language, template, skipConfirm) - if err != nil { - return err + // create.ResolveInput is the single resolver from raw flags to a + // normalized CreateInput for both modes. Interactively, each remaining + // problem is resolved by prompting for that field and re-resolving (so + // e.g. the template list reflects the chosen language). Non-interactively, + // every problem is reported at once in a single fail-fast error. + for { + in, problems := create.ResolveInput(appName, language, template, skipConfirm) + if len(problems) == 0 { + return c.Create(cmd.Context(), in) + } + if !interactive.IsInteractive() { + return interactive.ErrInputsRequired(create.ProblemMessages(problems)) } - return c.Create(cmd.Context(), in) - } - - appName, err := create.PromptForAppName(appName) - if err != nil { - return err - } - - language, err = create.PromptForLanguage(language) - if err != nil { - return err - } - template, err = create.PromptForTemplate(template, language) - if err != nil { - return err + p := problems[0] + switch p.Field { + case create.FieldName: + if appName != "" { + pterm.Warning.Println(p.Message) + } + v, err := create.PromptName() + if err != nil { + return err + } + appName = v + case create.FieldLanguage: + if language != "" { + pterm.Warning.Println(p.Message) + } + v, err := create.PromptLanguage() + if err != nil { + return err + } + language = v + case create.FieldTemplate: + if template != "" { + pterm.Warning.Println(p.Message) + } + v, err := create.PromptTemplate(in.Language) + if err != nil { + return err + } + template = v + case create.FieldOverwrite: + ok, err := create.PromptOverwrite(appName) + if err != nil { + return err + } + if !ok { + pterm.Warning.Println("Operation cancelled.") + return nil + } + skipConfirm = true + } } - - return c.Create(cmd.Context(), create.CreateInput{ - Name: appName, - Language: language, - Template: template, - SkipConfirm: skipConfirm, - }) } diff --git a/cmd/credential_providers.go b/cmd/credential_providers.go index 49d96225..d599606f 100644 --- a/cmd/credential_providers.go +++ b/cmd/credential_providers.go @@ -255,12 +255,13 @@ func (c CredentialProvidersCmd) Update(ctx context.Context, in CredentialProvide func (c CredentialProvidersCmd) Delete(ctx context.Context, in CredentialProvidersDeleteInput) error { if !in.SkipConfirm { - if !interactive.IsInteractive() { - return interactive.ErrConfirmationRequired(fmt.Sprintf("delete credential provider '%s'", in.ID)) + ok, err := interactive.Confirm( + fmt.Sprintf("delete credential provider '%s'", in.ID), + fmt.Sprintf("Are you sure you want to delete credential provider '%s'?", in.ID), + ) + if err != nil { + return err } - msg := fmt.Sprintf("Are you sure you want to delete credential provider '%s'?", in.ID) - pterm.DefaultInteractiveConfirm.DefaultText = msg - ok, _ := pterm.DefaultInteractiveConfirm.Show() if !ok { pterm.Info.Println("Deletion cancelled") return nil diff --git a/cmd/credentials.go b/cmd/credentials.go index dbb9455a..61d35165 100644 --- a/cmd/credentials.go +++ b/cmd/credentials.go @@ -282,12 +282,13 @@ func (c CredentialsCmd) Update(ctx context.Context, in CredentialsUpdateInput) e func (c CredentialsCmd) Delete(ctx context.Context, in CredentialsDeleteInput) error { if !in.SkipConfirm { - if !interactive.IsInteractive() { - return interactive.ErrConfirmationRequired(fmt.Sprintf("delete credential '%s'", in.Identifier)) + ok, err := interactive.Confirm( + fmt.Sprintf("delete credential '%s'", in.Identifier), + fmt.Sprintf("Are you sure you want to delete credential '%s'?", in.Identifier), + ) + if err != nil { + return err } - msg := fmt.Sprintf("Are you sure you want to delete credential '%s'?", in.Identifier) - pterm.DefaultInteractiveConfirm.DefaultText = msg - ok, _ := pterm.DefaultInteractiveConfirm.Show() if !ok { pterm.Info.Println("Deletion cancelled") return nil diff --git a/cmd/deploy.go b/cmd/deploy.go index d7430646..a5000d03 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -389,12 +389,13 @@ func runDeployDelete(cmd *cobra.Command, args []string) error { skipConfirm, _ := cmd.Flags().GetBool("yes") if !skipConfirm { - if !interactive.IsInteractive() { - return interactive.ErrConfirmationRequired(fmt.Sprintf("delete deployment '%s'", deploymentID)) + ok, err := interactive.Confirm( + fmt.Sprintf("delete deployment '%s'", deploymentID), + fmt.Sprintf("Are you sure you want to delete deployment '%s'? This cannot be undone.", deploymentID), + ) + if err != nil { + return err } - msg := fmt.Sprintf("Are you sure you want to delete deployment '%s'? This cannot be undone.", deploymentID) - pterm.DefaultInteractiveConfirm.DefaultText = msg - ok, _ := pterm.DefaultInteractiveConfirm.Show() if !ok { pterm.Info.Println("Deletion cancelled") return nil diff --git a/cmd/extensions.go b/cmd/extensions.go index 3fef666e..34bc02af 100644 --- a/cmd/extensions.go +++ b/cmd/extensions.go @@ -185,12 +185,13 @@ func (e ExtensionsCmd) Delete(ctx context.Context, in ExtensionsDeleteInput) err } if !in.SkipConfirm { - if !interactive.IsInteractive() { - return interactive.ErrConfirmationRequired(fmt.Sprintf("delete extension '%s'", in.Identifier)) + ok, err := interactive.Confirm( + fmt.Sprintf("delete extension '%s'", in.Identifier), + fmt.Sprintf("Are you sure you want to delete extension '%s'?", in.Identifier), + ) + if err != nil { + return err } - msg := fmt.Sprintf("Are you sure you want to delete extension '%s'?", in.Identifier) - pterm.DefaultInteractiveConfirm.DefaultText = msg - ok, _ := pterm.DefaultInteractiveConfirm.Show() if !ok { pterm.Info.Println("Deletion cancelled") return nil diff --git a/cmd/noninteractive_test.go b/cmd/noninteractive_test.go index 9cf7e7f5..a7419a07 100644 --- a/cmd/noninteractive_test.go +++ b/cmd/noninteractive_test.go @@ -1,20 +1,46 @@ package cmd import ( + "bytes" "context" + "os" + "os/exec" + "regexp" "testing" + "time" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/kernel-go-sdk/option" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// Under `go test` stdin is not a terminal, so any command path that would -// show an interactive confirmation must fail fast with a --yes hint instead -// of prompting (which would otherwise hang forever in agent/CI shells). +// TestMain doubles as an entry point for end-to-end subprocess tests: when +// KERNEL_CLI_E2E_EXEC=1 the test binary runs the real CLI (cmd.Execute) with +// os.Args instead of the test suite, so tests can assert on the final +// rendered stdout/stderr and exit code. +func TestMain(m *testing.M) { + if os.Getenv("KERNEL_CLI_E2E_EXEC") == "1" { + Execute(Metadata{Version: "0.0.0-test"}) + return + } + os.Exit(m.Run()) +} + +// forceNonInteractive arranges the condition under test explicitly instead of +// relying on the harness's ambient stdin (which may be a PTY). +func forceNonInteractive(t *testing.T) { + t.Helper() + t.Cleanup(interactive.ForceTerminal(false)) +} + +// Any command path that would show an interactive confirmation must fail fast +// with a --yes hint instead of prompting (which would otherwise hang forever +// in agent/CI shells). func TestAPIKeysDeleteFailsFastWhenNonInteractive(t *testing.T) { + forceNonInteractive(t) _ = capturePtermOutput(t) fake := &FakeAPIKeysService{ DeleteFunc: func(ctx context.Context, id string, opts ...option.RequestOption) error { @@ -31,10 +57,28 @@ func TestAPIKeysDeleteFailsFastWhenNonInteractive(t *testing.T) { assert.Contains(t, err.Error(), "not an interactive terminal") } +func TestExtensionsDeleteFailsFastWhenNonInteractive(t *testing.T) { + forceNonInteractive(t) + _ = capturePtermOutput(t) + fake := &FakeExtensionsService{ + DeleteFunc: func(ctx context.Context, idOrName string, opts ...option.RequestOption) error { + t.Fatal("delete must not be called without confirmation") + return nil + }, + } + e := ExtensionsCmd{extensions: fake} + + err := e.Delete(context.Background(), ExtensionsDeleteInput{Identifier: "e1"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "delete extension 'e1'") + assert.Contains(t, err.Error(), "--yes") +} + // In a non-interactive shell, `kernel create` must report every missing or -// invalid input in a single unwrapped error (no "failed to get ..." -// prefixes), so one retry can fix everything. +// invalid input in a single error so one retry can fix everything. func TestCreateFailsFastAggregatedWhenNonInteractive(t *testing.T) { + forceNonInteractive(t) + newCreateCmd := func(flags map[string]string) *cobra.Command { cmd := &cobra.Command{} cmd.Flags().String("name", "", "") @@ -72,18 +116,53 @@ func TestCreateFailsFastAggregatedWhenNonInteractive(t *testing.T) { }) } -func TestExtensionsDeleteFailsFastWhenNonInteractive(t *testing.T) { - _ = capturePtermOutput(t) - fake := &FakeExtensionsService{ - DeleteFunc: func(ctx context.Context, idOrName string, opts ...option.RequestOption) error { - t.Fatal("delete must not be called without confirmation") - return nil - }, - } - e := ExtensionsCmd{extensions: fake} +var ansiEscapes = regexp.MustCompile(`\x1b\[[0-9;]*[A-Za-z]|\x1b\][^\x07]*\x07`) - err := e.Delete(context.Background(), ExtensionsDeleteInput{Identifier: "e1"}) - require.Error(t, err) - assert.Contains(t, err.Error(), "delete extension 'e1'") - assert.Contains(t, err.Error(), "--yes") +// TestCreateNonInteractiveE2E runs the real CLI as a subprocess with stdin +// explicitly connected to /dev/null and asserts on the final rendered stderr: +// the process must exit 1 promptly (not hang on a prompt) and every flag +// token must survive rendering intact — i.e. not be split across lines by +// terminal-width re-wrapping. +func TestCreateNonInteractiveE2E(t *testing.T) { + exe, err := os.Executable() + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + devNull, err := os.Open(os.DevNull) + require.NoError(t, err) + defer devNull.Close() + + cmd := exec.CommandContext(ctx, exe, "create", "--language", "ruby") + cmd.Env = append(os.Environ(), "KERNEL_CLI_E2E_EXEC=1", "KERNEL_NO_UPDATE_CHECK=1") + cmd.Stdin = devNull + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + runErr := cmd.Run() + require.NoError(t, ctx.Err(), "CLI hung instead of failing fast; stderr=%q", stderr.String()) + var exitErr *exec.ExitError + require.ErrorAs(t, runErr, &exitErr, "expected non-zero exit; stdout=%q stderr=%q", stdout.String(), stderr.String()) + assert.Equal(t, 1, exitErr.ExitCode()) + + rendered := ansiEscapes.ReplaceAllString(stderr.String(), "") + for _, token := range []string{ + "--name is required", + "--language 'ruby' is invalid", + "--template is required", + "not an interactive terminal", + } { + assert.Contains(t, rendered, token, "rendered stderr must contain %q intact; full stderr:\n%s", token, rendered) + } + // Each problem renders on its own line (no width-based re-wrapping that + // would split flag tokens or merge problems). + for _, line := range []string{ + "\n - --name is required", + "\n - --language 'ruby' is invalid", + "\n - --template is required", + } { + assert.Contains(t, rendered, line, "each problem must start its own line; full stderr:\n%s", rendered) + } } diff --git a/cmd/profiles.go b/cmd/profiles.go index bb94c259..c3797bbd 100644 --- a/cmd/profiles.go +++ b/cmd/profiles.go @@ -228,12 +228,13 @@ func (p ProfilesCmd) Delete(ctx context.Context, in ProfilesDeleteInput) error { } if !in.SkipConfirm { - if !interactive.IsInteractive() { - return interactive.ErrConfirmationRequired(fmt.Sprintf("delete profile '%s'", in.Identifier)) + ok, err := interactive.Confirm( + fmt.Sprintf("delete profile '%s'", in.Identifier), + fmt.Sprintf("Are you sure you want to delete profile '%s'?", in.Identifier), + ) + if err != nil { + return err } - msg := fmt.Sprintf("Are you sure you want to delete profile '%s'?", in.Identifier) - pterm.DefaultInteractiveConfirm.DefaultText = msg - ok, _ := pterm.DefaultInteractiveConfirm.Show() if !ok { pterm.Info.Println("Deletion cancelled") return nil diff --git a/cmd/proxies/delete.go b/cmd/proxies/delete.go index 2bd3af21..d9e178ec 100644 --- a/cmd/proxies/delete.go +++ b/cmd/proxies/delete.go @@ -13,6 +13,8 @@ import ( func (p ProxyCmd) Delete(ctx context.Context, in ProxyDeleteInput) error { if !in.SkipConfirm { if !interactive.IsInteractive() { + // Fail fast before the proxy lookup below; the lookup only + // improves the prompt text. return interactive.ErrConfirmationRequired(fmt.Sprintf("delete proxy '%s'", in.ID)) } // Try to get the proxy details for better confirmation message @@ -32,8 +34,10 @@ func (p ProxyCmd) Delete(ctx context.Context, in ProxyDeleteInput) error { confirmMsg = fmt.Sprintf("Are you sure you want to delete proxy '%s'?", in.ID) } - pterm.DefaultInteractiveConfirm.DefaultText = confirmMsg - result, _ := pterm.DefaultInteractiveConfirm.Show() + result, err := interactive.Confirm(fmt.Sprintf("delete proxy '%s'", in.ID), confirmMsg) + if err != nil { + return err + } if !result { pterm.Info.Println("Deletion cancelled") return nil diff --git a/cmd/root.go b/cmd/root.go index ac9f18bb..de6c0d7c 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -9,12 +9,14 @@ import ( "runtime" "strings" "time" + "unicode" "github.com/charmbracelet/fang" "github.com/charmbracelet/lipgloss/v2" "github.com/kernel/cli/cmd/mcp" "github.com/kernel/cli/cmd/proxies" "github.com/kernel/cli/pkg/auth" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/cli/pkg/table" "github.com/kernel/cli/pkg/update" "github.com/kernel/cli/pkg/util" @@ -239,7 +241,19 @@ func Execute(m Metadata) { defer func() { pterm.Error.Writer = oldErrorWriter }() - pterm.Error.Println(errorTextStyle.Render(strings.TrimSpace(err.Error()))) + // Fail-fast interactivity errors render one problem per line. + // The default ErrorText style must not apply: its width-based + // word-wrap splits flag tokens like --template across lines, and + // its transform (strings.Fields + Join) collapses the newlines + // between problems. + msg := strings.TrimSpace(err.Error()) + style := errorTextStyle + var promptErr *interactive.PromptError + if errors.As(err, &promptErr) { + msg = capitalizeFirst(promptErr.Display()) + style = style.UnsetWidth().UnsetTransform() + } + pterm.Error.Println(style.Render(msg)) if isUsageError(err) { fmt.Fprintln(w) fmt.Fprintln(w, lipgloss.JoinHorizontal( @@ -259,6 +273,18 @@ func Execute(m Metadata) { // isUsageError is a hack to detect usage errors. // See: https://github.com/spf13/cobra/pull/2266 // from github.com/charmbracelet/fang/help.go +// capitalizeFirst uppercases the first letter of s, matching the visual +// convention of fang's default error transform (which we bypass for +// interactive.PromptError to preserve newlines and flag tokens). +func capitalizeFirst(s string) string { + r := []rune(s) + if len(r) == 0 { + return s + } + r[0] = unicode.ToUpper(r[0]) + return string(r) +} + func isUsageError(err error) bool { s := err.Error() for _, prefix := range []string{ diff --git a/pkg/create/prompts.go b/pkg/create/prompts.go index 5303c691..f8a17991 100644 --- a/pkg/create/prompts.go +++ b/pkg/create/prompts.go @@ -2,194 +2,63 @@ package create import ( "fmt" - "regexp" - "slices" - "strings" "github.com/kernel/cli/pkg/interactive" - "github.com/pterm/pterm" ) -// validateAppName validates that an app name follows the required format. -// Returns an error if the name is invalid. -func validateAppName(val any) error { - str, ok := val.(string) - if !ok { - return fmt.Errorf("invalid input type") - } - - if len(str) == 0 { - return fmt.Errorf("project name cannot be empty") - } - - // Validate project name: only letters, numbers, underscores, and hyphens - matched, err := regexp.MatchString(`^[A-Za-z\-_\d]+$`, str) - if err != nil { - return err - } - if !matched { - return fmt.Errorf("project name may only include letters, numbers, underscores, and hyphens") - } - return nil -} - -// languageOptionsHint renders the supported languages (with shorthands) for -// use in flag-usage hints, e.g. "typescript (ts), python (py)". -func languageOptionsHint() string { - opts := make([]string, 0, len(SupportedLanguages)) - for _, l := range SupportedLanguages { - if s := LanguageShorthand(l); s != "" { - opts = append(opts, fmt.Sprintf("%s (%s)", l, s)) - } else { - opts = append(opts, l) - } - } - return strings.Join(opts, ", ") -} - -// templateOptionsHint renders the template keys for use in flag-usage hints. -func templateOptionsHint(templateKVs TemplateKeyValues) string { - keys := make([]string, 0, len(templateKVs)) - for _, kv := range templateKVs { - keys = append(keys, kv.Key) - } - return strings.Join(keys, ", ") -} - -// handleAppNamePrompt prompts the user for an app name interactively. -func handleAppNamePrompt() (string, error) { - if !interactive.IsInteractive() { - return "", interactive.ErrInputRequired("app name", "pass --name to set the app name (e.g. --name "+DefaultAppName+")") - } - - promptText := fmt.Sprintf("%s (%s)", AppNamePrompt, DefaultAppName) - appName, err := pterm.DefaultInteractiveTextInput. - WithDefaultText(promptText). - Show() +// The prompts below are thin, per-field wrappers over pkg/interactive's +// prompt primitives. They perform no validation: ResolveInput owns +// validation and normalization for both the interactive and non-interactive +// flows, and the interactive flow re-resolves after each prompt. + +// PromptName prompts for the app name. An empty entry falls back to +// DefaultAppName. +func PromptName() (string, error) { + name, err := interactive.TextInput( + "app name", + "pass --name to set the app name (e.g. --name "+DefaultAppName+")", + fmt.Sprintf("%s (%s)", AppNamePrompt, DefaultAppName), + ) if err != nil { return "", err } - - if appName == "" { - appName = DefaultAppName + if name == "" { + return DefaultAppName, nil } - - if err := validateAppName(appName); err != nil { - pterm.Warning.Printf("Invalid app name '%s': %v\n", appName, err) - pterm.Info.Println("Please provide a valid app name.") - return handleAppNamePrompt() - } - - return appName, nil + return name, nil } -// PromptForAppName validates the provided app name or prompts the user for one. -// If the provided name is invalid, it shows a warning and prompts the user. -// In a non-interactive shell it fails fast instead of prompting. -func PromptForAppName(providedAppName string) (string, error) { - // If no app name was provided, prompt the user - if providedAppName == "" { - return handleAppNamePrompt() - } - - if err := validateAppName(providedAppName); err != nil { - if !interactive.IsInteractive() { - return "", fmt.Errorf("invalid --name '%s': %w", providedAppName, err) - } - pterm.Warning.Printf("Invalid app name '%s': %v\n", providedAppName, err) - pterm.Info.Println("Please provide a valid app name.") - return handleAppNamePrompt() - } - - return providedAppName, nil +// PromptLanguage prompts for the application language. +func PromptLanguage() (string, error) { + return interactive.Select( + "language selection", + "pass --language with one of: "+languageOptionsHint(), + LanguagePrompt, + SupportedLanguages, + ) } -func handleLanguagePrompt() (string, error) { - if !interactive.IsInteractive() { - return "", interactive.ErrInputRequired("language selection", "pass --language with one of: "+languageOptionsHint()) - } - - l, err := pterm.DefaultInteractiveSelect. - WithOptions(SupportedLanguages). - WithDefaultText(LanguagePrompt). - Show() +// PromptTemplate prompts for a template supported by the given (normalized) +// language. +func PromptTemplate(language string) (string, error) { + templateKVs := GetSupportedTemplatesForLanguage(language) + display, err := interactive.Select( + "template selection", + "pass --template with one of: "+templateOptionsHint(templateKVs), + TemplatePrompt, + templateKVs.GetTemplateDisplayValues(), + ) if err != nil { return "", err } - return l, nil + return templateKVs.GetTemplateKeyFromValue(display) } -// PromptForLanguage validates the provided language or prompts the user for -// one. In a non-interactive shell it fails fast instead of prompting. -func PromptForLanguage(providedLanguage string) (string, error) { - if providedLanguage == "" { - return handleLanguagePrompt() - } - - l := NormalizeLanguage(providedLanguage) - if slices.Contains(SupportedLanguages, l) { - return l, nil - } - - if !interactive.IsInteractive() { - return "", fmt.Errorf("invalid --language '%s': must be one of: %s", providedLanguage, languageOptionsHint()) - } - pterm.Warning.Printfln("Language '%s' not found. Please select from available languages.\n", providedLanguage) - return handleLanguagePrompt() -} - -func handleTemplatePrompt(templateKVs TemplateKeyValues) (string, error) { - if !interactive.IsInteractive() { - return "", interactive.ErrInputRequired("template selection", "pass --template with one of: "+templateOptionsHint(templateKVs)) - } - - template, err := pterm.DefaultInteractiveSelect. - WithOptions(templateKVs.GetTemplateDisplayValues()). - WithDefaultText(TemplatePrompt). - WithMaxHeight(len(templateKVs)). - Show() - if err != nil { - return "", err - } - - return templateKVs.GetTemplateKeyFromValue(template) -} - -// PromptForTemplate validates the provided template or prompts the user for -// one. In a non-interactive shell it fails fast instead of prompting. -func PromptForTemplate(providedTemplate string, providedLanguage string) (string, error) { - templateKVs := GetSupportedTemplatesForLanguage(NormalizeLanguage(providedLanguage)) - - if providedTemplate == "" { - return handleTemplatePrompt(templateKVs) - } - - if templateKVs.ContainsKey(providedTemplate) { - return providedTemplate, nil - } - - if !interactive.IsInteractive() { - return "", fmt.Errorf("invalid --template '%s' for language '%s': must be one of: %s", providedTemplate, NormalizeLanguage(providedLanguage), templateOptionsHint(templateKVs)) - } - pterm.Warning.Printfln("Template '%s' not found. Please select from available templates.\n", providedTemplate) - return handleTemplatePrompt(templateKVs) -} - -// PromptForOverwrite prompts the user to confirm overwriting an existing -// directory. In a non-interactive shell it fails fast instead of prompting; -// pass --yes to overwrite without confirmation. -func PromptForOverwrite(dirName string) (bool, error) { - if !interactive.IsInteractive() { - return false, interactive.ErrConfirmationRequired(fmt.Sprintf("overwrite existing directory '%s'", dirName)) - } - - overwrite, err := pterm.DefaultInteractiveConfirm. - WithDefaultText(fmt.Sprintf("\nDirectory %s already exists. Overwrite?", dirName)). - WithDefaultValue(false). - Show() - if err != nil { - return false, fmt.Errorf("failed to prompt for overwrite: %w", err) - } - - return overwrite, nil +// PromptOverwrite asks for confirmation before overwriting an existing +// directory. +func PromptOverwrite(dirName string) (bool, error) { + return interactive.Confirm( + fmt.Sprintf("overwrite existing directory '%s'", dirName), + fmt.Sprintf("Directory %s already exists. Overwrite?", dirName), + ) } diff --git a/pkg/create/prompts_test.go b/pkg/create/prompts_test.go index c1b2a415..8f2136d0 100644 --- a/pkg/create/prompts_test.go +++ b/pkg/create/prompts_test.go @@ -3,83 +3,35 @@ package create import ( "testing" + "github.com/kernel/cli/pkg/interactive" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// These tests run under `go test`, where stdin is not a terminal, so every -// prompt path must fail fast with a flag-usage hint instead of prompting. +// Every prompt wrapper must fail fast with a flag-usage hint when the shell +// is non-interactive. The terminal capability is injected explicitly so the +// tests do not depend on the harness's ambient stdin. +func TestPromptsFailFastWhenNonInteractive(t *testing.T) { + t.Cleanup(interactive.ForceTerminal(false)) -func TestPromptForAppNameNonInteractive(t *testing.T) { - t.Run("missing name fails fast with --name hint", func(t *testing.T) { - _, err := PromptForAppName("") - require.Error(t, err) - assert.Contains(t, err.Error(), "--name") - assert.Contains(t, err.Error(), "not an interactive terminal") - }) - - t.Run("invalid name fails fast with validation error", func(t *testing.T) { - _, err := PromptForAppName("bad name!") - require.Error(t, err) - assert.Contains(t, err.Error(), "invalid --name 'bad name!'") - }) - - t.Run("valid name passes through", func(t *testing.T) { - name, err := PromptForAppName("my-app") - require.NoError(t, err) - assert.Equal(t, "my-app", name) - }) -} - -func TestPromptForLanguageNonInteractive(t *testing.T) { - t.Run("missing language fails fast with --language hint", func(t *testing.T) { - _, err := PromptForLanguage("") - require.Error(t, err) - assert.Contains(t, err.Error(), "--language") - assert.Contains(t, err.Error(), LanguageTypeScript) - assert.Contains(t, err.Error(), LanguagePython) - }) - - t.Run("invalid language fails fast listing options", func(t *testing.T) { - _, err := PromptForLanguage("ruby") - require.Error(t, err) - assert.Contains(t, err.Error(), "invalid --language 'ruby'") - assert.Contains(t, err.Error(), LanguageTypeScript) - }) - - t.Run("valid language passes through", func(t *testing.T) { - l, err := PromptForLanguage("ts") - require.NoError(t, err) - assert.Equal(t, LanguageTypeScript, l) - }) -} - -func TestPromptForTemplateNonInteractive(t *testing.T) { - t.Run("missing template fails fast with --template hint", func(t *testing.T) { - _, err := PromptForTemplate("", LanguageTypeScript) - require.Error(t, err) - assert.Contains(t, err.Error(), "--template") - assert.Contains(t, err.Error(), TemplateSampleApp) - }) + _, err := PromptName() + require.Error(t, err) + assert.Contains(t, err.Error(), "--name") + assert.Contains(t, err.Error(), "not an interactive terminal") - t.Run("invalid template fails fast listing options", func(t *testing.T) { - _, err := PromptForTemplate("nope", LanguageTypeScript) - require.Error(t, err) - assert.Contains(t, err.Error(), "invalid --template 'nope'") - assert.Contains(t, err.Error(), TemplateSampleApp) - }) + _, err = PromptLanguage() + require.Error(t, err) + assert.Contains(t, err.Error(), "--language") + assert.Contains(t, err.Error(), LanguageTypeScript) - t.Run("valid template passes through", func(t *testing.T) { - tmpl, err := PromptForTemplate(TemplateSampleApp, LanguageTypeScript) - require.NoError(t, err) - assert.Equal(t, TemplateSampleApp, tmpl) - }) -} + _, err = PromptTemplate(LanguageTypeScript) + require.Error(t, err) + assert.Contains(t, err.Error(), "--template") + assert.Contains(t, err.Error(), TemplateSampleApp) -func TestPromptForOverwriteNonInteractive(t *testing.T) { - overwrite, err := PromptForOverwrite("existing-dir") + ok, err := PromptOverwrite("existing-dir") require.Error(t, err) - assert.False(t, overwrite) + assert.False(t, ok) assert.Contains(t, err.Error(), "overwrite existing directory 'existing-dir'") assert.Contains(t, err.Error(), "--yes") } diff --git a/pkg/create/resolve.go b/pkg/create/resolve.go new file mode 100644 index 00000000..4d82764d --- /dev/null +++ b/pkg/create/resolve.go @@ -0,0 +1,157 @@ +package create + +import ( + "fmt" + "os" + "regexp" + "slices" + "strings" +) + +// Field identifies a scaffolding input that a Problem refers to. +type Field string + +const ( + FieldName Field = "name" + FieldLanguage Field = "language" + FieldTemplate Field = "template" + FieldOverwrite Field = "overwrite" +) + +// Problem describes one input that is missing, invalid, or would require a +// confirmation. Message is phrased as the non-interactive fix instruction +// (naming the exact flag to pass); the interactive flow surfaces the same +// message as a warning before prompting for the field. +type Problem struct { + Field Field + Message string +} + +// ProblemMessages extracts the messages from problems, in order. +func ProblemMessages(problems []Problem) []string { + msgs := make([]string, len(problems)) + for i, p := range problems { + msgs[i] = p.Message + } + return msgs +} + +// validateAppName validates that an app name follows the required format. +// Returns an error if the name is invalid. +func validateAppName(val any) error { + str, ok := val.(string) + if !ok { + return fmt.Errorf("invalid input type") + } + + if len(str) == 0 { + return fmt.Errorf("project name cannot be empty") + } + + // Validate project name: only letters, numbers, underscores, and hyphens + matched, err := regexp.MatchString(`^[A-Za-z\-_\d]+$`, str) + if err != nil { + return err + } + if !matched { + return fmt.Errorf("project name may only include letters, numbers, underscores, and hyphens") + } + return nil +} + +// languageOptionsHint renders the supported languages (with shorthands) for +// use in flag-usage hints, e.g. "typescript (ts), python (py)". +func languageOptionsHint() string { + opts := make([]string, 0, len(SupportedLanguages)) + for _, l := range SupportedLanguages { + if s := LanguageShorthand(l); s != "" { + opts = append(opts, fmt.Sprintf("%s (%s)", l, s)) + } else { + opts = append(opts, l) + } + } + return strings.Join(opts, ", ") +} + +// templateOptionsHint renders the template keys for use in flag-usage hints. +func templateOptionsHint(templateKVs TemplateKeyValues) string { + keys := make([]string, 0, len(templateKVs)) + for _, kv := range templateKVs { + keys = append(keys, kv.Key) + } + return strings.Join(keys, ", ") +} + +// ResolveInput is the single canonical resolver from raw flag values to a +// normalized CreateInput. It validates and normalizes every field, collecting +// a Problem for each one that is missing, invalid, or (for an existing target +// directory) would require an overwrite confirmation. It never prompts. +// +// Both modes of `kernel create` consume its problem model: the interactive +// flow prompts to resolve one problem at a time and re-resolves, while the +// non-interactive flow reports all problems in a single fail-fast error. +// Problems are ordered name, language, template, overwrite so interactive +// resolution fixes the language before the language-dependent template list +// is needed. +// +// The returned CreateInput carries the fields that did resolve (with the +// language normalized) even when problems remain. +func ResolveInput(name, language, template string, skipOverwriteConfirm bool) (CreateInput, []Problem) { + var problems []Problem + + // --name + nameValid := false + if name == "" { + problems = append(problems, Problem{FieldName, "--name is required (e.g. --name " + DefaultAppName + ")"}) + } else if err := validateAppName(name); err != nil { + problems = append(problems, Problem{FieldName, fmt.Sprintf("--name '%s' is invalid: %v", name, err)}) + } else { + nameValid = true + } + + // --language + lang := "" + if language == "" { + problems = append(problems, Problem{FieldLanguage, "--language is required: one of: " + languageOptionsHint()}) + } else if l := NormalizeLanguage(language); slices.Contains(SupportedLanguages, l) { + lang = l + } else { + problems = append(problems, Problem{FieldLanguage, fmt.Sprintf("--language '%s' is invalid: must be one of: %s", language, languageOptionsHint())}) + } + + // --template (valid values depend on --language) + templateValid := false + if lang != "" { + templateKVs := GetSupportedTemplatesForLanguage(lang) + if template == "" { + problems = append(problems, Problem{FieldTemplate, "--template is required: one of: " + templateOptionsHint(templateKVs)}) + } else if templateKVs.ContainsKey(template) { + templateValid = true + } else { + problems = append(problems, Problem{FieldTemplate, fmt.Sprintf("--template '%s' is invalid for language '%s': must be one of: %s", template, lang, templateOptionsHint(templateKVs))}) + } + } else if template == "" { + problems = append(problems, Problem{FieldTemplate, "--template is required (run 'kernel create --help' for the full list)"}) + } else if _, ok := Templates[template]; !ok { + // Language is unknown, but the template doesn't exist for any + // language, so report it now rather than on the next retry. + problems = append(problems, Problem{FieldTemplate, fmt.Sprintf("--template '%s' is invalid (run 'kernel create --help' for the full list)", template)}) + } + + // Overwriting the target directory would need a confirmation prompt. + if nameValid && !skipOverwriteConfirm { + if _, err := os.Stat(name); err == nil { + problems = append(problems, Problem{FieldOverwrite, fmt.Sprintf("directory '%s' already exists: pass --yes to overwrite it, or choose a different --name", name)}) + } + } + + in := CreateInput{ + Name: name, + Language: lang, + SkipConfirm: skipOverwriteConfirm, + } + if templateValid { + in.Template = template + } + return in, problems +} diff --git a/pkg/create/resolve_test.go b/pkg/create/resolve_test.go new file mode 100644 index 00000000..31904f5e --- /dev/null +++ b/pkg/create/resolve_test.go @@ -0,0 +1,79 @@ +package create + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ResolveInput never prompts and never inspects the terminal; these tests +// exercise the problem model both flows consume. + +func problemFields(problems []Problem) []Field { + fields := make([]Field, len(problems)) + for i, p := range problems { + fields[i] = p.Field + } + return fields +} + +func TestResolveInputReportsAllProblems(t *testing.T) { + _, problems := ResolveInput("", "", "", false) + assert.Equal(t, []Field{FieldName, FieldLanguage, FieldTemplate}, problemFields(problems)) + + msgs := ProblemMessages(problems) + assert.Contains(t, msgs[0], "--name is required") + assert.Contains(t, msgs[1], "--language is required") + assert.Contains(t, msgs[2], "--template is required") +} + +func TestResolveInputInvalidValues(t *testing.T) { + _, problems := ResolveInput("bad name!", "ruby", "nope", false) + require.Len(t, problems, 3) + assert.Contains(t, problems[0].Message, "--name 'bad name!' is invalid") + assert.Contains(t, problems[1].Message, "--language 'ruby' is invalid") + // Language is unknown, but the template exists for no language at all, + // so it must still be reported in the same pass. + assert.Contains(t, problems[2].Message, "--template 'nope' is invalid") +} + +func TestResolveInputTemplateValidatedPerLanguage(t *testing.T) { + in, problems := ResolveInput("my-app", "ts", "nope", false) + require.Len(t, problems, 1) + assert.Equal(t, FieldTemplate, problems[0].Field) + assert.Contains(t, problems[0].Message, "--template 'nope' is invalid for language 'typescript'") + assert.Contains(t, problems[0].Message, TemplateSampleApp) + // Resolved fields are returned even when problems remain, so the + // interactive flow can prompt for the template of the chosen language. + assert.Equal(t, LanguageTypeScript, in.Language) + assert.Empty(t, in.Template) +} + +func TestResolveInputExistingDirectory(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "my-app"), 0o755)) + + _, problems := ResolveInput("my-app", "ts", TemplateSampleApp, false) + require.Len(t, problems, 1) + assert.Equal(t, FieldOverwrite, problems[0].Field) + assert.Contains(t, problems[0].Message, "directory 'my-app' already exists") + assert.Contains(t, problems[0].Message, "--yes") + + // --yes skips the overwrite confirmation, so it is not a problem. + in, problems := ResolveInput("my-app", "ts", TemplateSampleApp, true) + assert.Empty(t, problems) + assert.True(t, in.SkipConfirm) +} + +func TestResolveInputValid(t *testing.T) { + in, problems := ResolveInput("my-app", "py", TemplateSampleApp, false) + assert.Empty(t, problems) + assert.Equal(t, "my-app", in.Name) + assert.Equal(t, LanguagePython, in.Language) + assert.Equal(t, TemplateSampleApp, in.Template) + assert.False(t, in.SkipConfirm) +} diff --git a/pkg/create/validate.go b/pkg/create/validate.go deleted file mode 100644 index f7165cf8..00000000 --- a/pkg/create/validate.go +++ /dev/null @@ -1,72 +0,0 @@ -package create - -import ( - "fmt" - "os" - "slices" - - "github.com/kernel/cli/pkg/interactive" -) - -// ValidateNonInteractive validates every scaffolding input without prompting -// and aggregates all missing or invalid flags into a single error, so a -// non-interactive caller (usually an AI agent) can fix everything in one -// retry. It also flags an existing target directory when skipOverwriteConfirm -// is false, since that would otherwise require an interactive confirmation. -func ValidateNonInteractive(name, language, template string, skipOverwriteConfirm bool) (CreateInput, error) { - var problems []string - - // --name - nameValid := false - if name == "" { - problems = append(problems, "--name is required (e.g. --name "+DefaultAppName+")") - } else if err := validateAppName(name); err != nil { - problems = append(problems, fmt.Sprintf("--name '%s' is invalid: %v", name, err)) - } else { - nameValid = true - } - - // --language - lang := "" - if language == "" { - problems = append(problems, "--language is required: one of: "+languageOptionsHint()) - } else if l := NormalizeLanguage(language); slices.Contains(SupportedLanguages, l) { - lang = l - } else { - problems = append(problems, fmt.Sprintf("--language '%s' is invalid: must be one of: %s", language, languageOptionsHint())) - } - - // --template (valid values depend on --language) - if lang != "" { - templateKVs := GetSupportedTemplatesForLanguage(lang) - if template == "" { - problems = append(problems, "--template is required: one of: "+templateOptionsHint(templateKVs)) - } else if !templateKVs.ContainsKey(template) { - problems = append(problems, fmt.Sprintf("--template '%s' is invalid for language '%s': must be one of: %s", template, lang, templateOptionsHint(templateKVs))) - } - } else if template == "" { - problems = append(problems, "--template is required (run 'kernel create --help' for the full list)") - } else if _, ok := Templates[template]; !ok { - // Language is unknown, but the template doesn't exist for any - // language, so report it now rather than on the next retry. - problems = append(problems, fmt.Sprintf("--template '%s' is invalid (run 'kernel create --help' for the full list)", template)) - } - - // Overwriting the target directory would need a confirmation prompt. - if nameValid && !skipOverwriteConfirm { - if _, err := os.Stat(name); err == nil { - problems = append(problems, fmt.Sprintf("directory '%s' already exists: pass --yes to overwrite it, or choose a different --name", name)) - } - } - - if len(problems) > 0 { - return CreateInput{}, interactive.ErrInputsRequired(problems) - } - - return CreateInput{ - Name: name, - Language: lang, - Template: template, - SkipConfirm: skipOverwriteConfirm, - }, nil -} diff --git a/pkg/create/validate_test.go b/pkg/create/validate_test.go deleted file mode 100644 index c531bf21..00000000 --- a/pkg/create/validate_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package create - -import ( - "os" - "path/filepath" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestValidateNonInteractiveAggregatesAllProblems(t *testing.T) { - _, err := ValidateNonInteractive("", "", "", false) - require.Error(t, err) - assert.Contains(t, err.Error(), "--name is required") - assert.Contains(t, err.Error(), "--language is required") - assert.Contains(t, err.Error(), "--template is required") - assert.Contains(t, err.Error(), "not an interactive terminal") -} - -func TestValidateNonInteractiveInvalidValues(t *testing.T) { - _, err := ValidateNonInteractive("bad name!", "ruby", "nope", false) - require.Error(t, err) - assert.Contains(t, err.Error(), "--name 'bad name!' is invalid") - assert.Contains(t, err.Error(), "--language 'ruby' is invalid") - // Language is unknown, but the template exists for no language at all, - // so it must still be reported in the same error. - assert.Contains(t, err.Error(), "--template 'nope' is invalid") -} - -func TestValidateNonInteractiveTemplateValidatedPerLanguage(t *testing.T) { - _, err := ValidateNonInteractive("my-app", "ts", "nope", false) - require.Error(t, err) - assert.Contains(t, err.Error(), "--template 'nope' is invalid for language 'typescript'") - assert.Contains(t, err.Error(), TemplateSampleApp) -} - -func TestValidateNonInteractiveExistingDirectory(t *testing.T) { - dir := t.TempDir() - t.Chdir(dir) - require.NoError(t, os.MkdirAll(filepath.Join(dir, "my-app"), 0o755)) - - _, err := ValidateNonInteractive("my-app", "ts", TemplateSampleApp, false) - require.Error(t, err) - assert.Contains(t, err.Error(), "directory 'my-app' already exists") - assert.Contains(t, err.Error(), "--yes") - - // --yes skips the overwrite confirmation, so it is not a problem. - in, err := ValidateNonInteractive("my-app", "ts", TemplateSampleApp, true) - require.NoError(t, err) - assert.True(t, in.SkipConfirm) -} - -func TestValidateNonInteractiveValidInput(t *testing.T) { - in, err := ValidateNonInteractive("my-app", "py", TemplateSampleApp, false) - require.NoError(t, err) - assert.Equal(t, "my-app", in.Name) - assert.Equal(t, LanguagePython, in.Language) - assert.Equal(t, TemplateSampleApp, in.Template) - assert.False(t, in.SkipConfirm) -} diff --git a/pkg/interactive/interactive.go b/pkg/interactive/interactive.go index 9d0c0bb2..6cf9040b 100644 --- a/pkg/interactive/interactive.go +++ b/pkg/interactive/interactive.go @@ -1,11 +1,13 @@ -// Package interactive reports whether the CLI can show interactive prompts -// and builds the fail-fast errors used when it cannot. +// Package interactive owns every interactive prompt in the CLI: it detects +// whether stdin is attached to a terminal, executes pterm confirm/select/text +// prompts when it is, and fails fast with actionable errors when it is not. // -// Interactive prompts (pterm confirm/select/text input) read keystrokes from -// the terminal. In a non-interactive shell — an AI agent's bash tool, CI, or -// any piped stdin — they never return (the underlying keyboard listener spins -// forever), so every prompt call site must gate on IsInteractive first and -// fail fast with instructions for avoiding the prompt. +// Interactive prompts read keystrokes from the terminal. In a non-interactive +// shell — an AI agent's bash tool, CI, or any piped stdin — they never return +// (the underlying keyboard listener spins forever), so prompt execution is +// centralized here behind the TTY gate. Command code must call Confirm, +// Select, and TextInput instead of pterm's interactive printers; no direct +// pterm interactive calls should exist outside this package. package interactive import ( @@ -13,13 +15,79 @@ import ( "os" "strings" + "github.com/pterm/pterm" "golang.org/x/term" ) +// terminalCheck reports whether the given file is attached to a terminal. It +// is a package variable so tests can inject a fake terminal capability +// instead of relying on the ambient stdin of the test harness. +var terminalCheck = func(f *os.File) bool { + return term.IsTerminal(int(f.Fd())) +} + // IsInteractive reports whether stdin is attached to a terminal, i.e. whether // interactive prompts can be shown. func IsInteractive() bool { - return term.IsTerminal(int(os.Stdin.Fd())) + return terminalCheck(os.Stdin) +} + +// ForceTerminal overrides terminal detection for tests, making IsInteractive +// report isTTY regardless of the ambient stdin. It returns a function that +// restores the previous behavior; callers should register it with t.Cleanup. +func ForceTerminal(isTTY bool) (restore func()) { + prev := terminalCheck + terminalCheck = func(*os.File) bool { return isTTY } + return func() { terminalCheck = prev } +} + +// PromptError is returned instead of showing an interactive prompt when +// stdin is not a terminal. It carries every problem the caller must fix so a +// single retry can succeed. The CLI's top-level error handler renders it via +// Display without width-based re-wrapping, so flag tokens such as --yes or +// --template are never split across lines. +type PromptError struct { + // What names what would have been prompted for, e.g. "input" or + // "confirmation to delete profile 'foo'". + What string + // Problems lists the fixes, e.g. "--name is required" or "re-run with + // --yes to skip the confirmation prompt". Always at least one entry. + Problems []string +} + +// Error renders the problems inline on a single line, following the Go +// convention that error strings do not contain newlines. +func (e *PromptError) Error() string { + header := fmt.Sprintf("cannot prompt for %s: stdin is not an interactive terminal", e.What) + if len(e.Problems) == 1 { + return header + "; " + e.Problems[0] + } + var b strings.Builder + b.WriteString(header) + b.WriteString("; fix all of the following and re-run:") + for i, p := range e.Problems { + if i > 0 { + b.WriteString(";") + } + fmt.Fprintf(&b, " (%d) %s", i+1, p) + } + return b.String() +} + +// Display renders the problems for terminal output, one per line, so each +// fix instruction stays an intact, greppable token sequence regardless of +// terminal width. +func (e *PromptError) Display() string { + if len(e.Problems) == 1 { + return e.Error() + } + var b strings.Builder + fmt.Fprintf(&b, "cannot prompt for %s: stdin is not an interactive terminal; fix all of the following and re-run:", e.What) + for _, p := range e.Problems { + b.WriteString("\n - ") + b.WriteString(p) + } + return b.String() } // ErrConfirmationRequired builds the fail-fast error for confirmation @@ -27,15 +95,18 @@ func IsInteractive() bool { // "delete profile 'foo'". The resulting error tells the caller (often an AI // agent) to re-run with --yes. func ErrConfirmationRequired(action string) error { - return fmt.Errorf("cannot prompt for confirmation to %s: stdin is not an interactive terminal; re-run with --yes to skip the confirmation prompt", action) + return &PromptError{ + What: "confirmation to " + action, + Problems: []string{"re-run with --yes to skip the confirmation prompt"}, + } } -// ErrInputRequired builds the fail-fast error for text/select prompts. what -// describes the input that would have been prompted for, e.g. "app name"; -// hint names the flag(s) to pass instead, e.g. "pass --name to set the app -// name". +// ErrInputRequired builds the fail-fast error for a single text/select +// prompt. what describes the input that would have been prompted for, e.g. +// "app name"; hint names the flag(s) to pass instead, e.g. "pass --name to +// set the app name". func ErrInputRequired(what, hint string) error { - return fmt.Errorf("cannot prompt for %s: stdin is not an interactive terminal; %s", what, hint) + return &PromptError{What: what, Problems: []string{hint}} } // ErrInputsRequired builds the fail-fast error for one or more missing or @@ -43,24 +114,48 @@ func ErrInputRequired(what, hint string) error { // them all up front and report every problem in a single error, so a // non-interactive caller can fix everything in one retry instead of // discovering problems one invocation at a time. -// -// Problems are numbered inline — "(1) ...; (2) ..." — rather than -// newline-separated because the CLI error renderer reflows text and would -// collapse line breaks anyway. func ErrInputsRequired(problems []string) error { - switch len(problems) { - case 0: + if len(problems) == 0 { return nil - case 1: - return fmt.Errorf("cannot prompt for input: stdin is not an interactive terminal; %s", problems[0]) - default: - var b strings.Builder - for i, p := range problems { - if i > 0 { - b.WriteString("; ") - } - fmt.Fprintf(&b, "(%d) %s", i+1, p) - } - return fmt.Errorf("cannot prompt for input: stdin is not an interactive terminal; fix all of the following and re-run: %s", b.String()) } + return &PromptError{What: "input", Problems: problems} +} + +// Confirm shows a yes/no confirmation prompt with the given prompt text and +// reports the choice. In a non-interactive shell it fails fast with +// ErrConfirmationRequired(action) instead of prompting. +func Confirm(action, promptText string) (bool, error) { + if !IsInteractive() { + return false, ErrConfirmationRequired(action) + } + return pterm.DefaultInteractiveConfirm. + WithDefaultText(promptText). + WithDefaultValue(false). + Show() +} + +// Select shows a select prompt over options and returns the chosen option. +// In a non-interactive shell it fails fast with ErrInputRequired(what, hint) +// instead of prompting. +func Select(what, hint, promptText string, options []string) (string, error) { + if !IsInteractive() { + return "", ErrInputRequired(what, hint) + } + return pterm.DefaultInteractiveSelect. + WithOptions(options). + WithDefaultText(promptText). + WithMaxHeight(len(options)). + Show() +} + +// TextInput shows a free-text prompt and returns the entered text. In a +// non-interactive shell it fails fast with ErrInputRequired(what, hint) +// instead of prompting. +func TextInput(what, hint, promptText string) (string, error) { + if !IsInteractive() { + return "", ErrInputRequired(what, hint) + } + return pterm.DefaultInteractiveTextInput. + WithDefaultText(promptText). + Show() } diff --git a/pkg/interactive/interactive_test.go b/pkg/interactive/interactive_test.go index dab35859..599ba564 100644 --- a/pkg/interactive/interactive_test.go +++ b/pkg/interactive/interactive_test.go @@ -1,23 +1,75 @@ package interactive import ( + "os" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -// Under `go test` stdin is not attached to a terminal, so IsInteractive must -// report false. This is the same environment as CI pipelines and AI-agent -// shells — exactly the case the fail-fast gating exists for. -func TestIsInteractiveFalseWithoutTerminal(t *testing.T) { +// The default detector must report false for a file descriptor that is +// explicitly not a terminal (a pipe), independent of the test harness's +// ambient stdin. +func TestTerminalCheckReportsFalseForPipe(t *testing.T) { + r, w, err := os.Pipe() + require.NoError(t, err) + defer r.Close() + defer w.Close() + + assert.False(t, terminalCheck(r)) + assert.False(t, terminalCheck(w)) +} + +func TestForceTerminalOverridesDetection(t *testing.T) { + restore := ForceTerminal(true) + assert.True(t, IsInteractive()) + restore() + + t.Cleanup(ForceTerminal(false)) assert.False(t, IsInteractive()) } -func TestErrConfirmationRequired(t *testing.T) { +func TestPromptErrorSingleProblem(t *testing.T) { err := ErrConfirmationRequired("delete profile 'foo'") + + var promptErr *PromptError + require.ErrorAs(t, err, &promptErr) assert.Contains(t, err.Error(), "delete profile 'foo'") assert.Contains(t, err.Error(), "--yes") assert.Contains(t, err.Error(), "not an interactive terminal") + // Single-problem errors render identically in logs and on screen. + assert.Equal(t, err.Error(), promptErr.Display()) + assert.NotContains(t, err.Error(), "\n") +} + +func TestPromptErrorMultiProblemRendering(t *testing.T) { + err := ErrInputsRequired([]string{ + "--name is required", + "--language is required: one of: typescript (ts), python (py)", + "--template is required", + }) + + var promptErr *PromptError + require.ErrorAs(t, err, &promptErr) + + // Error() follows Go convention: single line, numbered problems. + msg := err.Error() + assert.NotContains(t, msg, "\n") + assert.Contains(t, msg, "(1) --name is required") + assert.Contains(t, msg, "(2) --language is required") + assert.Contains(t, msg, "(3) --template is required") + + // Display() puts each problem on its own line so flag tokens are never + // split by terminal-width re-wrapping. + display := promptErr.Display() + assert.Contains(t, display, "\n - --name is required") + assert.Contains(t, display, "\n - --language is required") + assert.Contains(t, display, "\n - --template is required") +} + +func TestErrInputsRequiredEmptyIsNil(t *testing.T) { + assert.NoError(t, ErrInputsRequired(nil)) } func TestErrInputRequired(t *testing.T) { @@ -26,3 +78,25 @@ func TestErrInputRequired(t *testing.T) { assert.Contains(t, err.Error(), "pass --name") assert.Contains(t, err.Error(), "not an interactive terminal") } + +// The prompt primitives must fail fast (never touch pterm) when the shell is +// non-interactive. +func TestPromptPrimitivesFailFastWhenNonInteractive(t *testing.T) { + t.Cleanup(ForceTerminal(false)) + + ok, err := Confirm("delete widget 'w'", "Are you sure?") + require.Error(t, err) + assert.False(t, ok) + assert.Contains(t, err.Error(), "delete widget 'w'") + assert.Contains(t, err.Error(), "--yes") + + _, err = Select("widget selection", "pass --widget", "Pick a widget:", []string{"a", "b"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "widget selection") + assert.Contains(t, err.Error(), "pass --widget") + + _, err = TextInput("widget name", "pass --name", "Name?") + require.Error(t, err) + assert.Contains(t, err.Error(), "widget name") + assert.Contains(t, err.Error(), "pass --name") +} From 954f36411b7dca7804802bf4e2fcf41c5f9d6ed1 Mon Sep 17 00:00:00 2001 From: Rafael Garcia Date: Fri, 24 Jul 2026 15:55:10 -0400 Subject: [PATCH 5/5] Carry terminal capability on a Prompter; make problem resolution exhaustive Addresses masnwilliams' second review round on #209: 1. No process-global test hook. ForceTerminal and the mutable terminalCheck package variable are gone. interactive.Prompter now carries the terminal capability per value: the zero value / NewPrompter() detects from stdin at prompt time, and NewPrompterWithTerminal(false) gives tests a fixed capability without mutating package state, so opposite terminal states coexist across parallel tests and nothing in the runtime API can flip terminal policy globally. Confirm/Select/TextInput are Prompter methods; the package-level helpers delegate to the default (ambient, immutable) Prompter for plain-function commands. Command structs that confirm (api-keys, auth connections, credentials, credential providers, extensions, profiles, proxies, create) carry an injected prompter field whose zero value preserves existing construction. 2. Exhaustive-by-construction problem dispatch. create.ResolveInput now emits every Problem with its own interactive resolution step (resolve closure applying the prompt answer to RawInput), so the cmd-side field switch is gone and a Problem cannot exist without a handler. Problem.Resolve also defends against a hand-constructed Problem with an internal error naming the field instead of letting the loop spin. Problem.Invalid replaces the raw-value inspection for the warn-before-reprompt behavior. Tests: gating tests inject NewPrompterWithTerminal(false) (no global state; marked parallel where possible), a regression test asserts every ResolveInput problem carries a resolution step across all field shapes, and the nil-step internal error is covered. Verified the compiled test binaries pass under a PTY harness, with -race -shuffle=on -count=2, and the interactive flow end-to-end under a real PTY (invalid-language warn -> language select -> template select -> scaffold). --- cmd/api_keys.go | 5 +- cmd/auth_connections.go | 5 +- cmd/create.go | 83 ++++++++----------- cmd/credential_providers.go | 3 +- cmd/credentials.go | 3 +- cmd/extensions.go | 3 +- cmd/noninteractive_test.go | 25 ++---- cmd/profiles.go | 3 +- cmd/proxies/delete.go | 4 +- cmd/proxies/types.go | 4 +- pkg/create/prompts.go | 16 ++-- pkg/create/prompts_test.go | 18 ++-- pkg/create/resolve.go | 124 ++++++++++++++++++++++------ pkg/create/resolve_test.go | 81 ++++++++++++++++-- pkg/interactive/interactive.go | 104 ++++++++++++++++------- pkg/interactive/interactive_test.go | 37 +++++---- 16 files changed, 347 insertions(+), 171 deletions(-) diff --git a/cmd/api_keys.go b/cmd/api_keys.go index c6b78baf..24d03d7e 100644 --- a/cmd/api_keys.go +++ b/cmd/api_keys.go @@ -22,7 +22,8 @@ type APIKeysService interface { } type APIKeysCmd struct { - apiKeys APIKeysService + apiKeys APIKeysService + prompter interactive.Prompter } type APIKeysCreateInput struct { @@ -182,7 +183,7 @@ func (c APIKeysCmd) Update(ctx context.Context, in APIKeysUpdateInput) error { func (c APIKeysCmd) Delete(ctx context.Context, in APIKeysDeleteInput) error { if !in.SkipConfirm { - ok, err := interactive.Confirm( + ok, err := c.prompter.Confirm( fmt.Sprintf("delete API key '%s'", in.ID), fmt.Sprintf("Are you sure you want to delete API key '%s'?", in.ID), ) diff --git a/cmd/auth_connections.go b/cmd/auth_connections.go index c0780a75..6dfa0ec5 100644 --- a/cmd/auth_connections.go +++ b/cmd/auth_connections.go @@ -30,7 +30,8 @@ type AuthConnectionService interface { // AuthConnectionCmd handles auth connection operations independent of cobra. type AuthConnectionCmd struct { - svc AuthConnectionService + svc AuthConnectionService + prompter interactive.Prompter } type AuthConnectionCreateInput struct { @@ -492,7 +493,7 @@ func (c AuthConnectionCmd) List(ctx context.Context, in AuthConnectionListInput) func (c AuthConnectionCmd) Delete(ctx context.Context, in AuthConnectionDeleteInput) error { if !in.SkipConfirm { - ok, err := interactive.Confirm( + ok, err := c.prompter.Confirm( fmt.Sprintf("delete managed auth '%s'", in.ID), fmt.Sprintf("Are you sure you want to delete managed auth '%s'?", in.ID), ) diff --git a/cmd/create.go b/cmd/create.go index f82610da..aa7080be 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -15,7 +15,9 @@ import ( ) // CreateCmd is a cobra-independent command handler for create operations -type CreateCmd struct{} +type CreateCmd struct { + prompter interactive.Prompter +} // Create executes the creating a new Kernel app logic func (c CreateCmd) Create(ctx context.Context, ci create.CreateInput) error { @@ -29,7 +31,7 @@ func (c CreateCmd) Create(ctx context.Context, ci create.CreateInput) error { // confirmation before calling Create. if _, err := os.Stat(appPath); err == nil { if !ci.SkipConfirm { - overwrite, err := create.PromptOverwrite(ci.Name) + overwrite, err := create.PromptOverwrite(c.prompter, ci.Name) if err != nil { return err } @@ -148,66 +150,47 @@ func buildCreateLongHelp() string { } func runCreateApp(cmd *cobra.Command, args []string) error { - appName, _ := cmd.Flags().GetString("name") - language, _ := cmd.Flags().GetString("language") - template, _ := cmd.Flags().GetString("template") - skipConfirm, _ := cmd.Flags().GetBool("yes") + return createApp(cmd, interactive.NewPrompter()) +} + +// createApp resolves the create inputs and scaffolds the app. The prompter +// carries the terminal capability, so tests can drive the non-interactive +// path deterministically without mutating any package state. +func createApp(cmd *cobra.Command, prompter interactive.Prompter) error { + raw := create.RawInput{} + raw.Name, _ = cmd.Flags().GetString("name") + raw.Language, _ = cmd.Flags().GetString("language") + raw.Template, _ = cmd.Flags().GetString("template") + raw.SkipOverwriteConfirm, _ = cmd.Flags().GetBool("yes") - c := CreateCmd{} + c := CreateCmd{prompter: prompter} // create.ResolveInput is the single resolver from raw flags to a // normalized CreateInput for both modes. Interactively, each remaining - // problem is resolved by prompting for that field and re-resolving (so - // e.g. the template list reflects the chosen language). Non-interactively, - // every problem is reported at once in a single fail-fast error. + // problem carries its own resolution step: prompt for that field, apply + // the answer, re-resolve (so e.g. the template list reflects the chosen + // language). Non-interactively, every problem is reported at once in a + // single fail-fast error. for { - in, problems := create.ResolveInput(appName, language, template, skipConfirm) + in, problems := create.ResolveInput(raw) if len(problems) == 0 { return c.Create(cmd.Context(), in) } - if !interactive.IsInteractive() { + if !prompter.CanPrompt() { return interactive.ErrInputsRequired(create.ProblemMessages(problems)) } p := problems[0] - switch p.Field { - case create.FieldName: - if appName != "" { - pterm.Warning.Println(p.Message) - } - v, err := create.PromptName() - if err != nil { - return err - } - appName = v - case create.FieldLanguage: - if language != "" { - pterm.Warning.Println(p.Message) - } - v, err := create.PromptLanguage() - if err != nil { - return err - } - language = v - case create.FieldTemplate: - if template != "" { - pterm.Warning.Println(p.Message) - } - v, err := create.PromptTemplate(in.Language) - if err != nil { - return err - } - template = v - case create.FieldOverwrite: - ok, err := create.PromptOverwrite(appName) - if err != nil { - return err - } - if !ok { - pterm.Warning.Println("Operation cancelled.") - return nil - } - skipConfirm = true + if p.Invalid { + pterm.Warning.Println(p.Message) + } + cancelled, err := p.Resolve(prompter, &raw) + if err != nil { + return err + } + if cancelled { + pterm.Warning.Println("Operation cancelled.") + return nil } } } diff --git a/cmd/credential_providers.go b/cmd/credential_providers.go index d599606f..2f472348 100644 --- a/cmd/credential_providers.go +++ b/cmd/credential_providers.go @@ -28,6 +28,7 @@ type CredentialProvidersService interface { // CredentialProvidersCmd handles credential provider operations independent of cobra. type CredentialProvidersCmd struct { providers CredentialProvidersService + prompter interactive.Prompter } type CredentialProvidersListInput struct { @@ -255,7 +256,7 @@ func (c CredentialProvidersCmd) Update(ctx context.Context, in CredentialProvide func (c CredentialProvidersCmd) Delete(ctx context.Context, in CredentialProvidersDeleteInput) error { if !in.SkipConfirm { - ok, err := interactive.Confirm( + ok, err := c.prompter.Confirm( fmt.Sprintf("delete credential provider '%s'", in.ID), fmt.Sprintf("Are you sure you want to delete credential provider '%s'?", in.ID), ) diff --git a/cmd/credentials.go b/cmd/credentials.go index 61d35165..8a58370e 100644 --- a/cmd/credentials.go +++ b/cmd/credentials.go @@ -27,6 +27,7 @@ type CredentialsService interface { // CredentialsCmd handles credential operations independent of cobra. type CredentialsCmd struct { credentials CredentialsService + prompter interactive.Prompter } type CredentialsListInput struct { @@ -282,7 +283,7 @@ func (c CredentialsCmd) Update(ctx context.Context, in CredentialsUpdateInput) e func (c CredentialsCmd) Delete(ctx context.Context, in CredentialsDeleteInput) error { if !in.SkipConfirm { - ok, err := interactive.Confirm( + ok, err := c.prompter.Confirm( fmt.Sprintf("delete credential '%s'", in.Identifier), fmt.Sprintf("Are you sure you want to delete credential '%s'?", in.Identifier), ) diff --git a/cmd/extensions.go b/cmd/extensions.go index 34bc02af..80f15d74 100644 --- a/cmd/extensions.go +++ b/cmd/extensions.go @@ -89,6 +89,7 @@ type ExtensionsUploadInput struct { // ExtensionsCmd handles extension operations independent of cobra. type ExtensionsCmd struct { extensions ExtensionsService + prompter interactive.Prompter } func (e ExtensionsCmd) List(ctx context.Context, in ExtensionsListInput) error { @@ -185,7 +186,7 @@ func (e ExtensionsCmd) Delete(ctx context.Context, in ExtensionsDeleteInput) err } if !in.SkipConfirm { - ok, err := interactive.Confirm( + ok, err := e.prompter.Confirm( fmt.Sprintf("delete extension '%s'", in.Identifier), fmt.Sprintf("Are you sure you want to delete extension '%s'?", in.Identifier), ) diff --git a/cmd/noninteractive_test.go b/cmd/noninteractive_test.go index a7419a07..ed40df29 100644 --- a/cmd/noninteractive_test.go +++ b/cmd/noninteractive_test.go @@ -28,19 +28,13 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -// forceNonInteractive arranges the condition under test explicitly instead of -// relying on the harness's ambient stdin (which may be a PTY). -func forceNonInteractive(t *testing.T) { - t.Helper() - t.Cleanup(interactive.ForceTerminal(false)) -} - // Any command path that would show an interactive confirmation must fail fast // with a --yes hint instead of prompting (which would otherwise hang forever -// in agent/CI shells). +// in agent/CI shells). The terminal capability is carried by the injected +// Prompter, so these tests do not depend on the harness's ambient stdin and +// mutate no package state. func TestAPIKeysDeleteFailsFastWhenNonInteractive(t *testing.T) { - forceNonInteractive(t) _ = capturePtermOutput(t) fake := &FakeAPIKeysService{ DeleteFunc: func(ctx context.Context, id string, opts ...option.RequestOption) error { @@ -48,7 +42,7 @@ func TestAPIKeysDeleteFailsFastWhenNonInteractive(t *testing.T) { return nil }, } - c := APIKeysCmd{apiKeys: fake} + c := APIKeysCmd{apiKeys: fake, prompter: interactive.NewPrompterWithTerminal(false)} err := c.Delete(context.Background(), APIKeysDeleteInput{ID: "key_123"}) require.Error(t, err) @@ -58,7 +52,6 @@ func TestAPIKeysDeleteFailsFastWhenNonInteractive(t *testing.T) { } func TestExtensionsDeleteFailsFastWhenNonInteractive(t *testing.T) { - forceNonInteractive(t) _ = capturePtermOutput(t) fake := &FakeExtensionsService{ DeleteFunc: func(ctx context.Context, idOrName string, opts ...option.RequestOption) error { @@ -66,7 +59,7 @@ func TestExtensionsDeleteFailsFastWhenNonInteractive(t *testing.T) { return nil }, } - e := ExtensionsCmd{extensions: fake} + e := ExtensionsCmd{extensions: fake, prompter: interactive.NewPrompterWithTerminal(false)} err := e.Delete(context.Background(), ExtensionsDeleteInput{Identifier: "e1"}) require.Error(t, err) @@ -77,7 +70,7 @@ func TestExtensionsDeleteFailsFastWhenNonInteractive(t *testing.T) { // In a non-interactive shell, `kernel create` must report every missing or // invalid input in a single error so one retry can fix everything. func TestCreateFailsFastAggregatedWhenNonInteractive(t *testing.T) { - forceNonInteractive(t) + nonInteractive := interactive.NewPrompterWithTerminal(false) newCreateCmd := func(flags map[string]string) *cobra.Command { cmd := &cobra.Command{} @@ -92,7 +85,7 @@ func TestCreateFailsFastAggregatedWhenNonInteractive(t *testing.T) { } t.Run("no flags reports all three inputs in one error", func(t *testing.T) { - err := runCreateApp(newCreateCmd(nil), nil) + err := createApp(newCreateCmd(nil), nonInteractive) require.Error(t, err) assert.Contains(t, err.Error(), "--name is required") assert.Contains(t, err.Error(), "--language is required") @@ -101,7 +94,7 @@ func TestCreateFailsFastAggregatedWhenNonInteractive(t *testing.T) { }) t.Run("mixed missing and invalid flags aggregated", func(t *testing.T) { - err := runCreateApp(newCreateCmd(map[string]string{"language": "ruby", "template": "nope"}), nil) + err := createApp(newCreateCmd(map[string]string{"language": "ruby", "template": "nope"}), nonInteractive) require.Error(t, err) assert.Contains(t, err.Error(), "--name is required") assert.Contains(t, err.Error(), "--language 'ruby' is invalid") @@ -109,7 +102,7 @@ func TestCreateFailsFastAggregatedWhenNonInteractive(t *testing.T) { }) t.Run("single problem stays a single-line error", func(t *testing.T) { - err := runCreateApp(newCreateCmd(map[string]string{"name": "my-app", "language": "typescript", "template": "nope"}), nil) + err := createApp(newCreateCmd(map[string]string{"name": "my-app", "language": "typescript", "template": "nope"}), nonInteractive) require.Error(t, err) assert.Contains(t, err.Error(), "--template 'nope' is invalid for language 'typescript'") assert.NotContains(t, err.Error(), "\n") diff --git a/cmd/profiles.go b/cmd/profiles.go index c3797bbd..b4b86437 100644 --- a/cmd/profiles.go +++ b/cmd/profiles.go @@ -61,6 +61,7 @@ type ProfilesDownloadInput struct { // ProfilesCmd handles profile operations independent of cobra. type ProfilesCmd struct { profiles ProfilesService + prompter interactive.Prompter } func (p ProfilesCmd) List(ctx context.Context, in ProfilesListInput) error { @@ -228,7 +229,7 @@ func (p ProfilesCmd) Delete(ctx context.Context, in ProfilesDeleteInput) error { } if !in.SkipConfirm { - ok, err := interactive.Confirm( + ok, err := p.prompter.Confirm( fmt.Sprintf("delete profile '%s'", in.Identifier), fmt.Sprintf("Are you sure you want to delete profile '%s'?", in.Identifier), ) diff --git a/cmd/proxies/delete.go b/cmd/proxies/delete.go index d9e178ec..45d6fe96 100644 --- a/cmd/proxies/delete.go +++ b/cmd/proxies/delete.go @@ -12,7 +12,7 @@ import ( func (p ProxyCmd) Delete(ctx context.Context, in ProxyDeleteInput) error { if !in.SkipConfirm { - if !interactive.IsInteractive() { + if !p.prompter.CanPrompt() { // Fail fast before the proxy lookup below; the lookup only // improves the prompt text. return interactive.ErrConfirmationRequired(fmt.Sprintf("delete proxy '%s'", in.ID)) @@ -34,7 +34,7 @@ func (p ProxyCmd) Delete(ctx context.Context, in ProxyDeleteInput) error { confirmMsg = fmt.Sprintf("Are you sure you want to delete proxy '%s'?", in.ID) } - result, err := interactive.Confirm(fmt.Sprintf("delete proxy '%s'", in.ID), confirmMsg) + result, err := p.prompter.Confirm(fmt.Sprintf("delete proxy '%s'", in.ID), confirmMsg) if err != nil { return err } diff --git a/cmd/proxies/types.go b/cmd/proxies/types.go index 366f149c..5cc7dbd9 100644 --- a/cmd/proxies/types.go +++ b/cmd/proxies/types.go @@ -3,6 +3,7 @@ package proxies import ( "context" + "github.com/kernel/cli/pkg/interactive" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" "github.com/kernel/kernel-go-sdk/packages/pagination" @@ -19,7 +20,8 @@ type ProxyService interface { // ProxyCmd handles proxy operations independent of cobra. type ProxyCmd struct { - proxies ProxyService + proxies ProxyService + prompter interactive.Prompter } // Input types for proxy operations diff --git a/pkg/create/prompts.go b/pkg/create/prompts.go index f8a17991..f72d6011 100644 --- a/pkg/create/prompts.go +++ b/pkg/create/prompts.go @@ -13,8 +13,8 @@ import ( // PromptName prompts for the app name. An empty entry falls back to // DefaultAppName. -func PromptName() (string, error) { - name, err := interactive.TextInput( +func PromptName(p interactive.Prompter) (string, error) { + name, err := p.TextInput( "app name", "pass --name to set the app name (e.g. --name "+DefaultAppName+")", fmt.Sprintf("%s (%s)", AppNamePrompt, DefaultAppName), @@ -29,8 +29,8 @@ func PromptName() (string, error) { } // PromptLanguage prompts for the application language. -func PromptLanguage() (string, error) { - return interactive.Select( +func PromptLanguage(p interactive.Prompter) (string, error) { + return p.Select( "language selection", "pass --language with one of: "+languageOptionsHint(), LanguagePrompt, @@ -40,9 +40,9 @@ func PromptLanguage() (string, error) { // PromptTemplate prompts for a template supported by the given (normalized) // language. -func PromptTemplate(language string) (string, error) { +func PromptTemplate(p interactive.Prompter, language string) (string, error) { templateKVs := GetSupportedTemplatesForLanguage(language) - display, err := interactive.Select( + display, err := p.Select( "template selection", "pass --template with one of: "+templateOptionsHint(templateKVs), TemplatePrompt, @@ -56,8 +56,8 @@ func PromptTemplate(language string) (string, error) { // PromptOverwrite asks for confirmation before overwriting an existing // directory. -func PromptOverwrite(dirName string) (bool, error) { - return interactive.Confirm( +func PromptOverwrite(p interactive.Prompter, dirName string) (bool, error) { + return p.Confirm( fmt.Sprintf("overwrite existing directory '%s'", dirName), fmt.Sprintf("Directory %s already exists. Overwrite?", dirName), ) diff --git a/pkg/create/prompts_test.go b/pkg/create/prompts_test.go index 8f2136d0..4728c5d0 100644 --- a/pkg/create/prompts_test.go +++ b/pkg/create/prompts_test.go @@ -8,28 +8,30 @@ import ( "github.com/stretchr/testify/require" ) -// Every prompt wrapper must fail fast with a flag-usage hint when the shell -// is non-interactive. The terminal capability is injected explicitly so the -// tests do not depend on the harness's ambient stdin. +// Every prompt wrapper must fail fast with a flag-usage hint when its +// Prompter cannot prompt. The terminal capability is carried by the injected +// Prompter, so the tests do not depend on the harness's ambient stdin and +// mutate no package state. func TestPromptsFailFastWhenNonInteractive(t *testing.T) { - t.Cleanup(interactive.ForceTerminal(false)) + t.Parallel() + p := interactive.NewPrompterWithTerminal(false) - _, err := PromptName() + _, err := PromptName(p) require.Error(t, err) assert.Contains(t, err.Error(), "--name") assert.Contains(t, err.Error(), "not an interactive terminal") - _, err = PromptLanguage() + _, err = PromptLanguage(p) require.Error(t, err) assert.Contains(t, err.Error(), "--language") assert.Contains(t, err.Error(), LanguageTypeScript) - _, err = PromptTemplate(LanguageTypeScript) + _, err = PromptTemplate(p, LanguageTypeScript) require.Error(t, err) assert.Contains(t, err.Error(), "--template") assert.Contains(t, err.Error(), TemplateSampleApp) - ok, err := PromptOverwrite("existing-dir") + ok, err := PromptOverwrite(p, "existing-dir") require.Error(t, err) assert.False(t, ok) assert.Contains(t, err.Error(), "overwrite existing directory 'existing-dir'") diff --git a/pkg/create/resolve.go b/pkg/create/resolve.go index 4d82764d..ea760e50 100644 --- a/pkg/create/resolve.go +++ b/pkg/create/resolve.go @@ -6,6 +6,8 @@ import ( "regexp" "slices" "strings" + + "github.com/kernel/cli/pkg/interactive" ) // Field identifies a scaffolding input that a Problem refers to. @@ -18,6 +20,16 @@ const ( FieldOverwrite Field = "overwrite" ) +// RawInput carries the raw `kernel create` flag values before resolution. +type RawInput struct { + Name string + Language string + Template string + // SkipOverwriteConfirm is set by --yes, or after the user interactively + // confirms overwriting an existing directory. + SkipOverwriteConfirm bool +} + // Problem describes one input that is missing, invalid, or would require a // confirmation. Message is phrased as the non-interactive fix instruction // (naming the exact flag to pass); the interactive flow surfaces the same @@ -25,6 +37,25 @@ const ( type Problem struct { Field Field Message string + // Invalid marks that a value was provided but rejected (as opposed to + // missing); the interactive flow warns with Message before prompting. + Invalid bool + // resolve prompts for the field and applies the answer to raw. + // ResolveInput sets it on every Problem it emits, so the + // resolver/dispatcher contract is exhaustive by construction: a Problem + // cannot exist without its interactive resolution step. + resolve func(p interactive.Prompter, raw *RawInput) (cancelled bool, err error) +} + +// Resolve prompts for this problem's field via p and applies the answer to +// raw. cancelled reports that the user declined a confirmation and the +// operation should stop. +func (pr Problem) Resolve(p interactive.Prompter, raw *RawInput) (cancelled bool, err error) { + if pr.resolve == nil { + // Defensive: only reachable for a Problem not built by ResolveInput. + return false, fmt.Errorf("internal error: no interactive resolution step for input %q; pass the corresponding flag instead", pr.Field) + } + return pr.resolve(p, raw) } // ProblemMessages extracts the messages from problems, in order. @@ -82,13 +113,56 @@ func templateOptionsHint(templateKVs TemplateKeyValues) string { return strings.Join(keys, ", ") } +// Per-field interactive resolution steps. Each prompts for its field and +// applies the answer to raw; the caller re-resolves afterwards, so the +// template step can derive its options from the by-then-resolved language. +func resolveName(p interactive.Prompter, raw *RawInput) (bool, error) { + v, err := PromptName(p) + if err != nil { + return false, err + } + raw.Name = v + return false, nil +} + +func resolveLanguage(p interactive.Prompter, raw *RawInput) (bool, error) { + v, err := PromptLanguage(p) + if err != nil { + return false, err + } + raw.Language = v + return false, nil +} + +func resolveTemplate(p interactive.Prompter, raw *RawInput) (bool, error) { + v, err := PromptTemplate(p, NormalizeLanguage(raw.Language)) + if err != nil { + return false, err + } + raw.Template = v + return false, nil +} + +func resolveOverwrite(p interactive.Prompter, raw *RawInput) (bool, error) { + ok, err := PromptOverwrite(p, raw.Name) + if err != nil { + return false, err + } + if !ok { + return true, nil + } + raw.SkipOverwriteConfirm = true + return false, nil +} + // ResolveInput is the single canonical resolver from raw flag values to a // normalized CreateInput. It validates and normalizes every field, collecting // a Problem for each one that is missing, invalid, or (for an existing target -// directory) would require an overwrite confirmation. It never prompts. +// directory) would require an overwrite confirmation. It never prompts, but +// every Problem it emits carries the prompt step that resolves it. // // Both modes of `kernel create` consume its problem model: the interactive -// flow prompts to resolve one problem at a time and re-resolves, while the +// flow resolves one problem at a time and re-resolves, while the // non-interactive flow reports all problems in a single fail-fast error. // Problems are ordered name, language, template, overwrite so interactive // resolution fixes the language before the language-dependent template list @@ -96,62 +170,62 @@ func templateOptionsHint(templateKVs TemplateKeyValues) string { // // The returned CreateInput carries the fields that did resolve (with the // language normalized) even when problems remain. -func ResolveInput(name, language, template string, skipOverwriteConfirm bool) (CreateInput, []Problem) { +func ResolveInput(raw RawInput) (CreateInput, []Problem) { var problems []Problem // --name nameValid := false - if name == "" { - problems = append(problems, Problem{FieldName, "--name is required (e.g. --name " + DefaultAppName + ")"}) - } else if err := validateAppName(name); err != nil { - problems = append(problems, Problem{FieldName, fmt.Sprintf("--name '%s' is invalid: %v", name, err)}) + if raw.Name == "" { + problems = append(problems, Problem{FieldName, "--name is required (e.g. --name " + DefaultAppName + ")", false, resolveName}) + } else if err := validateAppName(raw.Name); err != nil { + problems = append(problems, Problem{FieldName, fmt.Sprintf("--name '%s' is invalid: %v", raw.Name, err), true, resolveName}) } else { nameValid = true } // --language lang := "" - if language == "" { - problems = append(problems, Problem{FieldLanguage, "--language is required: one of: " + languageOptionsHint()}) - } else if l := NormalizeLanguage(language); slices.Contains(SupportedLanguages, l) { + if raw.Language == "" { + problems = append(problems, Problem{FieldLanguage, "--language is required: one of: " + languageOptionsHint(), false, resolveLanguage}) + } else if l := NormalizeLanguage(raw.Language); slices.Contains(SupportedLanguages, l) { lang = l } else { - problems = append(problems, Problem{FieldLanguage, fmt.Sprintf("--language '%s' is invalid: must be one of: %s", language, languageOptionsHint())}) + problems = append(problems, Problem{FieldLanguage, fmt.Sprintf("--language '%s' is invalid: must be one of: %s", raw.Language, languageOptionsHint()), true, resolveLanguage}) } // --template (valid values depend on --language) templateValid := false if lang != "" { templateKVs := GetSupportedTemplatesForLanguage(lang) - if template == "" { - problems = append(problems, Problem{FieldTemplate, "--template is required: one of: " + templateOptionsHint(templateKVs)}) - } else if templateKVs.ContainsKey(template) { + if raw.Template == "" { + problems = append(problems, Problem{FieldTemplate, "--template is required: one of: " + templateOptionsHint(templateKVs), false, resolveTemplate}) + } else if templateKVs.ContainsKey(raw.Template) { templateValid = true } else { - problems = append(problems, Problem{FieldTemplate, fmt.Sprintf("--template '%s' is invalid for language '%s': must be one of: %s", template, lang, templateOptionsHint(templateKVs))}) + problems = append(problems, Problem{FieldTemplate, fmt.Sprintf("--template '%s' is invalid for language '%s': must be one of: %s", raw.Template, lang, templateOptionsHint(templateKVs)), true, resolveTemplate}) } - } else if template == "" { - problems = append(problems, Problem{FieldTemplate, "--template is required (run 'kernel create --help' for the full list)"}) - } else if _, ok := Templates[template]; !ok { + } else if raw.Template == "" { + problems = append(problems, Problem{FieldTemplate, "--template is required (run 'kernel create --help' for the full list)", false, resolveTemplate}) + } else if _, ok := Templates[raw.Template]; !ok { // Language is unknown, but the template doesn't exist for any // language, so report it now rather than on the next retry. - problems = append(problems, Problem{FieldTemplate, fmt.Sprintf("--template '%s' is invalid (run 'kernel create --help' for the full list)", template)}) + problems = append(problems, Problem{FieldTemplate, fmt.Sprintf("--template '%s' is invalid (run 'kernel create --help' for the full list)", raw.Template), true, resolveTemplate}) } // Overwriting the target directory would need a confirmation prompt. - if nameValid && !skipOverwriteConfirm { - if _, err := os.Stat(name); err == nil { - problems = append(problems, Problem{FieldOverwrite, fmt.Sprintf("directory '%s' already exists: pass --yes to overwrite it, or choose a different --name", name)}) + if nameValid && !raw.SkipOverwriteConfirm { + if _, err := os.Stat(raw.Name); err == nil { + problems = append(problems, Problem{FieldOverwrite, fmt.Sprintf("directory '%s' already exists: pass --yes to overwrite it, or choose a different --name", raw.Name), false, resolveOverwrite}) } } in := CreateInput{ - Name: name, + Name: raw.Name, Language: lang, - SkipConfirm: skipOverwriteConfirm, + SkipConfirm: raw.SkipOverwriteConfirm, } if templateValid { - in.Template = template + in.Template = raw.Template } return in, problems } diff --git a/pkg/create/resolve_test.go b/pkg/create/resolve_test.go index 31904f5e..df9de0a5 100644 --- a/pkg/create/resolve_test.go +++ b/pkg/create/resolve_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "testing" + "github.com/kernel/cli/pkg/interactive" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -21,7 +22,8 @@ func problemFields(problems []Problem) []Field { } func TestResolveInputReportsAllProblems(t *testing.T) { - _, problems := ResolveInput("", "", "", false) + t.Parallel() + _, problems := ResolveInput(RawInput{}) assert.Equal(t, []Field{FieldName, FieldLanguage, FieldTemplate}, problemFields(problems)) msgs := ProblemMessages(problems) @@ -31,17 +33,32 @@ func TestResolveInputReportsAllProblems(t *testing.T) { } func TestResolveInputInvalidValues(t *testing.T) { - _, problems := ResolveInput("bad name!", "ruby", "nope", false) + t.Parallel() + _, problems := ResolveInput(RawInput{Name: "bad name!", Language: "ruby", Template: "nope"}) require.Len(t, problems, 3) assert.Contains(t, problems[0].Message, "--name 'bad name!' is invalid") assert.Contains(t, problems[1].Message, "--language 'ruby' is invalid") // Language is unknown, but the template exists for no language at all, // so it must still be reported in the same pass. assert.Contains(t, problems[2].Message, "--template 'nope' is invalid") + // Provided-but-rejected values are marked Invalid so the interactive + // flow warns before re-prompting; missing values are not. + for _, p := range problems { + assert.True(t, p.Invalid, "problem %q should be marked Invalid", p.Field) + } +} + +func TestResolveInputMissingValuesAreNotInvalid(t *testing.T) { + t.Parallel() + _, problems := ResolveInput(RawInput{}) + for _, p := range problems { + assert.False(t, p.Invalid, "missing input %q should not be marked Invalid", p.Field) + } } func TestResolveInputTemplateValidatedPerLanguage(t *testing.T) { - in, problems := ResolveInput("my-app", "ts", "nope", false) + t.Parallel() + in, problems := ResolveInput(RawInput{Name: "my-app", Language: "ts", Template: "nope"}) require.Len(t, problems, 1) assert.Equal(t, FieldTemplate, problems[0].Field) assert.Contains(t, problems[0].Message, "--template 'nope' is invalid for language 'typescript'") @@ -57,23 +74,75 @@ func TestResolveInputExistingDirectory(t *testing.T) { t.Chdir(dir) require.NoError(t, os.MkdirAll(filepath.Join(dir, "my-app"), 0o755)) - _, problems := ResolveInput("my-app", "ts", TemplateSampleApp, false) + _, problems := ResolveInput(RawInput{Name: "my-app", Language: "ts", Template: TemplateSampleApp}) require.Len(t, problems, 1) assert.Equal(t, FieldOverwrite, problems[0].Field) assert.Contains(t, problems[0].Message, "directory 'my-app' already exists") assert.Contains(t, problems[0].Message, "--yes") // --yes skips the overwrite confirmation, so it is not a problem. - in, problems := ResolveInput("my-app", "ts", TemplateSampleApp, true) + in, problems := ResolveInput(RawInput{Name: "my-app", Language: "ts", Template: TemplateSampleApp, SkipOverwriteConfirm: true}) assert.Empty(t, problems) assert.True(t, in.SkipConfirm) } func TestResolveInputValid(t *testing.T) { - in, problems := ResolveInput("my-app", "py", TemplateSampleApp, false) + t.Parallel() + in, problems := ResolveInput(RawInput{Name: "my-app", Language: "py", Template: TemplateSampleApp}) assert.Empty(t, problems) assert.Equal(t, "my-app", in.Name) assert.Equal(t, LanguagePython, in.Language) assert.Equal(t, TemplateSampleApp, in.Template) assert.False(t, in.SkipConfirm) } + +// Every Problem emitted by ResolveInput must carry its interactive +// resolution step: a problem the dispatcher cannot resolve would otherwise +// loop forever, which is exactly the failure class this package removes. +func TestEveryProblemCarriesResolutionStep(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "my-app"), 0o755)) + + raws := []RawInput{ + {}, // all missing + {Name: "bad name!", Language: "ruby", Template: "nope"}, // all invalid + {Name: "my-app", Language: "ts", Template: TemplateSampleApp}, // overwrite + {Name: "my-app", Language: "ts", Template: "nope"}, // per-language template + } + seen := map[Field]bool{} + for _, raw := range raws { + _, problems := ResolveInput(raw) + for _, p := range problems { + seen[p.Field] = true + assert.NotNil(t, p.resolve, "problem %q must carry a resolution step", p.Field) + } + } + // All fields exercised. + assert.Equal(t, map[Field]bool{FieldName: true, FieldLanguage: true, FieldTemplate: true, FieldOverwrite: true}, seen) +} + +// A Problem constructed without a resolution step (i.e. not by ResolveInput) +// must fail with an internal error naming the field rather than spin. +func TestProblemWithoutResolutionStepErrors(t *testing.T) { + t.Parallel() + var raw RawInput + cancelled, err := Problem{Field: "future-field"}.Resolve(interactive.NewPrompterWithTerminal(true), &raw) + require.Error(t, err) + assert.False(t, cancelled) + assert.Contains(t, err.Error(), "internal error") + assert.Contains(t, err.Error(), "future-field") +} + +// Resolution steps must fail fast through the injected Prompter in a +// non-interactive shell (they are also unreachable in practice because the +// dispatcher checks CanPrompt first). +func TestProblemResolutionFailsFastWhenNonInteractive(t *testing.T) { + t.Parallel() + raw := RawInput{} + _, problems := ResolveInput(raw) + require.NotEmpty(t, problems) + _, err := problems[0].Resolve(interactive.NewPrompterWithTerminal(false), &raw) + require.Error(t, err) + assert.Contains(t, err.Error(), "not an interactive terminal") +} diff --git a/pkg/interactive/interactive.go b/pkg/interactive/interactive.go index 6cf9040b..aaa96c14 100644 --- a/pkg/interactive/interactive.go +++ b/pkg/interactive/interactive.go @@ -5,9 +5,10 @@ // Interactive prompts read keystrokes from the terminal. In a non-interactive // shell — an AI agent's bash tool, CI, or any piped stdin — they never return // (the underlying keyboard listener spins forever), so prompt execution is -// centralized here behind the TTY gate. Command code must call Confirm, -// Select, and TextInput instead of pterm's interactive printers; no direct -// pterm interactive calls should exist outside this package. +// centralized here behind the TTY gate. Command code must prompt through a +// Prompter (or the package-level helpers backed by the default Prompter) +// instead of pterm's interactive printers; no direct pterm interactive calls +// should exist outside this package. package interactive import ( @@ -19,26 +20,49 @@ import ( "golang.org/x/term" ) -// terminalCheck reports whether the given file is attached to a terminal. It -// is a package variable so tests can inject a fake terminal capability -// instead of relying on the ambient stdin of the test harness. -var terminalCheck = func(f *os.File) bool { +// isTerminal reports whether the given file is attached to a terminal. +func isTerminal(f *os.File) bool { return term.IsTerminal(int(f.Fd())) } -// IsInteractive reports whether stdin is attached to a terminal, i.e. whether -// interactive prompts can be shown. -func IsInteractive() bool { - return terminalCheck(os.Stdin) +// Prompter executes interactive prompts against a terminal capability. Each +// value owns its capability — there is no package-level mutable state — so +// tests can construct a Prompter with a fixed capability without affecting +// other goroutines or invocations. +// +// The zero value is equivalent to NewPrompter(): it detects the terminal +// from stdin at prompt time. +type Prompter struct { + // isTerminal overrides terminal detection when non-nil. + isTerminal func() bool +} + +// NewPrompter returns a Prompter that detects terminal capability from +// stdin at prompt time. +func NewPrompter() Prompter { + return Prompter{} } -// ForceTerminal overrides terminal detection for tests, making IsInteractive -// report isTTY regardless of the ambient stdin. It returns a function that -// restores the previous behavior; callers should register it with t.Cleanup. -func ForceTerminal(isTTY bool) (restore func()) { - prev := terminalCheck - terminalCheck = func(*os.File) bool { return isTTY } - return func() { terminalCheck = prev } +// NewPrompterWithTerminal returns a Prompter with a fixed terminal +// capability. Tests use NewPrompterWithTerminal(false) to exercise the +// fail-fast paths deterministically, regardless of the harness's stdin. +func NewPrompterWithTerminal(isTTY bool) Prompter { + return Prompter{isTerminal: func() bool { return isTTY }} +} + +// CanPrompt reports whether this Prompter can show interactive prompts. +func (p Prompter) CanPrompt() bool { + if p.isTerminal != nil { + return p.isTerminal() + } + return isTerminal(os.Stdin) +} + +// IsInteractive reports whether stdin is attached to a terminal, i.e. +// whether interactive prompts can be shown. Equivalent to +// NewPrompter().CanPrompt(). +func IsInteractive() bool { + return NewPrompter().CanPrompt() } // PromptError is returned instead of showing an interactive prompt when @@ -122,10 +146,10 @@ func ErrInputsRequired(problems []string) error { } // Confirm shows a yes/no confirmation prompt with the given prompt text and -// reports the choice. In a non-interactive shell it fails fast with -// ErrConfirmationRequired(action) instead of prompting. -func Confirm(action, promptText string) (bool, error) { - if !IsInteractive() { +// reports the choice. When the Prompter cannot prompt it fails fast with +// ErrConfirmationRequired(action) instead. +func (p Prompter) Confirm(action, promptText string) (bool, error) { + if !p.CanPrompt() { return false, ErrConfirmationRequired(action) } return pterm.DefaultInteractiveConfirm. @@ -135,10 +159,10 @@ func Confirm(action, promptText string) (bool, error) { } // Select shows a select prompt over options and returns the chosen option. -// In a non-interactive shell it fails fast with ErrInputRequired(what, hint) -// instead of prompting. -func Select(what, hint, promptText string, options []string) (string, error) { - if !IsInteractive() { +// When the Prompter cannot prompt it fails fast with +// ErrInputRequired(what, hint) instead. +func (p Prompter) Select(what, hint, promptText string, options []string) (string, error) { + if !p.CanPrompt() { return "", ErrInputRequired(what, hint) } return pterm.DefaultInteractiveSelect. @@ -148,14 +172,32 @@ func Select(what, hint, promptText string, options []string) (string, error) { Show() } -// TextInput shows a free-text prompt and returns the entered text. In a -// non-interactive shell it fails fast with ErrInputRequired(what, hint) -// instead of prompting. -func TextInput(what, hint, promptText string) (string, error) { - if !IsInteractive() { +// TextInput shows a free-text prompt and returns the entered text. When the +// Prompter cannot prompt it fails fast with ErrInputRequired(what, hint) +// instead. +func (p Prompter) TextInput(what, hint, promptText string) (string, error) { + if !p.CanPrompt() { return "", ErrInputRequired(what, hint) } return pterm.DefaultInteractiveTextInput. WithDefaultText(promptText). Show() } + +// Confirm is Prompter.Confirm on the default (ambient-stdin) Prompter, for +// command code without an injected Prompter. +func Confirm(action, promptText string) (bool, error) { + return NewPrompter().Confirm(action, promptText) +} + +// Select is Prompter.Select on the default (ambient-stdin) Prompter, for +// command code without an injected Prompter. +func Select(what, hint, promptText string, options []string) (string, error) { + return NewPrompter().Select(what, hint, promptText, options) +} + +// TextInput is Prompter.TextInput on the default (ambient-stdin) Prompter, +// for command code without an injected Prompter. +func TextInput(what, hint, promptText string) (string, error) { + return NewPrompter().TextInput(what, hint, promptText) +} diff --git a/pkg/interactive/interactive_test.go b/pkg/interactive/interactive_test.go index 599ba564..6785d1a1 100644 --- a/pkg/interactive/interactive_test.go +++ b/pkg/interactive/interactive_test.go @@ -11,26 +11,27 @@ import ( // The default detector must report false for a file descriptor that is // explicitly not a terminal (a pipe), independent of the test harness's // ambient stdin. -func TestTerminalCheckReportsFalseForPipe(t *testing.T) { +func TestIsTerminalReportsFalseForPipe(t *testing.T) { + t.Parallel() r, w, err := os.Pipe() require.NoError(t, err) defer r.Close() defer w.Close() - assert.False(t, terminalCheck(r)) - assert.False(t, terminalCheck(w)) + assert.False(t, isTerminal(r)) + assert.False(t, isTerminal(w)) } -func TestForceTerminalOverridesDetection(t *testing.T) { - restore := ForceTerminal(true) - assert.True(t, IsInteractive()) - restore() - - t.Cleanup(ForceTerminal(false)) - assert.False(t, IsInteractive()) +// Each Prompter owns its terminal capability; constructing one never touches +// package state, so opposite capabilities coexist across parallel tests. +func TestPrompterCarriesItsOwnTerminalCapability(t *testing.T) { + t.Parallel() + assert.True(t, NewPrompterWithTerminal(true).CanPrompt()) + assert.False(t, NewPrompterWithTerminal(false).CanPrompt()) } func TestPromptErrorSingleProblem(t *testing.T) { + t.Parallel() err := ErrConfirmationRequired("delete profile 'foo'") var promptErr *PromptError @@ -44,6 +45,7 @@ func TestPromptErrorSingleProblem(t *testing.T) { } func TestPromptErrorMultiProblemRendering(t *testing.T) { + t.Parallel() err := ErrInputsRequired([]string{ "--name is required", "--language is required: one of: typescript (ts), python (py)", @@ -69,33 +71,36 @@ func TestPromptErrorMultiProblemRendering(t *testing.T) { } func TestErrInputsRequiredEmptyIsNil(t *testing.T) { + t.Parallel() assert.NoError(t, ErrInputsRequired(nil)) } func TestErrInputRequired(t *testing.T) { + t.Parallel() err := ErrInputRequired("app name", "pass --name to set the app name") assert.Contains(t, err.Error(), "app name") assert.Contains(t, err.Error(), "pass --name") assert.Contains(t, err.Error(), "not an interactive terminal") } -// The prompt primitives must fail fast (never touch pterm) when the shell is -// non-interactive. +// The prompt primitives must fail fast (never touch pterm) when the Prompter +// cannot prompt. func TestPromptPrimitivesFailFastWhenNonInteractive(t *testing.T) { - t.Cleanup(ForceTerminal(false)) + t.Parallel() + p := NewPrompterWithTerminal(false) - ok, err := Confirm("delete widget 'w'", "Are you sure?") + ok, err := p.Confirm("delete widget 'w'", "Are you sure?") require.Error(t, err) assert.False(t, ok) assert.Contains(t, err.Error(), "delete widget 'w'") assert.Contains(t, err.Error(), "--yes") - _, err = Select("widget selection", "pass --widget", "Pick a widget:", []string{"a", "b"}) + _, err = p.Select("widget selection", "pass --widget", "Pick a widget:", []string{"a", "b"}) require.Error(t, err) assert.Contains(t, err.Error(), "widget selection") assert.Contains(t, err.Error(), "pass --widget") - _, err = TextInput("widget name", "pass --name", "Name?") + _, err = p.TextInput("widget name", "pass --name", "Name?") require.Error(t, err) assert.Contains(t, err.Error(), "widget name") assert.Contains(t, err.Error(), "pass --name")