diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index 8b8084a1f..9c9b8ed68 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -70,6 +70,9 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co // Populate ProblemStatement from arg if len(args) > 0 { opts.ProblemStatement = args[0] + if strings.TrimSpace(opts.ProblemStatement) == "" { + return cmdutil.FlagErrorf("task description cannot be empty") + } } else if opts.ProblemStatementFile == "" && !opts.IO.CanPrompt() { return cmdutil.FlagErrorf("a task description or -F is required when running non-interactively") } @@ -121,36 +124,30 @@ func createRun(opts *CreateOptions) error { } if opts.ProblemStatement == "" { - // Load initial problem statement from file, if provided if opts.ProblemStatementFile != "" { fileContent, err := cmdutil.ReadFile(opts.ProblemStatementFile, opts.IO.In) if err != nil { - return cmdutil.FlagErrorf("could not read task description file: %v", err) + return fmt.Errorf("could not read task description file: %w", err) } - opts.ProblemStatement = strings.TrimSpace(string(fileContent)) - } - if opts.IO.CanPrompt() { + trimmed := strings.TrimSpace(string(fileContent)) + if trimmed == "" { + return errors.New("task description file cannot be empty") + } + + opts.ProblemStatement = trimmed + } else { desc, err := opts.Prompter.MarkdownEditor("Enter the task description", opts.ProblemStatement, false) if err != nil { return err } - opts.ProblemStatement = strings.TrimSpace(desc) - } - } - if opts.ProblemStatement == "" { - fmt.Fprintf(opts.IO.ErrOut, "a task description is required.\n") - return cmdutil.SilentError - } + trimmed := strings.TrimSpace(string(desc)) + if trimmed == "" { + return errors.New("a task description is required") + } - if opts.IO.CanPrompt() { - confirm, err := opts.Prompter.Confirm("Submit agent task", true) - if err != nil { - return err - } - if !confirm { - return cmdutil.SilentError + opts.ProblemStatement = trimmed } } @@ -168,6 +165,10 @@ func createRun(opts *CreateOptions) error { return err } + if opts.Follow { + return followLogs(opts, client, job.SessionID) + } + sessionURL, err := fetchJobSessionURL(ctx, client, repo, job, opts.BackOff) opts.IO.StopProgressIndicator() @@ -183,9 +184,6 @@ func createRun(opts *CreateOptions) error { fmt.Fprintf(opts.IO.Out, "job %s queued. View progress: %s\n", job.ID, capi.AgentsHomeURL) } - if opts.Follow { - return followLogs(opts, client, job.SessionID) - } return nil } @@ -256,8 +254,13 @@ func fetchJobWithBackoff(ctx context.Context, client capi.CapiClient, repo ghrep } func followLogs(opts *CreateOptions, capiClient capi.CapiClient, sessionID string) error { - ctx := context.Background() + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "error starting pager: %v\n", err) + } + ctx := context.Background() renderer := opts.LogRenderer() var called bool @@ -273,6 +276,5 @@ func followLogs(opts *CreateOptions, capiClient capi.CapiClient, sessionID strin return raw, nil } - fmt.Fprintln(opts.IO.Out, "") return renderer.Follow(fetcher, opts.IO.Out, opts.IO) } diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index d1298e606..ef7f529ac 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -46,6 +46,21 @@ func TestNewCmdCreate(t *testing.T) { ProblemStatementFile: "", }, }, + { + name: "empty arg", + args: "''", + wantErr: "task description cannot be empty", + }, + { + name: "whitespace arg", + args: "' '", + wantErr: "task description cannot be empty", + }, + { + name: "whitespace and newline arg", + args: "'\n'", + wantErr: "task description cannot be empty", + }, { name: "mutually exclusive arg and file", args: "'some task inline' -F foo.md", @@ -96,7 +111,7 @@ func TestNewCmdCreate(t *testing.T) { _, err = cmd.ExecuteC() if tt.wantErr != "" { - require.Error(t, err, tt.wantErr) + require.EqualError(t, err, tt.wantErr) } else { require.NoError(t, err) } @@ -158,101 +173,41 @@ func Test_createRun(t *testing.T) { wantErrIs error }{ { - name: "interactive with file prompts to edit with file contents", - opts: &CreateOptions{ - BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - ProblemStatement: "", - ProblemStatementFile: taskDescFile, - Prompter: &prompter.PrompterMock{ - MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) { - require.Equal(t, "Enter the task description", prompt) - require.Equal(t, "task description from file", defaultValue) - return "edited task description", nil - }, - ConfirmFunc: func(message string, defaultValue bool) (bool, error) { - require.Equal(t, "Submit agent task", message) - return true, nil - }, - }, - }, + name: "interactive, problem statement from arg", isTTY: true, + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + ProblemStatement: "task description from arg", + }, capiStubs: func(t *testing.T, m *capi.CapiClientMock) { m.CreateJobFunc = func(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*capi.Job, error) { require.Equal(t, "OWNER", owner) require.Equal(t, "REPO", repo) - require.Equal(t, "edited task description", problemStatement) + require.Equal(t, "task description from arg", problemStatement) return &createdJobSuccessWithPR, nil } }, wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", }, { - name: "interactively rejecting confirmation prompt aborts task creation", + name: "non-interactive, problem statement from arg", opts: &CreateOptions{ BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - ProblemStatement: "", - Prompter: &prompter.PrompterMock{ - MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) { - require.Equal(t, "Enter the task description", prompt) - return "From editor", nil - }, - ConfirmFunc: func(message string, defaultValue bool) (bool, error) { - require.Equal(t, "Submit agent task", message) - return false, nil - }, - }, - }, - isTTY: true, - wantErr: "SilentError", - wantErrIs: cmdutil.SilentError, - wantStdErr: "", - }, - { - name: "interactively entering task description with editor, no file", - isTTY: true, - opts: &CreateOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - ProblemStatement: "", - Prompter: &prompter.PrompterMock{ - MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) { - require.Equal(t, "Enter the task description", prompt) - return "From editor", nil - }, - ConfirmFunc: func(message string, defaultValue bool) (bool, error) { - require.Equal(t, "Submit agent task", message) - return true, nil - }, - }, + ProblemStatement: "task description from arg", }, capiStubs: func(t *testing.T, m *capi.CapiClientMock) { m.CreateJobFunc = func(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*capi.Job, error) { - require.Equal(t, "From editor", problemStatement) + require.Equal(t, "OWNER", owner) + require.Equal(t, "REPO", repo) + require.Equal(t, "task description from arg", problemStatement) return &createdJobSuccessWithPR, nil } }, wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", }, { - name: "empty task description from interactive prompt returns error", + name: "interactive, problem statement from file", isTTY: true, - opts: &CreateOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Prompter: &prompter.PrompterMock{ - MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) { - return " ", nil - }, - }, - }, - wantErr: "SilentError", - wantErrIs: cmdutil.SilentError, - wantStdErr: "a task description is required.\n", - }, - { - name: "problem statement loaded from file non-interactively doesn't prompt or return error", opts: &CreateOptions{ BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, ProblemStatement: "", @@ -268,6 +223,60 @@ func Test_createRun(t *testing.T) { }, wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", }, + { + name: "non-interactive, problem statement loaded from file", + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + ProblemStatement: "", + ProblemStatementFile: taskDescFile, + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.CreateJobFunc = func(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*capi.Job, error) { + require.Equal(t, "OWNER", owner) + require.Equal(t, "REPO", repo) + require.Equal(t, "task description from file", problemStatement) + return &createdJobSuccessWithPR, nil + } + }, + wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", + }, + { + name: "interactive, problem statement from prompt/editor", + isTTY: true, + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Prompter: &prompter.PrompterMock{ + MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) { + require.Equal(t, "Enter the task description", prompt) + return "From editor", nil + }, + }, + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.CreateJobFunc = func(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*capi.Job, error) { + require.Equal(t, "From editor", problemStatement) + return &createdJobSuccessWithPR, nil + } + }, + wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", + }, + { + name: "interactive, empty task description from editor returns error", + isTTY: true, + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Prompter: &prompter.PrompterMock{ + MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) { + return " ", nil + }, + }, + }, + wantErr: "a task description is required", + }, { name: "missing repo returns error", opts: &CreateOptions{ @@ -276,18 +285,6 @@ func Test_createRun(t *testing.T) { }}, wantErr: "a repository is required; re-run in a repository or supply one with --repo owner/name", }, - { - name: "non-interactive empty description returns error", - opts: &CreateOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - ProblemStatement: "", - }, - wantErr: "SilentError", - wantErrIs: cmdutil.SilentError, - wantStdErr: "a task description is required.\n", - }, { name: "problem statement loaded from arg non-interactively doesn't prompt or return error", opts: &CreateOptions{ @@ -491,8 +488,6 @@ func Test_createRun(t *testing.T) { } }, wantStdout: heredoc.Doc(` - https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1 - (rendered:) (rendered:) `),