From 3574ee9c30a96f12ca36b9593ab8b05b370b8741 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 2 Sep 2025 16:56:10 -0600 Subject: [PATCH 01/15] Add agent task creation command and job API Introduces the 'create' subcommand for agent tasks, allowing users to create agent jobs via the Copilot API. Adds job API client methods, job model, and polling logic to retrieve associated pull requests. Includes tests for various job creation scenarios. --- pkg/cmd/agent-task/agent_task.go | 2 + pkg/cmd/agent-task/capi/client.go | 2 + pkg/cmd/agent-task/capi/job.go | 122 +++++++++++++++ pkg/cmd/agent-task/create/create.go | 173 ++++++++++++++++++++ pkg/cmd/agent-task/create/create_test.go | 191 +++++++++++++++++++++++ 5 files changed, 490 insertions(+) create mode 100644 pkg/cmd/agent-task/capi/job.go create mode 100644 pkg/cmd/agent-task/create/create.go create mode 100644 pkg/cmd/agent-task/create/create_test.go diff --git a/pkg/cmd/agent-task/agent_task.go b/pkg/cmd/agent-task/agent_task.go index cbbd3e278..e732e31de 100644 --- a/pkg/cmd/agent-task/agent_task.go +++ b/pkg/cmd/agent-task/agent_task.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + cmdCreate "github.com/cli/cli/v2/pkg/cmd/agent-task/create" cmdList "github.com/cli/cli/v2/pkg/cmd/agent-task/list" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/go-gh/v2/pkg/auth" @@ -29,6 +30,7 @@ func NewCmdAgentTask(f *cmdutil.Factory) *cobra.Command { // register subcommands cmd.AddCommand(cmdList.NewCmdList(f, nil)) + cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) return cmd } diff --git a/pkg/cmd/agent-task/capi/client.go b/pkg/cmd/agent-task/capi/client.go index 480692c16..9021d6086 100644 --- a/pkg/cmd/agent-task/capi/client.go +++ b/pkg/cmd/agent-task/capi/client.go @@ -15,6 +15,8 @@ const capiHost = "api.githubcopilot.com" type CapiClient interface { ListSessionsForViewer(ctx context.Context, limit int) ([]*Session, error) ListSessionsForRepo(ctx context.Context, owner string, repo string, limit int) ([]*Session, error) + CreateJob(ctx context.Context, owner, repo, problemStatement string) (*Job, error) + GetJob(ctx context.Context, owner, repo, jobID string) (*Job, error) } // CAPIClient is a client for interacting with the Copilot API diff --git a/pkg/cmd/agent-task/capi/job.go b/pkg/cmd/agent-task/capi/job.go new file mode 100644 index 000000000..47410526e --- /dev/null +++ b/pkg/cmd/agent-task/capi/job.go @@ -0,0 +1,122 @@ +package capi + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "time" +) + +const defaultEventType = "gh_cli" + +// Job represents a coding agent's task. Used to request a new session. +type Job struct { + ID string `json:"job_id"` + SessionID string `json:"session_id"` + ProblemStatement string `json:"problem_statement,omitempty"` + ContentFilterMode string `json:"content_filter_mode,omitempty"` + Status string `json:"status,omitempty"` + Result string `json:"result,omitempty"` + Actor *JobActor `json:"actor,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + PullRequest *JobPullRequest `json:"pull_request,omitempty"` + WorkflowRun *struct { + ID string `json:"id"` + } `json:"workflow_run,omitempty"` + ErrorInfo *JobError `json:"error,omitempty"` +} + +type JobActor struct { + ID int `json:"id"` + Login string `json:"login"` +} + +type JobPullRequest struct { + ID int `json:"id"` + Number int `json:"number"` +} + +type JobError struct { + Message string `json:"message"` + ResponseStatusCode int `json:"response_status_code,string"` + Service string `json:"service"` +} + +const jobsBasePathV1 = baseCAPIURL + "/agents/swe/v1/jobs" + +// CreateJob queues a new job using the v1 Jobs API. It may or may not +// return Pull Request information. If Pull Request information is required +// following up by polling GetJob with the job ID is necessary. +func (c *CAPIClient) CreateJob(ctx context.Context, owner, repo, problemStatement string) (*Job, error) { + if owner == "" || repo == "" { + return nil, errors.New("owner and repo are required") + } + if problemStatement == "" { + return nil, errors.New("problem statement is required") + } + + url := fmt.Sprintf("%s/%s/%s", jobsBasePathV1, owner, repo) + body := map[string]any{ + "problem_statement": problemStatement, + "event_type": defaultEventType, + } + b, _ := json.Marshal(body) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(b)) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + res, err := c.httpClient.Do(req) + if err != nil { + return nil, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusCreated && res.StatusCode != http.StatusOK { // accept 201 or 200 + // Attempt to parse error body for message + var er struct { + Error struct { + Message string `json:"message"` + } `json:"error"` + } + _ = json.NewDecoder(res.Body).Decode(&er) + msg := er.Error.Message + if msg == "" { + msg = res.Status + } + return nil, fmt.Errorf("failed to create job: %s", msg) + } + var j Job + if err := json.NewDecoder(res.Body).Decode(&j); err != nil { + return nil, fmt.Errorf("failed to decode create job response: %w", err) + } + return &j, nil +} + +// GetJob retrieves a agent job +func (c *CAPIClient) GetJob(ctx context.Context, owner, repo, jobID string) (*Job, error) { + if owner == "" || repo == "" || jobID == "" { + return nil, errors.New("owner, repo, and jobID are required") + } + url := fmt.Sprintf("%s/%s/%s/%s", jobsBasePathV1, owner, repo, jobID) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) + if err != nil { + return nil, err + } + res, err := c.httpClient.Do(req) + if err != nil { + return nil, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("failed to get job: %s", res.Status) + } + var j Job + if err := json.NewDecoder(res.Body).Decode(&j); err != nil { + return nil, fmt.Errorf("failed to decode get job response: %w", err) + } + return &j, nil +} diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go new file mode 100644 index 000000000..b7667dcf5 --- /dev/null +++ b/pkg/cmd/agent-task/create/create.go @@ -0,0 +1,173 @@ +package create + +import ( + "context" + "errors" + "fmt" + "io" + "strings" + "time" + + "github.com/cenkalti/backoff/v4" + + "github.com/cli/cli/v2/internal/gh" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +// 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 +} + +func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { + opts := &CreateOptions{ + IO: f.IOStreams, + } + cmd := &cobra.Command{ + Use: "create ", + Short: "Create an agent task (preview)", + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + opts.ProblemStatement = strings.Join(args, " ") + // Support -R/--repo override + if f != nil { + opts.BaseRepo = f.BaseRepo + } + if runF != nil { + return runF(opts) + } + return createRun(opts) + }, + } + if f != nil { + cmdutil.EnableRepoOverride(cmd, f) + } + + opts.CapiClient = func() (capi.CapiClient, error) { + cfg, err := f.Config() + if err != nil { + return nil, err + } + httpClient, err := f.HttpClient() + if err != nil { + return nil, err + } + authCfg := cfg.Authentication() + return capi.NewCAPIClient(httpClient, authCfg), nil + } + + return cmd +} + +func createRun(opts *CreateOptions) error { + if opts.ProblemStatement == "" { + return cmdutil.FlagErrorf("a problem statement is required") + } + if opts.BaseRepo == nil { + return errors.New("failed to resolve repository") + } + repo, err := opts.BaseRepo() + if err != nil || repo == nil || repo.RepoOwner() == "" || repo.RepoName() == "" { + // Not printing the error that came back from BaseRepo() here because we want + // something clear, human friendly, and actionable. + return fmt.Errorf("error: a repository is required; re-run in a repository or supply one with --repo owner/name") + } + + client, err := opts.CapiClient() + if err != nil { + return err + } + + ctx := context.Background() + opts.IO.StartProgressIndicatorWithLabel(fmt.Sprintf("Creating agent task in %s/%s...", repo.RepoOwner(), repo.RepoName())) + defer opts.IO.StopProgressIndicator() + + job, err := client.CreateJob(ctx, repo.RepoOwner(), repo.RepoName(), opts.ProblemStatement) + if err != nil { + return err + } + + // Print this agent session URL and exit if we happen to get it. + // Right now, this never happens. + if job.PullRequest != nil && job.PullRequest.Number > 0 { + fmt.Fprintf(opts.IO.Out, "%s\n", agentSessionWebURL(repo, job)) + return nil + } + + // Otherwise, poll using exponential backoff until we either observe a PR or hit the overall timeout. + // Ensure we have a backoff strategy. + if opts.BackOff == nil { + opts.BackOff = backoff.NewExponentialBackOff( + backoff.WithMaxElapsedTime(4*time.Second), + backoff.WithInitialInterval(300*time.Millisecond), + backoff.WithMaxInterval(2*time.Second), + backoff.WithMultiplier(1.5), + ) + } + + jobWithPR, err := fetchJobWithBackoff(ctx, client, repo, job.ID, opts.IO.ErrOut, opts.BackOff) + if err != nil { + return err + } + + if jobWithPR != nil { + opts.IO.StopProgressIndicator() + fmt.Fprintln(opts.IO.Out, agentSessionWebURL(repo, jobWithPR)) + return nil + } + + // Fallback if PR not yet ready + opts.IO.StopProgressIndicator() + fmt.Fprintf(opts.IO.Out, "job %s queued. View progress: https://github.com/copilot/agents\n", job.ID) + return nil +} + +func agentSessionWebURL(repo ghrepo.Interface, j *capi.Job) string { + if j == nil || j.PullRequest == nil { + return "" + } + if j.SessionID == "" { + return fmt.Sprintf("https://github.com/%s/%s/pull/%d", repo.RepoOwner(), repo.RepoName(), j.PullRequest.Number) + } + return fmt.Sprintf("https://github.com/%s/%s/pull/%d/agent-sessions/%s", repo.RepoOwner(), repo.RepoName(), j.PullRequest.Number, j.SessionID) +} + +// fetchJobWithBackoff polls the job resource until a PR number is present or the overall +// timeout elapses. It returns the updated Job on success, (nil, nil) on timeout, +// and (nil, error) only for non-retryable failures. +func fetchJobWithBackoff(ctx context.Context, client capi.CapiClient, repo ghrepo.Interface, jobID string, errOut io.Writer, bo backoff.BackOff) (*capi.Job, error) { + // sentinel error to signal retry without surfacing to caller + var errPRNotReady = errors.New("job not ready") + + var result *capi.Job + retryErr := backoff.Retry(func() error { + j, getErr := client.GetJob(ctx, repo.RepoOwner(), repo.RepoName(), jobID) + if getErr != nil { + fmt.Fprintf(errOut, "warning: failed to get job status: %v\n", getErr) + return errPRNotReady + } + if j.PullRequest != nil && j.PullRequest.Number > 0 { + result = j + return nil + } + return errPRNotReady + }, backoff.WithContext(bo, ctx)) + + if retryErr != nil { + if errors.Is(retryErr, errPRNotReady) { + // Timed out or failed to fetch + return nil, nil + } + return nil, retryErr + } + return result, nil +} diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go new file mode 100644 index 000000000..67ca541e9 --- /dev/null +++ b/pkg/cmd/agent-task/create/create_test.go @@ -0,0 +1,191 @@ +package create + +import ( + "net/http" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cenkalti/backoff/v4" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/require" +) + +// Test basic option parsing & repository requirement +func TestNewCmdCreate_Args(t *testing.T) { + f := &cmdutil.Factory{} + cmd := NewCmdCreate(f, func(o *CreateOptions) error { return nil }) + // no args should error via cobra MinimumNArgs before our runF + // TODO once we support more sources of problem statement input, + // this will change. + _, err := cmd.ExecuteC() + require.Error(t, err) +} + +func Test_createRun(t *testing.T) { + createdJobSuccessResponse := heredoc.Doc(`{ + "job_id":"job123", + "session_id":"sess1", + "actor":{"id":1,"login":"octocat"}, + "created_at":"2025-08-29T00:00:00Z", + "updated_at":"2025-08-29T00:00:00Z" + }`) + createdJobSuccessWithPRResponse := heredoc.Doc(`{ + "job_id":"job123", + "session_id":"sess1", + "actor":{"id":1,"login":"octocat"}, + "created_at":"2025-08-29T00:00:00Z", + "updated_at":"2025-08-29T00:00:00Z", + "pull_request":{"id":101,"number":42} + }`) + createdJobTimeoutResponse := heredoc.Doc(`{ + "job_id":"jobABC", + "session_id":"sess1", + "actor":{"id":1,"login":"octocat"}, + "created_at":"2025-08-29T00:00:00Z", + "updated_at":"2025-08-29T00:00:00Z" + }`) + + tests := []struct { + name string + stubs func(*httpmock.Registry) + baseRepo ghrepo.Interface + baseRepoErr error + problemStatement string + wantStdout string + wantErr string + }{ + { + name: "success with immediate PR", + baseRepo: ghrepo.New("OWNER", "REPO"), + problemStatement: "Do the thing", + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(201, createdJobSuccessWithPRResponse), + ) + }, + wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", + }, + { + name: "success with delayed PR after polling", + baseRepo: ghrepo.New("OWNER", "REPO"), + problemStatement: "Do the thing", + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(201, createdJobSuccessResponse), + ) + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/job123"), "api.githubcopilot.com"), + httpmock.StringResponse(`{"job_id":"job123","pull_request":{"id":101,"number":42}}`), + ) + }, + wantStdout: "https://github.com/OWNER/REPO/pull/42\n", + }, + { + name: "fallback after timeout returns link to global agents page", + baseRepo: ghrepo.New("OWNER", "REPO"), + problemStatement: "Do the thing", + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(201, createdJobTimeoutResponse), + ) + for range 3 { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/jobABC"), "api.githubcopilot.com"), + httpmock.StringResponse(`{"job_id":"jobABC"}`), + ) + } + }, + wantStdout: "job jobABC queued. View progress: https://github.com/copilot/agents\n", + }, + { + name: "missing repo returns error", + problemStatement: "task", + baseRepo: ghrepo.New("", ""), + wantErr: "error: a repository is required; re-run in a repository or supply one with --repo owner/name", + }, + { + name: "create task API failure returns error", + baseRepo: ghrepo.New("OWNER", "REPO"), + problemStatement: "do the thing", + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(400, `{"error":{"message":"some API error"}}`), + ) + }, + wantErr: "failed to create job: some API error", + }, + { + name: "error fetching job during polling returns error and falls back to global agents page", + baseRepo: ghrepo.New("OWNER", "REPO"), + problemStatement: "Do the thing", + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(201, createdJobTimeoutResponse), + ) + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/jobABC"), "api.githubcopilot.com"), + httpmock.StringResponse(`{"job_id":"jobABC"}`), + ) + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/jobABC"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(500, `{"error":{"message":"something went wrong"}}`), + ) + }, + wantStdout: "job jobABC queued. View progress: https://github.com/copilot/agents\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, _ := iostreams.Test() + opts := &CreateOptions{ + IO: ios, + ProblemStatement: tt.problemStatement, + } + + if tt.baseRepo != nil || tt.baseRepoErr != nil { + br, bre := tt.baseRepo, tt.baseRepoErr + opts.BaseRepo = func() (ghrepo.Interface, error) { return br, bre } + } + + // A backoff with no internal 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) + + reg := &httpmock.Registry{} + if tt.stubs != nil { + tt.stubs(reg) + cfg := config.NewBlankConfig() + cfg.Set("github.com", "oauth_token", "OTOKEN") + authCfg := cfg.Authentication() + client := capi.NewCAPIClient(&http.Client{Transport: reg}, authCfg) + opts.CapiClient = func() (capi.CapiClient, error) { return client, nil } + } + + err := createRun(opts) + + if tt.wantErr != "" { + require.Error(t, err) + require.Equal(t, err.Error(), tt.wantErr) + } else { + require.NoError(t, err) + } + if tt.wantStdout != "" { + require.Equal(t, tt.wantStdout, stdout.String()) + } + if tt.stubs != nil { + reg.Verify(t) + } + }) + } +} From a3fa83071d791e024c76b8f52fec902a51272fa1 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:03:48 -0600 Subject: [PATCH 02/15] Update agent-task create command argument handling Changed argument parsing to accept a maximum of one argument and assign it directly to ProblemStatement. Added error handling for missing problem statement and removed unused strings import. --- pkg/cmd/agent-task/create/create.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index b7667dcf5..df0b14658 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "strings" "time" "github.com/cenkalti/backoff/v4" @@ -35,9 +34,15 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "create ", Short: "Create an agent task (preview)", - Args: cobra.MinimumNArgs(1), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - opts.ProblemStatement = strings.Join(args, " ") + // TODO: We'll support prompting for the problem statement if not provided + // and from file flags, later. + if len(args) == 0 { + return cmdutil.FlagErrorf("a problem statement is required") + } + + opts.ProblemStatement = args[0] // Support -R/--repo override if f != nil { opts.BaseRepo = f.BaseRepo From a81cff3fdfddb21adb8c544d14f70a442440e565 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:06:30 -0600 Subject: [PATCH 03/15] Update command usage for agent task creation Changed the command usage string from 'create ' to 'create ""' for improved clarity in the agent task creation command. --- pkg/cmd/agent-task/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index df0b14658..7238bf286 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -32,7 +32,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co IO: f.IOStreams, } cmd := &cobra.Command{ - Use: "create ", + Use: "create \"\"", Short: "Create an agent task (preview)", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { From 3a7465ed96c5206f423cf33bfc7e7b95f823ffc1 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:26:58 -0600 Subject: [PATCH 04/15] Remove redundant 'error:' prefix from repo error Simplifies the error message when a repository is missing by removing the unnecessary 'error:' prefix, making the output clearer and more user-friendly. --- pkg/cmd/agent-task/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index 7238bf286..dfed399be 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -84,7 +84,7 @@ func createRun(opts *CreateOptions) error { if err != nil || repo == nil || repo.RepoOwner() == "" || repo.RepoName() == "" { // Not printing the error that came back from BaseRepo() here because we want // something clear, human friendly, and actionable. - return fmt.Errorf("error: a repository is required; re-run in a repository or supply one with --repo owner/name") + return fmt.Errorf("a repository is required; re-run in a repository or supply one with --repo owner/name") } client, err := opts.CapiClient() From 2bec2bcf65df40642f51f0787f1505f817d7d852 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:28:39 -0600 Subject: [PATCH 05/15] Update error message in createRun test Removed 'error:' prefix from expected error message in the 'missing repo returns error' test case for consistency with actual output. --- pkg/cmd/agent-task/create/create_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 67ca541e9..3bbdcc18a 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -109,7 +109,7 @@ func Test_createRun(t *testing.T) { name: "missing repo returns error", problemStatement: "task", baseRepo: ghrepo.New("", ""), - wantErr: "error: a repository is required; re-run in a repository or supply one with --repo owner/name", + wantErr: "a repository is required; re-run in a repository or supply one with --repo owner/name", }, { name: "create task API failure returns error", From 3d8d5f3e312321b4993583b694a0468ffcd2da60 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:29:38 -0600 Subject: [PATCH 06/15] Update test to expect 500 error response Changed the mocked API response status from 400 to 500 in Test_createRun to better reflect the expected error scenario. --- pkg/cmd/agent-task/create/create_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 3bbdcc18a..3e7ba4043 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -118,7 +118,7 @@ func Test_createRun(t *testing.T) { stubs: func(reg *httpmock.Registry) { reg.Register( httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), - httpmock.StatusStringResponse(400, `{"error":{"message":"some API error"}}`), + httpmock.StatusStringResponse(500, `{"error":{"message":"some API error"}}`), ) }, wantErr: "failed to create job: some API error", From c3bbd374aa97444298f7cd76d017919b0bd4fcea Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:33:31 -0600 Subject: [PATCH 07/15] Remove nil check for job in agentSessionWebURL Simplifies the agentSessionWebURL function by removing the redundant nil check for the job parameter, assuming it is always non-nil when called. --- pkg/cmd/agent-task/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index dfed399be..843579ba6 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -137,7 +137,7 @@ func createRun(opts *CreateOptions) error { } func agentSessionWebURL(repo ghrepo.Interface, j *capi.Job) string { - if j == nil || j.PullRequest == nil { + if j.PullRequest == nil { return "" } if j.SessionID == "" { From 33d119664525dc2f7fc78dab68c45507b24355f1 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:40:00 -0600 Subject: [PATCH 08/15] Escape URL path segments in agent session links Uses url.PathEscape for repo owner, repo name, and session ID when constructing agent session URLs to ensure proper encoding and prevent issues with special characters. --- pkg/cmd/agent-task/create/create.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index 843579ba6..af3b27bfa 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "net/url" "time" "github.com/cenkalti/backoff/v4" @@ -141,9 +142,9 @@ func agentSessionWebURL(repo ghrepo.Interface, j *capi.Job) string { return "" } if j.SessionID == "" { - return fmt.Sprintf("https://github.com/%s/%s/pull/%d", repo.RepoOwner(), repo.RepoName(), j.PullRequest.Number) + return fmt.Sprintf("https://github.com/%s/%s/pull/%d", url.PathEscape(repo.RepoOwner()), url.PathEscape(repo.RepoName()), j.PullRequest.Number) } - return fmt.Sprintf("https://github.com/%s/%s/pull/%d/agent-sessions/%s", repo.RepoOwner(), repo.RepoName(), j.PullRequest.Number, j.SessionID) + return fmt.Sprintf("https://github.com/%s/%s/pull/%d/agent-sessions/%s", url.PathEscape(repo.RepoOwner()), url.PathEscape(repo.RepoName()), j.PullRequest.Number, url.PathEscape(j.SessionID)) } // fetchJobWithBackoff polls the job resource until a PR number is present or the overall From b890f880c0ed6a5f984735a6e6bc2cc43b542906 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:48:17 -0600 Subject: [PATCH 09/15] Escape owner and repo in job creation URL Uses url.PathEscape for owner and repo when constructing the job creation URL to ensure proper encoding and prevent issues with special characters. --- pkg/cmd/agent-task/capi/job.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/capi/job.go b/pkg/cmd/agent-task/capi/job.go index 47410526e..405bbf443 100644 --- a/pkg/cmd/agent-task/capi/job.go +++ b/pkg/cmd/agent-task/capi/job.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "time" ) @@ -59,7 +60,7 @@ func (c *CAPIClient) CreateJob(ctx context.Context, owner, repo, problemStatemen return nil, errors.New("problem statement is required") } - url := fmt.Sprintf("%s/%s/%s", jobsBasePathV1, owner, repo) + url := fmt.Sprintf("%s/%s/%s", jobsBasePathV1, url.PathEscape(owner), url.PathEscape(repo)) body := map[string]any{ "problem_statement": problemStatement, "event_type": defaultEventType, From 9ec10a4d318c2d4f68bd60959862c34e457e7a4e Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:49:35 -0600 Subject: [PATCH 10/15] Escape URL path parameters in GetJob request Uses url.PathEscape for owner, repo, and jobID in the GetJob API request to ensure proper encoding and prevent issues with special characters. --- pkg/cmd/agent-task/capi/job.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/capi/job.go b/pkg/cmd/agent-task/capi/job.go index 405bbf443..f07da20f2 100644 --- a/pkg/cmd/agent-task/capi/job.go +++ b/pkg/cmd/agent-task/capi/job.go @@ -102,7 +102,7 @@ func (c *CAPIClient) GetJob(ctx context.Context, owner, repo, jobID string) (*Jo if owner == "" || repo == "" || jobID == "" { return nil, errors.New("owner, repo, and jobID are required") } - url := fmt.Sprintf("%s/%s/%s/%s", jobsBasePathV1, owner, repo, jobID) + url := fmt.Sprintf("%s/%s/%s/%s", jobsBasePathV1, url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(jobID)) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) if err != nil { return nil, err From 5472fcffe9a41ae84f8a7d0c216ddd13e17c5cc9 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 11:28:16 -0600 Subject: [PATCH 11/15] Use Job struct in request Refactored CreateJob to use the Job struct as the payload instead of a map, improving consistency and maintainability. --- pkg/cmd/agent-task/capi/job.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/agent-task/capi/job.go b/pkg/cmd/agent-task/capi/job.go index f07da20f2..f504b66e0 100644 --- a/pkg/cmd/agent-task/capi/job.go +++ b/pkg/cmd/agent-task/capi/job.go @@ -15,15 +15,16 @@ const defaultEventType = "gh_cli" // Job represents a coding agent's task. Used to request a new session. type Job struct { - ID string `json:"job_id"` - SessionID string `json:"session_id"` + ID string `json:"job_id,omitempty"` + SessionID string `json:"session_id,omitempty"` ProblemStatement string `json:"problem_statement,omitempty"` + EventType string `json:"event_type,omitempty"` ContentFilterMode string `json:"content_filter_mode,omitempty"` Status string `json:"status,omitempty"` Result string `json:"result,omitempty"` Actor *JobActor `json:"actor,omitempty"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + CreatedAt time.Time `json:"created_at,omitempty"` + UpdatedAt time.Time `json:"updated_at,omitempty"` PullRequest *JobPullRequest `json:"pull_request,omitempty"` WorkflowRun *struct { ID string `json:"id"` @@ -61,11 +62,13 @@ func (c *CAPIClient) CreateJob(ctx context.Context, owner, repo, problemStatemen } url := fmt.Sprintf("%s/%s/%s", jobsBasePathV1, url.PathEscape(owner), url.PathEscape(repo)) - body := map[string]any{ - "problem_statement": problemStatement, - "event_type": defaultEventType, + + payload := &Job{ + ProblemStatement: problemStatement, + EventType: defaultEventType, } - b, _ := json.Marshal(body) + b, _ := json.Marshal(payload) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(b)) if err != nil { return nil, err From 32bf9159ea429de66c8130760009c5343e3ca711 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 12:34:13 -0600 Subject: [PATCH 12/15] Improve job error handling and update tests Normalizes job API error messages to include status code and text, ensures errors from job polling are surfaced to stderr without halting execution, and updates tests to verify error output and remove outdated fallback behavior. --- pkg/cmd/agent-task/capi/job.go | 4 +- pkg/cmd/agent-task/create/create.go | 22 ++++++----- pkg/cmd/agent-task/create/create_test.go | 49 +++++++++++++----------- 3 files changed, 41 insertions(+), 34 deletions(-) diff --git a/pkg/cmd/agent-task/capi/job.go b/pkg/cmd/agent-task/capi/job.go index f504b66e0..ec215c5fe 100644 --- a/pkg/cmd/agent-task/capi/job.go +++ b/pkg/cmd/agent-task/capi/job.go @@ -116,7 +116,9 @@ func (c *CAPIClient) GetJob(ctx context.Context, owner, repo, jobID string) (*Jo } defer res.Body.Close() if res.StatusCode != http.StatusOK { - return nil, fmt.Errorf("failed to get job: %s", res.Status) + // Normalize to " " form + statusText := fmt.Sprintf("%d %s", res.StatusCode, http.StatusText(res.StatusCode)) + return nil, fmt.Errorf("failed to get job: %s", statusText) } var j Job if err := json.NewDecoder(res.Body).Decode(&j); err != nil { diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index af3b27bfa..61e3d54b9 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "io" "net/url" "time" @@ -120,9 +119,12 @@ func createRun(opts *CreateOptions) error { ) } - jobWithPR, err := fetchJobWithBackoff(ctx, client, repo, job.ID, opts.IO.ErrOut, opts.BackOff) + jobWithPR, err := fetchJobWithBackoff(ctx, client, repo, job.ID, opts.BackOff) if err != nil { - return err + // If this does happen ever, we still want the user to get the + // fallback message and URL. So, we don't return with this error, + // but we do still want to print it. + fmt.Fprintf(opts.IO.ErrOut, "%v\n", err) } if jobWithPR != nil { @@ -150,16 +152,16 @@ func agentSessionWebURL(repo ghrepo.Interface, j *capi.Job) string { // fetchJobWithBackoff polls the job resource until a PR number is present or the overall // timeout elapses. It returns the updated Job on success, (nil, nil) on timeout, // and (nil, error) only for non-retryable failures. -func fetchJobWithBackoff(ctx context.Context, client capi.CapiClient, repo ghrepo.Interface, jobID string, errOut io.Writer, bo backoff.BackOff) (*capi.Job, error) { - // sentinel error to signal retry without surfacing to caller +func fetchJobWithBackoff(ctx context.Context, client capi.CapiClient, repo ghrepo.Interface, jobID string, bo backoff.BackOff) (*capi.Job, error) { + // sentinel error to signal timeout var errPRNotReady = errors.New("job not ready") var result *capi.Job retryErr := backoff.Retry(func() error { - j, getErr := client.GetJob(ctx, repo.RepoOwner(), repo.RepoName(), jobID) - if getErr != nil { - fmt.Fprintf(errOut, "warning: failed to get job status: %v\n", getErr) - return errPRNotReady + j, err := client.GetJob(ctx, repo.RepoOwner(), repo.RepoName(), jobID) + if err != nil { + // Do not retry on GetJob errors; surface immediately. + return backoff.Permanent(err) } if j.PullRequest != nil && j.PullRequest.Number > 0 { result = j @@ -170,7 +172,7 @@ func fetchJobWithBackoff(ctx context.Context, client capi.CapiClient, repo ghrep if retryErr != nil { if errors.Is(retryErr, errPRNotReady) { - // Timed out or failed to fetch + // Timed out return nil, nil } return nil, retryErr diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 3e7ba4043..aec6831c4 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -57,8 +57,26 @@ func Test_createRun(t *testing.T) { baseRepoErr error problemStatement string wantStdout string + wantStdErr string wantErr string }{ + { + name: "get job API failure surfaces error", + baseRepo: ghrepo.New("OWNER", "REPO"), + problemStatement: "Do the thing", + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(201, createdJobTimeoutResponse), + ) + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/jobABC"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(500, `{"error":{"message":"internal server error"}}`), + ) + }, + wantStdErr: "failed to get job: 500 Internal Server Error\n", + wantStdout: "job jobABC queued. View progress: https://github.com/copilot/agents\n", + }, { name: "success with immediate PR", baseRepo: ghrepo.New("OWNER", "REPO"), @@ -96,7 +114,8 @@ func Test_createRun(t *testing.T) { httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), httpmock.StatusStringResponse(201, createdJobTimeoutResponse), ) - for range 3 { + // 4 attempts: initial + 3 retries + for range 4 { reg.Register( httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/jobABC"), "api.githubcopilot.com"), httpmock.StringResponse(`{"job_id":"jobABC"}`), @@ -123,31 +142,12 @@ func Test_createRun(t *testing.T) { }, wantErr: "failed to create job: some API error", }, - { - name: "error fetching job during polling returns error and falls back to global agents page", - baseRepo: ghrepo.New("OWNER", "REPO"), - problemStatement: "Do the thing", - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), - httpmock.StatusStringResponse(201, createdJobTimeoutResponse), - ) - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/jobABC"), "api.githubcopilot.com"), - httpmock.StringResponse(`{"job_id":"jobABC"}`), - ) - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/jobABC"), "api.githubcopilot.com"), - httpmock.StatusStringResponse(500, `{"error":{"message":"something went wrong"}}`), - ) - }, - wantStdout: "job jobABC queued. View progress: https://github.com/copilot/agents\n", - }, + // Removed test case that previously expected fallback after polling error. } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ios, _, stdout, _ := iostreams.Test() + ios, _, stdout, stderr := iostreams.Test() opts := &CreateOptions{ IO: ios, ProblemStatement: tt.problemStatement, @@ -176,13 +176,16 @@ func Test_createRun(t *testing.T) { if tt.wantErr != "" { require.Error(t, err) - require.Equal(t, err.Error(), tt.wantErr) + require.Equal(t, tt.wantErr, err.Error()) } else { require.NoError(t, err) } if tt.wantStdout != "" { require.Equal(t, tt.wantStdout, stdout.String()) } + + require.Equal(t, tt.wantStdErr, stderr.String()) + if tt.stubs != nil { reg.Verify(t) } From 44e81b021c0436a895486fb0762869c0d9d35d05 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 12:36:12 -0600 Subject: [PATCH 13/15] Simplify stdout assertion in createRun test Removed unnecessary conditional check for wantStdout in Test_createRun. Now always asserts equality between wantStdout and actual stdout output, improving test clarity. --- pkg/cmd/agent-task/create/create_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index aec6831c4..79cf24631 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -180,10 +180,8 @@ func Test_createRun(t *testing.T) { } else { require.NoError(t, err) } - if tt.wantStdout != "" { - require.Equal(t, tt.wantStdout, stdout.String()) - } + require.Equal(t, tt.wantStdout, stdout.String()) require.Equal(t, tt.wantStdErr, stderr.String()) if tt.stubs != nil { From 21ccabc832f1c8e2ea88e7fb9678815e97dc330a Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 13:01:40 -0600 Subject: [PATCH 14/15] Simplify error handling in CreateJob response Refactored the CreateJob method to decode the response body into the Job struct before checking for error status codes. Error messages are now extracted directly from the Job struct, removing redundant error parsing logic. --- pkg/cmd/agent-task/capi/job.go | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/agent-task/capi/job.go b/pkg/cmd/agent-task/capi/job.go index ec215c5fe..03eaa376d 100644 --- a/pkg/cmd/agent-task/capi/job.go +++ b/pkg/cmd/agent-task/capi/job.go @@ -79,24 +79,16 @@ func (c *CAPIClient) CreateJob(ctx context.Context, owner, repo, problemStatemen return nil, err } defer res.Body.Close() - if res.StatusCode != http.StatusCreated && res.StatusCode != http.StatusOK { // accept 201 or 200 - // Attempt to parse error body for message - var er struct { - Error struct { - Message string `json:"message"` - } `json:"error"` - } - _ = json.NewDecoder(res.Body).Decode(&er) - msg := er.Error.Message - if msg == "" { - msg = res.Status - } - return nil, fmt.Errorf("failed to create job: %s", msg) - } + var j Job if err := json.NewDecoder(res.Body).Decode(&j); err != nil { return nil, fmt.Errorf("failed to decode create job response: %w", err) } + + if res.StatusCode != http.StatusCreated && res.StatusCode != http.StatusOK { // accept 201 or 200 + return nil, fmt.Errorf("failed to create job: %s", j.ErrorInfo.Message) + } + return &j, nil } From a821b408d41db3113a56bfd10666a649e604e494 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 3 Sep 2025 14:25:38 -0600 Subject: [PATCH 15/15] Update error messages and test repo handling in agent-task create Replaces 'problem statement' with 'task description' in error messages for clarity. Refactors tests to use a BaseRepo function instead of direct repo objects, and adds a test for missing task description error. --- pkg/cmd/agent-task/create/create.go | 6 ++--- pkg/cmd/agent-task/create/create_test.go | 28 ++++++++++++------------ 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index 61e3d54b9..41f615c3f 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -39,7 +39,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co // TODO: We'll support prompting for the problem statement if not provided // and from file flags, later. if len(args) == 0 { - return cmdutil.FlagErrorf("a problem statement is required") + return cmdutil.FlagErrorf("a task description is required") } opts.ProblemStatement = args[0] @@ -75,13 +75,13 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co func createRun(opts *CreateOptions) error { if opts.ProblemStatement == "" { - return cmdutil.FlagErrorf("a problem statement is required") + return cmdutil.FlagErrorf("a task description is required") } if opts.BaseRepo == nil { return errors.New("failed to resolve repository") } repo, err := opts.BaseRepo() - if err != nil || repo == nil || repo.RepoOwner() == "" || repo.RepoName() == "" { + if err != nil || repo == nil { // Not printing the error that came back from BaseRepo() here because we want // something clear, human friendly, and actionable. return fmt.Errorf("a repository is required; re-run in a repository or supply one with --repo owner/name") diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 79cf24631..977d32dfb 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -53,8 +53,7 @@ func Test_createRun(t *testing.T) { tests := []struct { name string stubs func(*httpmock.Registry) - baseRepo ghrepo.Interface - baseRepoErr error + baseRepoFunc func() (ghrepo.Interface, error) problemStatement string wantStdout string wantStdErr string @@ -62,7 +61,7 @@ func Test_createRun(t *testing.T) { }{ { name: "get job API failure surfaces error", - baseRepo: ghrepo.New("OWNER", "REPO"), + baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, problemStatement: "Do the thing", stubs: func(reg *httpmock.Registry) { reg.Register( @@ -79,7 +78,7 @@ func Test_createRun(t *testing.T) { }, { name: "success with immediate PR", - baseRepo: ghrepo.New("OWNER", "REPO"), + baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, problemStatement: "Do the thing", stubs: func(reg *httpmock.Registry) { reg.Register( @@ -91,7 +90,7 @@ func Test_createRun(t *testing.T) { }, { name: "success with delayed PR after polling", - baseRepo: ghrepo.New("OWNER", "REPO"), + baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, problemStatement: "Do the thing", stubs: func(reg *httpmock.Registry) { reg.Register( @@ -107,7 +106,7 @@ func Test_createRun(t *testing.T) { }, { name: "fallback after timeout returns link to global agents page", - baseRepo: ghrepo.New("OWNER", "REPO"), + baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, problemStatement: "Do the thing", stubs: func(reg *httpmock.Registry) { reg.Register( @@ -127,12 +126,12 @@ func Test_createRun(t *testing.T) { { name: "missing repo returns error", problemStatement: "task", - baseRepo: ghrepo.New("", ""), + 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: "create task API failure returns error", - baseRepo: ghrepo.New("OWNER", "REPO"), + baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, problemStatement: "do the thing", stubs: func(reg *httpmock.Registry) { reg.Register( @@ -142,7 +141,12 @@ func Test_createRun(t *testing.T) { }, wantErr: "failed to create job: some API error", }, - // Removed test case that previously expected fallback after polling error. + { + name: "missing task description returns error", + baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + problemStatement: "", + wantErr: "a task description is required", + }, } for _, tt := range tests { @@ -151,11 +155,7 @@ func Test_createRun(t *testing.T) { opts := &CreateOptions{ IO: ios, ProblemStatement: tt.problemStatement, - } - - if tt.baseRepo != nil || tt.baseRepoErr != nil { - br, bre := tt.baseRepo, tt.baseRepoErr - opts.BaseRepo = func() (ghrepo.Interface, error) { return br, bre } + BaseRepo: tt.baseRepoFunc, } // A backoff with no internal between retries to keep tests fast,