Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions pkg/cli/engine_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ import (

var engineSecretsLog = logger.New("cli:engine_secrets")

// promptCancelled handles graceful cancellation of an interactive prompt.
// It prints "Cancelled." to stderr and returns an ExitCodeError with code 130.
func promptCancelled() error {
fmt.Fprintln(os.Stderr, "Cancelled.")
return &ExitCodeError{Code: 130}
}

// SecretRequirement represents a unified secret requirement for agentic workflows.
// This type unifies the legacy tokenSpec and EngineOption secret information.
type SecretRequirement struct {
Expand Down Expand Up @@ -322,6 +329,9 @@ func promptForCopilotPATUnified(req SecretRequirement, config EngineSecretConfig
)

if err := form.RunWithContext(config.ctx()); err != nil {
if console.IsCancelled(err) {
return promptCancelled()
}
return fmt.Errorf("failed to get Copilot token: %w", err)
}

Expand Down Expand Up @@ -366,6 +376,9 @@ func promptForSystemTokenUnified(req SecretRequirement, config EngineSecretConfi
)

if err := form.RunWithContext(config.ctx()); err != nil {
if console.IsCancelled(err) {
return promptCancelled()
}
return fmt.Errorf("failed to get %s token: %w", req.Name, err)
}

Expand Down Expand Up @@ -415,6 +428,9 @@ func promptForGenericAPIKeyUnified(req SecretRequirement, config EngineSecretCon
)

if err := form.RunWithContext(config.ctx()); err != nil {
if console.IsCancelled(err) {
return promptCancelled()
}
return fmt.Errorf("failed to get %s API key: %w", label, err)
}

Expand Down
6 changes: 5 additions & 1 deletion pkg/cli/run_interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,11 @@ func confirmExecution(ctx context.Context, wf *WorkflowOption, inputs []string)
)

if err := form.RunWithContext(ctx); err != nil {
runInteractiveLog.Printf("Confirmation failed: %v", err)
if console.IsCancelled(err) {
runInteractiveLog.Print("User aborted confirmation")
} else {
runInteractiveLog.Printf("Confirmation failed: %v", err)
}
return false
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] confirmExecution still returns false on Ctrl-C — the exit-code-130 signal isn't propagated here, unlike the three secret-prompt call sites. A caller that treats false as 'user said no' will exit 1, not 130.

💡 Suggested fix

Consider changing the signature to confirmExecution(...) (bool, error) and returning an error on abort so the caller can propagate ExitCodeError{130}:

if console.IsCancelled(err) {
    runInteractiveLog.Print("User aborted confirmation")
    return false, promptCancelled()
}

This makes cancellation handling consistent across all interactive prompts.

@copilot please address this.

Expand Down
9 changes: 9 additions & 0 deletions pkg/console/prompt_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
package console

import (
"errors"

"charm.land/huh/v2"
"github.com/github/gh-aw/pkg/styles"
)
Expand All @@ -26,3 +28,10 @@ func NewSelectForm[T comparable](selectField *huh.Select[T]) *huh.Form {
func NewConfirmForm(confirm *huh.Confirm) *huh.Form {
return NewForm(huh.NewGroup(confirm))
}

// IsCancelled reports whether err represents a deliberate user cancellation
// (Ctrl-C / Esc before form submission, i.e. huh.ErrUserAborted).
// Use this to distinguish graceful cancellation from genuine failures.
func IsCancelled(err error) bool {
return errors.Is(err, huh.ErrUserAborted)
}
21 changes: 21 additions & 0 deletions pkg/console/prompt_form_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
package console

import (
"errors"
"fmt"
"testing"

"charm.land/huh/v2"
Expand All @@ -21,3 +23,22 @@ func TestPromptWrappersReturnNonNilForms(t *testing.T) {
var confirmValue bool
require.NotNil(t, NewConfirmForm(huh.NewConfirm().Value(&confirmValue)))
}

func TestIsCancelled(t *testing.T) {
t.Run("returns true for huh.ErrUserAborted", func(t *testing.T) {
require.True(t, IsCancelled(huh.ErrUserAborted))
})

t.Run("returns true for wrapped huh.ErrUserAborted", func(t *testing.T) {
wrapped := fmt.Errorf("outer: %w", huh.ErrUserAborted)
require.True(t, IsCancelled(wrapped))
})

t.Run("returns false for other errors", func(t *testing.T) {
require.False(t, IsCancelled(errors.New("some other error")))
})

t.Run("returns false for nil", func(t *testing.T) {
require.False(t, IsCancelled(nil))
})
}
Loading