From cb9808fa6e6657f27d0cf5ac2b0b33ef6abe1d10 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 13:51:13 +0100 Subject: [PATCH 1/3] fix(agent-task create): avoid prompting when problem statement is provided Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/create/create.go | 32 ++--- pkg/cmd/agent-task/create/create_test.go | 150 ++++++++++------------- 2 files changed, 79 insertions(+), 103 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index 8b8084a1f..b602d0a70 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -121,36 +121,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 } } diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index d1298e606..69bfef2ab 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -158,101 +158,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 +208,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 +270,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{ From c7a811e56769a95d67bc9c3a5a4321d827356a08 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 13:55:37 +0100 Subject: [PATCH 2/3] fix(agent-task create): use pager when following logs Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/create/create.go | 15 ++++++++++----- pkg/cmd/agent-task/create/create_test.go | 2 -- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index b602d0a70..a460a12e8 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -162,6 +162,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() @@ -177,9 +181,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 } @@ -250,8 +251,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 @@ -267,6 +273,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 69bfef2ab..c96a2ef24 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -473,8 +473,6 @@ func Test_createRun(t *testing.T) { } }, wantStdout: heredoc.Doc(` - https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1 - (rendered:) (rendered:) `), From 863329b4c1b69046b02ceadf3438bbbe38363bd1 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 14:14:59 +0100 Subject: [PATCH 3/3] fix(agent-task create): block empty problem statement arg Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/create/create.go | 3 +++ pkg/cmd/agent-task/create/create_test.go | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index a460a12e8..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") } diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index c96a2ef24..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) }