From c0a5b9aced943d6be73fad752241ce9fb30567a0 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 10 Sep 2025 18:03:42 -0600 Subject: [PATCH 1/7] Refactor agent-task create to improve file input handling Moves file reading for the problem statement from argument parsing to execution, storing the file path in CreateOptions. Updates error handling and user prompts to better distinguish between interactive and non-interactive modes. Refactors and expands tests to match the new logic and improve coverage for file and prompt scenarios. --- pkg/cmd/agent-task/create/create.go | 70 ++--- pkg/cmd/agent-task/create/create_test.go | 321 +++++++++++++++-------- 2 files changed, 255 insertions(+), 136 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index c0d1c292e..6ebbbc938 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -23,14 +23,15 @@ import ( // CreateOptions holds options for create command type CreateOptions struct { - IO *iostreams.IOStreams - BaseRepo func() (ghrepo.Interface, error) - CapiClient func() (capi.CapiClient, error) - Config func() (gh.Config, error) - ProblemStatement string - BackOff backoff.BackOff - BaseBranch string - Prompter prompter.Prompter + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + CapiClient func() (capi.CapiClient, error) + Config func() (gh.Config, error) + ProblemStatement string + BackOff backoff.BackOff + BaseBranch string + Prompter prompter.Prompter + ProblemStatementFile string } func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { @@ -41,8 +42,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co Prompter: f.Prompter, } - var fromFileName string - cmd := &cobra.Command{ Use: "create [] [flags]", Short: "Create an agent task (preview)", @@ -51,23 +50,15 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co // Support -R/--repo override opts.BaseRepo = f.BaseRepo - if err := cmdutil.MutuallyExclusive("only one of -F or arg can be provided", len(args) > 0, fromFileName != ""); err != nil { + if err := cmdutil.MutuallyExclusive("only one of -F or arg can be provided", len(args) > 0, opts.ProblemStatementFile != ""); err != nil { return err } - // Gather arg inputs for ProblemStatement + // Populate ProblemStatement from arg if len(args) > 0 { opts.ProblemStatement = args[0] - } else if fromFileName != "" { - fileContent, err := cmdutil.ReadFile(fromFileName, opts.IO.In) - if err != nil { - return cmdutil.FlagErrorf("could not read task description file: %v", err) - } - trimmed := strings.TrimSpace(string(fileContent)) - if trimmed == "" { - return cmdutil.FlagErrorf("task description file is empty") - } - opts.ProblemStatement = trimmed + } else if opts.ProblemStatementFile == "" && !opts.IO.CanPrompt() { + return cmdutil.FlagErrorf("a task description or -F is required when running non-interactively") } if runF != nil { @@ -95,7 +86,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmdutil.EnableRepoOverride(cmd, f) - cmd.Flags().StringVarP(&fromFileName, "from-file", "F", "", "Read task description from `file` (use \"-\" to read from standard input)") + cmd.Flags().StringVarP(&opts.ProblemStatementFile, "from-file", "F", "", "Read task description from `file` (use \"-\" to read from standard input)") cmd.Flags().StringVarP(&opts.BaseBranch, "base", "b", "", "Base branch for the pull request (use default branch if not provided)") return cmd @@ -110,20 +101,37 @@ func createRun(opts *CreateOptions) error { } if opts.ProblemStatement == "" { - if !opts.IO.CanPrompt() { - return cmdutil.FlagErrorf("a task description or -F is required when running non-interactively") + // 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) + } + opts.ProblemStatement = strings.TrimSpace(string(fileContent)) } - desc, err := opts.Prompter.MarkdownEditor("Enter the task description", opts.ProblemStatement, false) + if opts.IO.CanPrompt() { + 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 + } + + if opts.IO.CanPrompt() { + confirm, err := opts.Prompter.Confirm("Submit agent task", true) if err != nil { return err } - - trimmed := strings.TrimSpace(desc) - if trimmed == "" { - return cmdutil.FlagErrorf("a task description is required") + if !confirm { + return cmdutil.SilentError } - opts.ProblemStatement = trimmed } client, err := opts.CapiClient() diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index edf03f5c9..2969bdcc7 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -3,7 +3,6 @@ package create import ( "context" "errors" - "fmt" "io" "os" "path/filepath" @@ -21,38 +20,36 @@ import ( ) func TestNewCmdCreate(t *testing.T) { - tmpDir := t.TempDir() - - tmpEmptyFile := filepath.Join(tmpDir, "empty-task-description.md") - err := os.WriteFile(tmpEmptyFile, []byte(" \n\n"), 0600) - require.NoError(t, err) - - tmpFile := filepath.Join(tmpDir, "task-description.md") - err = os.WriteFile(tmpFile, []byte("task description from file"), 0600) - require.NoError(t, err) - tests := []struct { name string args string stdin string - wantOpts *CreateOptions // nil when expecting error + tty bool + wantOpts *CreateOptions wantErr string }{ { name: "no args nor file returns no error (prompting path)", + tty: true, + wantOpts: &CreateOptions{ + ProblemStatement: "", + ProblemStatementFile: "", + }, }, { name: "arg only success", args: "'task description from args'", wantOpts: &CreateOptions{ - ProblemStatement: "task description from args", + ProblemStatement: "task description from args", + ProblemStatementFile: "", }, }, { - name: "from-file success", - args: fmt.Sprintf("-F '%s'", tmpFile), + name: "from-file succes", + args: "-F task-description.md", wantOpts: &CreateOptions{ - ProblemStatement: "task description from file", + ProblemStatement: "", + ProblemStatementFile: "task-description.md", }, }, { @@ -60,7 +57,8 @@ func TestNewCmdCreate(t *testing.T) { args: "-F -", stdin: "task description from stdin", wantOpts: &CreateOptions{ - ProblemStatement: "task description from stdin", + ProblemStatement: "", + ProblemStatementFile: "-", }, }, { @@ -69,29 +67,41 @@ func TestNewCmdCreate(t *testing.T) { wantErr: "only one of -F or arg can be provided", }, { - name: "missing file path", - args: "-F does-not-exist.md", - wantErr: "could not read task description file: open does-not-exist.md:", + name: "missing file path is accepted", + args: "-F does-not-exist.md", + wantOpts: &CreateOptions{ + ProblemStatement: "", + ProblemStatementFile: "does-not-exist.md", + }, }, { - name: "empty file", - args: fmt.Sprintf("-F '%s'", tmpEmptyFile), - wantErr: "task description file is empty", + name: "empty file accepted at", + args: "-F empty-task-description.md", + wantOpts: &CreateOptions{ + ProblemStatement: "", + ProblemStatementFile: "empty-task-description.md", + }, }, { - name: "empty from stdin", - args: "-F -", - stdin: " \n\n", - wantErr: "task description file is empty", + name: "empty from stdin accepted", + args: "-F -", + stdin: " \n\n", + wantOpts: &CreateOptions{ + ProblemStatement: "", + ProblemStatementFile: "-", + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ios, stdin, _, _ := iostreams.Test() - f := &cmdutil.Factory{ - IOStreams: ios, + if tt.tty { + ios.SetStdinTTY(true) + ios.SetStdoutTTY(true) + ios.SetStderrTTY(true) } + f := &cmdutil.Factory{IOStreams: ios} var gotOpts *CreateOptions cmd := NewCmdCreate(f, func(o *CreateOptions) error { @@ -102,31 +112,35 @@ func TestNewCmdCreate(t *testing.T) { argv, err := shlex.Split(tt.args) require.NoError(t, err) cmd.SetArgs(argv) - cmd.SetIn(stdin) cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) - if tt.stdin != "" { stdin.WriteString(tt.stdin) } _, err = cmd.ExecuteC() - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) - return + require.Error(t, err, tt.wantErr) + } else { + require.NoError(t, err) } - require.NoError(t, err) if tt.wantOpts != nil { require.Equal(t, tt.wantOpts.ProblemStatement, gotOpts.ProblemStatement) + require.Equal(t, tt.wantOpts.ProblemStatementFile, gotOpts.ProblemStatementFile) } }) } } func Test_createRun(t *testing.T) { + tmpDir := t.TempDir() + taskDescFile := filepath.Join(tmpDir, "task-description.md") + emptyTaskDescFile := filepath.Join(tmpDir, "empty-task-description.md") + require.NoError(t, os.WriteFile(taskDescFile, []byte("task description from file"), 0600)) + require.NoError(t, os.WriteFile(emptyTaskDescFile, []byte(" \n\n"), 0600)) + sampleDateString := "2025-08-29T00:00:00Z" sampleDate, err := time.Parse(time.RFC3339, sampleDateString) require.NoError(t, err) @@ -157,64 +171,149 @@ func Test_createRun(t *testing.T) { } tests := []struct { - name string - capiStubs func(*testing.T, *capi.CapiClientMock) - baseRepoFunc func() (ghrepo.Interface, error) - baseBranch string - isTTY bool - prompterMock *prompter.PrompterMock - problemStatement string - wantStdout string - wantStdErr string - wantErr string + name string + isTTY bool + capiStubs func(*testing.T, *capi.CapiClientMock) + opts *CreateOptions // input options (IO & BackOff set later) + wantStdout string + wantStdErr string + wantErr string }{ { - name: "missing repo returns error", - baseRepoFunc: func() (ghrepo.Interface, error) { return nil, nil }, - wantErr: "a repository is required; re-run in a repository or supply one with --repo owner/name", + 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 prompt + "+From editor", nil + }, + ConfirmFunc: func(message string, defaultValue bool) (bool, error) { + require.Equal(t, "Submit agent task", message) + return true, nil + }, + }, + }, + isTTY: true, + 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, "Enter the task description+From editor", problemStatement) + return &createdJobSuccessWithPR, nil + } + }, + wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", }, { - name: "non-interactive empty description returns error", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - problemStatement: "", - wantErr: "a task description or -F is required when running non-interactively", + name: "interactively rejecting confirmation prompt aborts task creation", + 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", + wantStdErr: "", }, { - name: "interactive prompt success", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - isTTY: true, - problemStatement: "", + 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 + }, + }, + }, 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 } }, - prompterMock: &prompter.PrompterMock{ - MarkdownEditorFunc: func(prompt, defaultValue string, blankAllowed bool) (string, error) { - require.Equal(t, "Enter the task description", prompt) - return "From editor", nil + wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", + }, + { + name: "empty task description from interactive prompt 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: "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: "", + 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 prompt empty returns error", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - isTTY: true, - problemStatement: "", - prompterMock: &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{ + BaseRepo: func() (ghrepo.Interface, error) { + return nil, nil + }}, + wantErr: "a repository is required; re-run in a repository or supply one with --repo owner/name", }, { - name: "base branch included in create payload", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - baseBranch: "feature", - problemStatement: "Do the thing", + name: "non-interactive empty description returns error", + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + ProblemStatement: "", + }, + wantErr: "SilentError", + wantStdErr: "a task description is required.\n", + }, + { + name: "base branch included in create payload", + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + ProblemStatement: "Do the thing", + BaseBranch: "feature", + }, 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) @@ -233,24 +332,32 @@ func Test_createRun(t *testing.T) { wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", }, { - name: "create task API failure returns error", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - problemStatement: "Do the thing", + name: "create task API failure returns error", + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + ProblemStatement: "Do the thing", + }, 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, "Do the thing", problemStatement) require.Equal(t, "", baseBranch) - return nil, errors.New("some error") + return nil, errors.New("some API error") } }, - wantErr: "some error", + wantErr: "some API error", }, { - name: "get job API failure surfaces error", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - problemStatement: "Do the thing", + name: "get job API failure surfaces error", + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + ProblemStatement: "Do the thing", + }, 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) @@ -267,9 +374,13 @@ func Test_createRun(t *testing.T) { wantStdout: "job job123 queued. View progress: https://github.com/copilot/agents\n", }, { - name: "success with immediate PR", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - problemStatement: "Do the thing", + name: "success with immediate PR", + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + ProblemStatement: "Do the thing", + }, 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) @@ -282,9 +393,13 @@ func Test_createRun(t *testing.T) { wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", }, { - name: "success with delayed PR after polling", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - problemStatement: "Do the thing", + name: "success with delayed PR after polling", + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + ProblemStatement: "Do the thing", + }, 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) @@ -303,9 +418,13 @@ func Test_createRun(t *testing.T) { wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", }, { - name: "fallback after timeout returns link to global agents page", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - problemStatement: "Do the thing", + name: "fallback after polling timeout returns link to global agents page", + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + ProblemStatement: "Do the thing", + }, 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) @@ -341,23 +460,15 @@ func Test_createRun(t *testing.T) { ios.SetStdoutTTY(true) } - opts := &CreateOptions{ - IO: ios, - ProblemStatement: tt.problemStatement, - BaseRepo: tt.baseRepoFunc, - BaseBranch: tt.baseBranch, - Prompter: tt.prompterMock, - CapiClient: func() (capi.CapiClient, error) { - return capiClientMock, nil - }, + tt.opts.IO = ios + tt.opts.CapiClient = func() (capi.CapiClient, error) { + return capiClientMock, nil } - // A backoff with no interval between retries to keep tests fast, - // and also a max number of retries so we don't infinitely poll. - opts.BackOff = backoff.WithMaxRetries(&backoff.ZeroBackOff{}, 3) - - err := createRun(opts) + // fast backoff + tt.opts.BackOff = backoff.WithMaxRetries(&backoff.ZeroBackOff{}, 3) + err := createRun(tt.opts) if tt.wantErr != "" { require.Error(t, err) require.Equal(t, tt.wantErr, err.Error()) From deee0c61eda2a897dc25faa21a2828611d5748e9 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 10 Sep 2025 18:39:35 -0600 Subject: [PATCH 2/7] Update command examples for agent-task create Improves the help text for the 'gh agent-task create' command by clarifying the editor usage example and adding an example for using a file as a template. --- pkg/cmd/agent-task/create/create.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index 6ebbbc938..08c1f8c02 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -76,9 +76,12 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co # Create a task with problem statement from stdin $ echo "build me a new app" | gh agent-task create -F - - # Create a task with an editor prompt (interactive) + # Create a task with an editor $ gh agent-task create + # Create a task with an editor and a file as a template + $ gh agent-task create -F task-desc.md + # Select a different base branch for the PR $ gh agent-task create "fix errors" --base branch `), From 872cf495c24be303183a182d64b2b22e15a0c735 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 11 Sep 2025 09:19:52 -0600 Subject: [PATCH 3/7] Refactor create command tests and add base branch case Removed redundant test cases for file input and stdin in TestNewCmdCreate, and added a new test to verify that the base branch argument sets the BaseBranch field correctly. --- pkg/cmd/agent-task/create/create_test.go | 44 ++++-------------------- 1 file changed, 6 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 2969bdcc7..5780b70b3 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -44,51 +44,18 @@ func TestNewCmdCreate(t *testing.T) { ProblemStatementFile: "", }, }, - { - name: "from-file succes", - args: "-F task-description.md", - wantOpts: &CreateOptions{ - ProblemStatement: "", - ProblemStatementFile: "task-description.md", - }, - }, - { - name: "file content from stdin success", - args: "-F -", - stdin: "task description from stdin", - wantOpts: &CreateOptions{ - ProblemStatement: "", - ProblemStatementFile: "-", - }, - }, { name: "mutually exclusive arg and file", args: "'some task inline' -F foo.md", wantErr: "only one of -F or arg can be provided", }, { - name: "missing file path is accepted", - args: "-F does-not-exist.md", + name: "base branch sets baseBranch field", + args: "'task description' -b feature", wantOpts: &CreateOptions{ - ProblemStatement: "", - ProblemStatementFile: "does-not-exist.md", - }, - }, - { - name: "empty file accepted at", - args: "-F empty-task-description.md", - wantOpts: &CreateOptions{ - ProblemStatement: "", - ProblemStatementFile: "empty-task-description.md", - }, - }, - { - name: "empty from stdin accepted", - args: "-F -", - stdin: " \n\n", - wantOpts: &CreateOptions{ - ProblemStatement: "", - ProblemStatementFile: "-", + ProblemStatement: "task description", + ProblemStatementFile: "", + BaseBranch: "feature", }, }, } @@ -129,6 +96,7 @@ func TestNewCmdCreate(t *testing.T) { if tt.wantOpts != nil { require.Equal(t, tt.wantOpts.ProblemStatement, gotOpts.ProblemStatement) require.Equal(t, tt.wantOpts.ProblemStatementFile, gotOpts.ProblemStatementFile) + require.Equal(t, tt.wantOpts.BaseBranch, gotOpts.BaseBranch) } }) } From 666f5574e4f8074063370c54c5e0cf2cf7d2ac3c Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 11 Sep 2025 09:20:48 -0600 Subject: [PATCH 4/7] Remove unused stdin field from create command tests The 'stdin' field and related code were removed from TestNewCmdCreate as they were no longer used. This simplifies the test structure and eliminates unnecessary code. --- pkg/cmd/agent-task/create/create_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 5780b70b3..06e9c7c4e 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -23,7 +23,6 @@ func TestNewCmdCreate(t *testing.T) { tests := []struct { name string args string - stdin string tty bool wantOpts *CreateOptions wantErr string @@ -82,9 +81,6 @@ func TestNewCmdCreate(t *testing.T) { cmd.SetIn(stdin) cmd.SetOut(io.Discard) cmd.SetErr(io.Discard) - if tt.stdin != "" { - stdin.WriteString(tt.stdin) - } _, err = cmd.ExecuteC() if tt.wantErr != "" { From f22dc9271b19c31f41b43bb400eaa06a3b69ac45 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 11 Sep 2025 09:22:27 -0600 Subject: [PATCH 5/7] Update test to use edited task description Modified the test in create_test.go to return and expect 'edited task description' instead of concatenating the prompt string. This clarifies the test's intent and expected behavior for the MarkdownEditorFunc and CreateJobFunc. --- pkg/cmd/agent-task/create/create_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 06e9c7c4e..638559e74 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -153,7 +153,7 @@ func Test_createRun(t *testing.T) { 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 prompt + "+From editor", nil + return "edited task description", nil }, ConfirmFunc: func(message string, defaultValue bool) (bool, error) { require.Equal(t, "Submit agent task", message) @@ -166,7 +166,7 @@ func Test_createRun(t *testing.T) { 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, "Enter the task description+From editor", problemStatement) + require.Equal(t, "edited task description", problemStatement) return &createdJobSuccessWithPR, nil } }, From 1dbb694790ba59c12958a254b78e8030adb755a2 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 11 Sep 2025 09:24:42 -0600 Subject: [PATCH 6/7] Add test for non-interactive problem statement input Adds a test case to ensure that when a problem statement is provided as an argument non-interactively, the command does not prompt or return an error, and the correct job is created. --- pkg/cmd/agent-task/create/create_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 638559e74..36653374a 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -271,6 +271,22 @@ func Test_createRun(t *testing.T) { wantErr: "SilentError", wantStdErr: "a task description is required.\n", }, + { + name: "problem statement loaded from arg non-interactively doesn't prompt or return error", + opts: &CreateOptions{ + BaseRepo: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + ProblemStatement: "task description", + }, + 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", problemStatement) + return &createdJobSuccessWithPR, nil + } + }, + wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", + }, { name: "base branch included in create payload", opts: &CreateOptions{ From 3831380d131bd5fde42186a051f10e584e341e84 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 11 Sep 2025 11:23:18 -0600 Subject: [PATCH 7/7] Add error type assertion to createRun tests Introduces a new 'wantErrIs' field to test cases in Test_createRun to assert specific error types using require.ErrorIs. This enhances test coverage by verifying not only error messages but also error types. --- pkg/cmd/agent-task/create/create_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 36653374a..b855ce687 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -142,6 +142,7 @@ func Test_createRun(t *testing.T) { wantStdout string wantStdErr string wantErr string + wantErrIs error }{ { name: "interactive with file prompts to edit with file contents", @@ -190,6 +191,7 @@ func Test_createRun(t *testing.T) { }, isTTY: true, wantErr: "SilentError", + wantErrIs: cmdutil.SilentError, wantStdErr: "", }, { @@ -233,6 +235,7 @@ func Test_createRun(t *testing.T) { }, }, wantErr: "SilentError", + wantErrIs: cmdutil.SilentError, wantStdErr: "a task description is required.\n", }, { @@ -269,6 +272,7 @@ func Test_createRun(t *testing.T) { ProblemStatement: "", }, wantErr: "SilentError", + wantErrIs: cmdutil.SilentError, wantStdErr: "a task description is required.\n", }, { @@ -449,10 +453,13 @@ func Test_createRun(t *testing.T) { tt.opts.BackOff = backoff.WithMaxRetries(&backoff.ZeroBackOff{}, 3) err := createRun(tt.opts) + if tt.wantErrIs != nil { + require.ErrorIs(t, err, tt.wantErrIs) + } if tt.wantErr != "" { require.Error(t, err) require.Equal(t, tt.wantErr, err.Error()) - } else { + } else if tt.wantErrIs == nil { require.NoError(t, err) }