From 107edc3dd6f85515062362046d86f9bb37145478 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 3 Sep 2025 11:09:56 +0100 Subject: [PATCH 01/20] fix(agent-task/shared): add `CapiClientFunc` helper Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/shared/capi.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 pkg/cmd/agent-task/shared/capi.go diff --git a/pkg/cmd/agent-task/shared/capi.go b/pkg/cmd/agent-task/shared/capi.go new file mode 100644 index 000000000..f23ee86d2 --- /dev/null +++ b/pkg/cmd/agent-task/shared/capi.go @@ -0,0 +1,23 @@ +package shared + +import ( + "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" + "github.com/cli/cli/v2/pkg/cmdutil" +) + +func CapiClientFunc(f *cmdutil.Factory) func() (capi.CapiClient, error) { + return 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 + } +} From f3c3797d5c608f094393683773fc73c173831a6b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Wed, 3 Sep 2025 11:10:33 +0100 Subject: [PATCH 02/20] refactor(agent-task list): use shared `CapiClientFunc` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/create/create.go | 25 ++++++++++++------------- pkg/cmd/agent-task/list/list.go | 23 +++++------------------ pkg/cmd/agent-task/list/list_test.go | 2 +- 3 files changed, 18 insertions(+), 32 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index 03d526180..6a5d169be 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -14,6 +14,7 @@ import ( "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/cmd/agent-task/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" @@ -46,6 +47,12 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return err } + // 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 task description is required") + } + // Populate ProblemStatement from either arg or file if len(args) > 0 { opts.ProblemStatement = args[0] @@ -66,7 +73,12 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co // Support -R/--repo override if f != nil { opts.BaseRepo = f.BaseRepo + + if opts.CapiClient == nil { + opts.CapiClient = shared.CapiClientFunc(f) + } } + if runF != nil { return runF(opts) } @@ -93,19 +105,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().StringVarP(&fromFileName, "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)") - 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 } diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index c6ed68de2..9c19c61f6 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -6,7 +6,6 @@ import ( "time" "github.com/cli/cli/v2/internal/browser" - "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" @@ -23,9 +22,8 @@ const defaultLimit = 30 // ListOptions are the options for the list command type ListOptions struct { IO *iostreams.IOStreams - Config func() (gh.Config, error) Limit int - CapiClient func() (*capi.CAPIClient, error) + CapiClient func() (capi.CapiClient, error) BaseRepo func() (ghrepo.Interface, error) Web bool Browser browser.Browser @@ -35,7 +33,6 @@ type ListOptions struct { func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { opts := &ListOptions{ IO: f.IOStreams, - Config: f.Config, Limit: defaultLimit, Browser: f.Browser, } @@ -46,9 +43,12 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { // Support -R/--repo override - opts.BaseRepo = f.BaseRepo + if opts.CapiClient == nil { + opts.CapiClient = shared.CapiClientFunc(f) + } + if opts.Limit < 1 { return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit) } @@ -66,19 +66,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().IntVarP(&opts.Limit, "limit", "L", defaultLimit, fmt.Sprintf("Maximum number of agent tasks to fetch (default %d)", defaultLimit)) cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open agent tasks in the browser") - opts.CapiClient = func() (*capi.CAPIClient, error) { - cfg, err := opts.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 } diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index ef3cf8a7b..178704dc7 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -229,7 +229,7 @@ func Test_listRun(t *testing.T) { Limit: tt.limit, Web: tt.web, Browser: br, - CapiClient: func() (*capi.CAPIClient, error) { + CapiClient: func() (capi.CapiClient, error) { if tt.web { require.FailNow(t, "CapiClient was called with --web") } From 0138bf3dab50e797e5a0b6029cdaba46e0b9f8be Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 12:56:56 +0100 Subject: [PATCH 03/20] refactor(agent-task/capi): improve pagination Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 42 +++++++++++++++++------------ 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index 50ce36a1e..a3e1ded1e 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -16,7 +16,7 @@ import ( "github.com/vmihailenco/msgpack/v5" ) -const defaultSessionsPerPage = 50 +var defaultSessionsPerPage = 50 // session is an in-flight agent task type session struct { @@ -65,20 +65,23 @@ type Session struct { // ListSessionsForViewer lists all agent sessions for the // authenticated user up to limit. func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*Session, error) { + if limit == 0 { + return nil, nil + } + url := baseCAPIURL + "/agents/sessions" + pageSize := defaultSessionsPerPage - var sessions []session - page := 1 - perPage := defaultSessionsPerPage + sessions := make([]session, 0, limit+pageSize) - for { + for page := 1; ; page++ { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) if err != nil { return nil, err } q := req.URL.Query() - q.Set("page_size", strconv.Itoa(perPage)) + q.Set("page_size", strconv.Itoa(pageSize)) q.Set("page_number", strconv.Itoa(page)) req.URL.RawQuery = q.Encode() @@ -96,11 +99,11 @@ func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*S if err := json.NewDecoder(res.Body).Decode(&response); err != nil { return nil, fmt.Errorf("failed to decode sessions response: %w", err) } - if len(response.Sessions) == 0 || len(sessions) >= limit { + + sessions = append(sessions, response.Sessions...) + if len(response.Sessions) < pageSize || len(sessions) >= limit { break } - sessions = append(sessions, response.Sessions...) - page++ } // Drop any above the limit @@ -123,20 +126,23 @@ func (c *CAPIClient) ListSessionsForRepo(ctx context.Context, owner string, repo return nil, fmt.Errorf("owner and repo are required") } + if limit == 0 { + return nil, nil + } + url := fmt.Sprintf("%s/agents/sessions/nwo/%s/%s", baseCAPIURL, url.PathEscape(owner), url.PathEscape(repo)) + pageSize := defaultSessionsPerPage - var sessions []session - page := 1 - perPage := defaultSessionsPerPage + sessions := make([]session, 0, limit+pageSize) - for { + for page := 1; ; page++ { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) if err != nil { return nil, err } q := req.URL.Query() - q.Set("page_size", strconv.Itoa(perPage)) + q.Set("page_size", strconv.Itoa(pageSize)) q.Set("page_number", strconv.Itoa(page)) req.URL.RawQuery = q.Encode() @@ -154,11 +160,11 @@ func (c *CAPIClient) ListSessionsForRepo(ctx context.Context, owner string, repo if err := json.NewDecoder(res.Body).Decode(&response); err != nil { return nil, fmt.Errorf("failed to decode sessions response: %w", err) } - if len(response.Sessions) == 0 || len(sessions) >= limit { + + sessions = append(sessions, response.Sessions...) + if len(response.Sessions) < pageSize || len(sessions) >= limit { break } - sessions = append(sessions, response.Sessions...) - page++ } // Drop any above the limit @@ -194,10 +200,12 @@ func (c *CAPIClient) hydrateSessionPullRequests(sessions []session) ([]*Session, var resp struct { Nodes []struct { + TypeName string `graphql:"__typename"` PullRequest sessionPullRequest `graphql:"... on PullRequest"` } `graphql:"nodes(ids: $ids)"` } + // TODO handle pagination host, _ := c.authCfg.DefaultHost() err := apiClient.Query(host, "FetchPRsForAgentTaskSessions", &resp, map[string]any{ "ids": prNodeIds, From 77bb72c2d4be918138e1f28b0e6a94c764e61d7a Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 14:35:43 +0100 Subject: [PATCH 04/20] fix(agent-task/capi): improve returned errs Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index a3e1ded1e..eee4ca649 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -114,7 +114,7 @@ func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*S // Hydrate the result with pull request data. result, err := c.hydrateSessionPullRequests(sessions) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to fetch session resources: %w", err) } return result, nil @@ -175,7 +175,7 @@ func (c *CAPIClient) ListSessionsForRepo(ctx context.Context, owner string, repo // Hydrate the result with pull request data. result, err := c.hydrateSessionPullRequests(sessions) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to fetch session resources: %w", err) } return result, nil } From 0f5fd6ece0795a5e3619eb9658e913f9faa254d6 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 14:36:12 +0100 Subject: [PATCH 05/20] fix(agent-task/capi): handle non-JSON error response Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/job.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/capi/job.go b/pkg/cmd/agent-task/capi/job.go index 26bd3cf51..c45d972de 100644 --- a/pkg/cmd/agent-task/capi/job.go +++ b/pkg/cmd/agent-task/capi/job.go @@ -94,7 +94,11 @@ func (c *CAPIClient) CreateJob(ctx context.Context, owner, repo, problemStatemen } 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) + if j.ErrorInfo != nil { + return nil, fmt.Errorf("failed to create job: %s", j.ErrorInfo.Message) + } + statusText := fmt.Sprintf("%d %s", res.StatusCode, http.StatusText(res.StatusCode)) + return nil, fmt.Errorf("failed to create job: %s", statusText) } return &j, nil From 79602d3334dc6a27e8a29adcede727e45d49c7a7 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 14:37:37 +0100 Subject: [PATCH 06/20] test(agent-task/capi): add tests for session-related methods Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions_test.go | 822 +++++++++++++++++++++++ 1 file changed, 822 insertions(+) create mode 100644 pkg/cmd/agent-task/capi/sessions_test.go diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go new file mode 100644 index 000000000..2ac2dab6e --- /dev/null +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -0,0 +1,822 @@ +package capi + +import ( + "context" + "net/http" + "net/url" + "testing" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/api" + + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestListSessionsForViewer(t *testing.T) { + sampleDateString := "2025-08-29T00:00:00Z" + sampleDate, err := time.Parse(time.RFC3339, sampleDateString) + require.NoError(t, err) + + tests := []struct { + name string + perPage int + limit int + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + wantOut []*Session + }{ + { + name: "zero limit", + limit: 0, + wantOut: nil, + }, + { + name: "no sessions", + limit: 10, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ + "page_number": {"1"}, + "page_size": {"50"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(`{"sessions":[]}`), + ) + }, + wantOut: nil, + }, + { + name: "single session", + limit: 10, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ + "page_number": {"1"}, + "page_size": {"50"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sess1", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2000, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + // GraphQL hydration + reg.Register( + httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQLQuery(heredoc.Docf(` + { + "data": { + "nodes": [ + { + "__typename": "PullRequest", + "id": "PR_node", + "fullDatabaseId": "2000", + "number": 42, + "title": "Improve docs", + "state": "OPEN", + "url": "https://github.com/OWNER/REPO/pull/42", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "repository": { + "nameWithOwner": "OWNER/REPO" + } + } + ] + } + }`, + sampleDateString, + ), func(q string, vars map[string]interface{}) { + assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A"}, vars["ids"]) + }), + ) + }, + wantOut: []*Session{ + { + session: session{ + ID: "sess1", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2000, + CreatedAt: sampleDate, + }, + PullRequest: &api.PullRequest{ + ID: "PR_node", + FullDatabaseID: "2000", + Number: 42, + Title: "Improve docs", + State: "OPEN", + URL: "https://github.com/OWNER/REPO/pull/42", + Body: "", + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, + }, + { + name: "multiple sessions, paginated", + perPage: 1, // to enforce pagination + limit: 2, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ + "page_number": {"1"}, + "page_size": {"1"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sess1", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2000, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + + // Second page + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ + "page_number": {"2"}, + "page_size": {"1"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sess2", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2001, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + // GraphQL hydration + reg.Register( + httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQLQuery(heredoc.Docf(` + { + "data": { + "nodes": [ + { + "__typename": "PullRequest", + "id": "PR_node", + "fullDatabaseId": "2000", + "number": 42, + "title": "Improve docs", + "state": "OPEN", + "url": "https://github.com/OWNER/REPO/pull/42", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "repository": { + "nameWithOwner": "OWNER/REPO" + } + }, + { + "__typename": "PullRequest", + "id": "PR_node", + "fullDatabaseId": "2001", + "number": 43, + "title": "Improve docs", + "state": "OPEN", + "url": "https://github.com/OWNER/REPO/pull/43", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "repository": { + "nameWithOwner": "OWNER/REPO" + } + } + ] + } + }`, + sampleDateString, + ), func(q string, vars map[string]interface{}) { + assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "PR_kwDNA-jNB9E"}, vars["ids"]) + }), + ) + }, + wantOut: []*Session{ + { + session: session{ + ID: "sess1", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2000, + CreatedAt: sampleDate, + }, + PullRequest: &api.PullRequest{ + ID: "PR_node", + FullDatabaseID: "2000", + Number: 42, + Title: "Improve docs", + State: "OPEN", + URL: "https://github.com/OWNER/REPO/pull/42", + Body: "", + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + session: session{ + ID: "sess2", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2001, + CreatedAt: sampleDate, + }, + PullRequest: &api.PullRequest{ + ID: "PR_node", + FullDatabaseID: "2001", + Number: 43, + Title: "Improve docs", + State: "OPEN", + URL: "https://github.com/OWNER/REPO/pull/43", + Body: "", + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, + }, + { + name: "API error", + limit: 10, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ + "page_number": {"1"}, + "page_size": {"50"}, + }), + "api.githubcopilot.com", + ), + httpmock.StatusStringResponse(500, "{}"), + ) + }, + wantErr: "failed to list sessions:", + }, { + name: "API error at hydration", + limit: 10, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ + "page_number": {"1"}, + "page_size": {"50"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sess1", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2000, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + // GraphQL hydration + reg.Register( + httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.StatusStringResponse(500, `{}`), + ) + }, + wantErr: `failed to fetch session resources: non-200 OK status code:`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + if tt.httpStubs != nil { + tt.httpStubs(t, reg) + } + defer reg.Verify(t) + + httpClient := &http.Client{Transport: reg} + + cfg := config.NewBlankConfig() + capiClient := NewCAPIClient(httpClient, cfg.Authentication()) + + if tt.perPage != 0 { + last := defaultSessionsPerPage + defaultSessionsPerPage = tt.perPage + defer func() { + defaultSessionsPerPage = last + }() + } + + sessions, err := capiClient.ListSessionsForViewer(context.Background(), tt.limit) + + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + require.Nil(t, sessions) + return + } + + require.NoError(t, err) + require.Equal(t, tt.wantOut, sessions) + }) + } +} + +func TestListSessionForRepoRequiresRepo(t *testing.T) { + client := &CAPIClient{} + + _, err := client.ListSessionsForRepo(context.Background(), "", "only-repo", 0) + assert.EqualError(t, err, "owner and repo are required") + _, err = client.ListSessionsForRepo(context.Background(), "only-owner", "", 0) + assert.EqualError(t, err, "owner and repo are required") + _, err = client.ListSessionsForRepo(context.Background(), "", "", 0) + assert.EqualError(t, err, "owner and repo are required") +} + +func TestListSessionsForRepo(t *testing.T) { + sampleDateString := "2025-08-29T00:00:00Z" + sampleDate, err := time.Parse(time.RFC3339, sampleDateString) + require.NoError(t, err) + + tests := []struct { + name string + perPage int + limit int + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + wantOut []*Session + }{ + { + name: "zero limit", + limit: 0, + wantOut: nil, + }, + { + name: "no sessions", + limit: 10, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + "page_number": {"1"}, + "page_size": {"50"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(`{"sessions":[]}`), + ) + }, + wantOut: nil, + }, + { + name: "single session", + limit: 10, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + "page_number": {"1"}, + "page_size": {"50"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sess1", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2000, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + // GraphQL hydration + reg.Register( + httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQLQuery(heredoc.Docf(` + { + "data": { + "nodes": [ + { + "__typename": "PullRequest", + "id": "PR_node", + "fullDatabaseId": "2000", + "number": 42, + "title": "Improve docs", + "state": "OPEN", + "url": "https://github.com/OWNER/REPO/pull/42", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "repository": { + "nameWithOwner": "OWNER/REPO" + } + } + ] + } + }`, + sampleDateString, + ), func(q string, vars map[string]interface{}) { + assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A"}, vars["ids"]) + }), + ) + }, + wantOut: []*Session{ + { + session: session{ + ID: "sess1", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2000, + CreatedAt: sampleDate, + }, + PullRequest: &api.PullRequest{ + ID: "PR_node", + FullDatabaseID: "2000", + Number: 42, + Title: "Improve docs", + State: "OPEN", + URL: "https://github.com/OWNER/REPO/pull/42", + Body: "", + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, + }, + { + name: "multiple sessions, paginated", + perPage: 1, // to enforce pagination + limit: 2, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + "page_number": {"1"}, + "page_size": {"1"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sess1", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2000, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + + // Second page + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + "page_number": {"2"}, + "page_size": {"1"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sess2", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2001, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + // GraphQL hydration + reg.Register( + httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQLQuery(heredoc.Docf(` + { + "data": { + "nodes": [ + { + "__typename": "PullRequest", + "id": "PR_node", + "fullDatabaseId": "2000", + "number": 42, + "title": "Improve docs", + "state": "OPEN", + "url": "https://github.com/OWNER/REPO/pull/42", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "repository": { + "nameWithOwner": "OWNER/REPO" + } + }, + { + "__typename": "PullRequest", + "id": "PR_node", + "fullDatabaseId": "2001", + "number": 43, + "title": "Improve docs", + "state": "OPEN", + "url": "https://github.com/OWNER/REPO/pull/43", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "repository": { + "nameWithOwner": "OWNER/REPO" + } + } + ] + } + }`, + sampleDateString, + ), func(q string, vars map[string]interface{}) { + assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "PR_kwDNA-jNB9E"}, vars["ids"]) + }), + ) + }, + wantOut: []*Session{ + { + session: session{ + ID: "sess1", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2000, + CreatedAt: sampleDate, + }, + PullRequest: &api.PullRequest{ + ID: "PR_node", + FullDatabaseID: "2000", + Number: 42, + Title: "Improve docs", + State: "OPEN", + URL: "https://github.com/OWNER/REPO/pull/42", + Body: "", + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + session: session{ + ID: "sess2", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2001, + CreatedAt: sampleDate, + }, + PullRequest: &api.PullRequest{ + ID: "PR_node", + FullDatabaseID: "2001", + Number: 43, + Title: "Improve docs", + State: "OPEN", + URL: "https://github.com/OWNER/REPO/pull/43", + Body: "", + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, + }, + { + name: "API error", + limit: 10, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + "page_number": {"1"}, + "page_size": {"50"}, + }), + "api.githubcopilot.com", + ), + httpmock.StatusStringResponse(500, "{}"), + ) + }, + wantErr: "failed to list sessions:", + }, { + name: "API error at hydration", + limit: 10, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + "page_number": {"1"}, + "page_size": {"50"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sess1", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2000, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + // GraphQL hydration + reg.Register( + httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.StatusStringResponse(500, `{}`), + ) + }, + wantErr: `failed to fetch session resources: non-200 OK status code:`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + if tt.httpStubs != nil { + tt.httpStubs(t, reg) + } + defer reg.Verify(t) + + httpClient := &http.Client{Transport: reg} + + cfg := config.NewBlankConfig() + capiClient := NewCAPIClient(httpClient, cfg.Authentication()) + + if tt.perPage != 0 { + last := defaultSessionsPerPage + defaultSessionsPerPage = tt.perPage + defer func() { + defaultSessionsPerPage = last + }() + } + + sessions, err := capiClient.ListSessionsForRepo(context.Background(), "OWNER", "REPO", tt.limit) + + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + require.Nil(t, sessions) + return + } + + require.NoError(t, err) + require.Equal(t, tt.wantOut, sessions) + }) + } +} From be8e6f64919205bdcae96a3dd24929956940c6ba Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 14:37:59 +0100 Subject: [PATCH 07/20] test(agent-task/capi): add tests for job-related methods Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/job_test.go | 369 ++++++++++++++++++++++++++++ 1 file changed, 369 insertions(+) create mode 100644 pkg/cmd/agent-task/capi/job_test.go diff --git a/pkg/cmd/agent-task/capi/job_test.go b/pkg/cmd/agent-task/capi/job_test.go new file mode 100644 index 000000000..987b43b9f --- /dev/null +++ b/pkg/cmd/agent-task/capi/job_test.go @@ -0,0 +1,369 @@ +package capi + +import ( + "context" + "net/http" + "testing" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetJobRequiresRepoAndJobID(t *testing.T) { + client := &CAPIClient{} + _, err := client.GetJob(context.Background(), "", "", "only-job-id") + assert.EqualError(t, err, "owner, repo, and jobID are required") + _, err = client.GetJob(context.Background(), "", "only-repo", "") + assert.EqualError(t, err, "owner, repo, and jobID are required") + _, err = client.GetJob(context.Background(), "only-owner", "", "") + assert.EqualError(t, err, "owner, repo, and jobID are required") + _, err = client.GetJob(context.Background(), "", "", "") + assert.EqualError(t, err, "owner, repo, and jobID are required") +} + +func TestGetJob(t *testing.T) { + sampleDateString := "2025-08-29T00:00:00Z" + sampleDate, err := time.Parse(time.RFC3339, sampleDateString) + require.NoError(t, err) + + tests := []struct { + name string + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + wantOut *Job + }{ + { + name: "job without PR", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/job123"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(200, heredoc.Docf(` + { + "job_id": "job123", + "session_id": "sess1", + "problem_statement": "Do the thing", + "event_type": "foo", + "content_filter_mode": "foo", + "status": "foo", + "result": "foo", + "actor": { + "id": 1, + "login": "octocat" + }, + "created_at": "%[1]s", + "updated_at": "%[1]s" + }`, + sampleDateString, + )), + ) + }, + wantOut: &Job{ + ID: "job123", + SessionID: "sess1", + ProblemStatement: "Do the thing", + EventType: "foo", + ContentFilterMode: "foo", + Status: "foo", + Result: "foo", + Actor: &JobActor{ + ID: 1, + Login: "octocat", + }, + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + }, + }, + { + name: "job with PR", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/job123"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(200, heredoc.Docf(` + { + "job_id": "job123", + "session_id": "sess1", + "problem_statement": "Do the thing", + "event_type": "foo", + "content_filter_mode": "foo", + "status": "foo", + "result": "foo", + "actor": { + "id": 1, + "login": "octocat" + }, + "created_at": "%[1]s", + "updated_at": "%[1]s", + "pull_request": { + "id": 101, + "number": 42 + } + }`, + sampleDateString, + )), + ) + }, + wantOut: &Job{ + ID: "job123", + SessionID: "sess1", + ProblemStatement: "Do the thing", + EventType: "foo", + ContentFilterMode: "foo", + Status: "foo", + Result: "foo", + Actor: &JobActor{ + ID: 1, + Login: "octocat", + }, + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + PullRequest: &JobPullRequest{ + ID: 101, + Number: 42, + }, + }, + }, + { + name: "job not found", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/job123"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(404, `{}`), + ) + }, + wantErr: "failed to get job: 404 Not Found", + }, + { + name: "API error", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/job123"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(500, `{}`), + ) + }, + wantErr: "failed to get job: 500 Internal Server Error", + }, + { + name: "invalid JSON response", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/swe/v1/jobs/OWNER/REPO/job123"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(200, ``), + ) + }, + wantErr: "failed to decode get job response: EOF", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + if tt.httpStubs != nil { + tt.httpStubs(t, reg) + } + defer reg.Verify(t) + + httpClient := &http.Client{Transport: reg} + + cfg := config.NewBlankConfig() + capiClient := NewCAPIClient(httpClient, cfg.Authentication()) + + job, err := capiClient.GetJob(context.Background(), "OWNER", "REPO", "job123") + + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + require.Nil(t, job) + return + } + + require.NoError(t, err) + require.Equal(t, tt.wantOut, job) + }) + } +} + +func TestCreateJobRequiresRepoAndProblemStatement(t *testing.T) { + client := &CAPIClient{} + + _, err := client.CreateJob(context.Background(), "", "only-repo", "", "") + assert.EqualError(t, err, "owner and repo are required") + _, err = client.CreateJob(context.Background(), "only-owner", "", "", "") + assert.EqualError(t, err, "owner and repo are required") + _, err = client.CreateJob(context.Background(), "", "", "", "") + assert.EqualError(t, err, "owner and repo are required") + + _, err = client.CreateJob(context.Background(), "owner", "repo", "", "") + assert.EqualError(t, err, "problem statement is required") +} + +func TestCreateJob(t *testing.T) { + sampleDateString := "2025-08-29T00:00:00Z" + sampleDate, err := time.Parse(time.RFC3339, sampleDateString) + require.NoError(t, err) + + tests := []struct { + name string + baseBranch string + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + wantOut *Job + }{ + { + name: "success", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.RESTPayload(201, + heredoc.Docf(` + { + "job_id": "job123", + "session_id": "sess1", + "problem_statement": "Do the thing", + "event_type": "foo", + "content_filter_mode": "foo", + "status": "foo", + "result": "foo", + "actor": { + "id": 1, + "login": "octocat" + }, + "created_at": "%[1]s", + "updated_at": "%[1]s" + } + `, sampleDateString), + func(payload map[string]interface{}) { + assert.Equal(t, "Do the thing", payload["problem_statement"]) + assert.Equal(t, "gh_cli", payload["event_type"]) + }, + ), + ) + }, + wantOut: &Job{ + ID: "job123", + SessionID: "sess1", + ProblemStatement: "Do the thing", + EventType: "foo", + ContentFilterMode: "foo", + Status: "foo", + Result: "foo", + Actor: &JobActor{ + ID: 1, + Login: "octocat", + }, + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + }, + }, + { + name: "success with base branch", + baseBranch: "some-branch", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.RESTPayload(201, + heredoc.Docf(` + { + "job_id": "job123", + "session_id": "sess1", + "problem_statement": "Do the thing", + "event_type": "foo", + "content_filter_mode": "foo", + "status": "foo", + "result": "foo", + "actor": { + "id": 1, + "login": "octocat" + }, + "created_at": "%[1]s", + "updated_at": "%[1]s" + } + `, sampleDateString), + func(payload map[string]interface{}) { + assert.Equal(t, "Do the thing", payload["problem_statement"]) + assert.Equal(t, "gh_cli", payload["event_type"]) + assert.Equal(t, "refs/heads/some-branch", payload["pull_request"].(map[string]interface{})["base_ref"]) + }, + ), + ) + }, + wantOut: &Job{ + ID: "job123", + SessionID: "sess1", + ProblemStatement: "Do the thing", + EventType: "foo", + ContentFilterMode: "foo", + Status: "foo", + Result: "foo", + Actor: &JobActor{ + ID: 1, + Login: "octocat", + }, + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + }, + }, + { + name: "API error, included in response body", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(500, heredoc.Doc(`{ + "error": { + "message": "some error" + } + }`)), + ) + }, + wantErr: "failed to create job: some error", + }, + { + name: "API error", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(500, `{}`), + ) + }, + wantErr: "failed to create job: 500 Internal Server Error", + }, + { + name: "invalid JSON response", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(200, ``), + ) + }, + wantErr: "failed to decode create job response: EOF", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + if tt.httpStubs != nil { + tt.httpStubs(t, reg) + } + defer reg.Verify(t) + + httpClient := &http.Client{Transport: reg} + + cfg := config.NewBlankConfig() + capiClient := NewCAPIClient(httpClient, cfg.Authentication()) + + job, err := capiClient.CreateJob(context.Background(), "OWNER", "REPO", "Do the thing", tt.baseBranch) + + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + require.Nil(t, job) + return + } + + require.NoError(t, err) + require.Equal(t, tt.wantOut, job) + }) + } +} From ab99ee530aca664e7c0fd9efc1ea06c36cfaa346 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 14:38:39 +0100 Subject: [PATCH 08/20] test(agent-task/capi): add `go:generate` directive to gen mock Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/client.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/agent-task/capi/client.go b/pkg/cmd/agent-task/capi/client.go index 1e9cad3c8..ecec9a024 100644 --- a/pkg/cmd/agent-task/capi/client.go +++ b/pkg/cmd/agent-task/capi/client.go @@ -7,6 +7,8 @@ import ( "github.com/cli/cli/v2/internal/gh" ) +//go:generate moq -rm -out client_mock.go . CapiClient + const baseCAPIURL = "https://api.githubcopilot.com" const capiHost = "api.githubcopilot.com" From 585b6392736ec024b2e80fefab52a0d6c85b352d Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 14:39:12 +0100 Subject: [PATCH 09/20] test(agent-task/capi): add `CapiClientMock` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/client_mock.go | 273 +++++++++++++++++++++++++ 1 file changed, 273 insertions(+) create mode 100644 pkg/cmd/agent-task/capi/client_mock.go diff --git a/pkg/cmd/agent-task/capi/client_mock.go b/pkg/cmd/agent-task/capi/client_mock.go new file mode 100644 index 000000000..621585587 --- /dev/null +++ b/pkg/cmd/agent-task/capi/client_mock.go @@ -0,0 +1,273 @@ +// Code generated by moq; DO NOT EDIT. +// github.com/matryer/moq + +package capi + +import ( + "context" + "sync" +) + +// Ensure, that CapiClientMock does implement CapiClient. +// If this is not the case, regenerate this file with moq. +var _ CapiClient = &CapiClientMock{} + +// CapiClientMock is a mock implementation of CapiClient. +// +// func TestSomethingThatUsesCapiClient(t *testing.T) { +// +// // make and configure a mocked CapiClient +// mockedCapiClient := &CapiClientMock{ +// CreateJobFunc: func(ctx context.Context, owner string, repo string, problemStatement string, baseBranch string) (*Job, error) { +// panic("mock out the CreateJob method") +// }, +// GetJobFunc: func(ctx context.Context, owner string, repo string, jobID string) (*Job, error) { +// panic("mock out the GetJob method") +// }, +// ListSessionsForRepoFunc: func(ctx context.Context, owner string, repo string, limit int) ([]*Session, error) { +// panic("mock out the ListSessionsForRepo method") +// }, +// ListSessionsForViewerFunc: func(ctx context.Context, limit int) ([]*Session, error) { +// panic("mock out the ListSessionsForViewer method") +// }, +// } +// +// // use mockedCapiClient in code that requires CapiClient +// // and then make assertions. +// +// } +type CapiClientMock struct { + // CreateJobFunc mocks the CreateJob method. + CreateJobFunc func(ctx context.Context, owner string, repo string, problemStatement string, baseBranch string) (*Job, error) + + // GetJobFunc mocks the GetJob method. + GetJobFunc func(ctx context.Context, owner string, repo string, jobID string) (*Job, error) + + // ListSessionsForRepoFunc mocks the ListSessionsForRepo method. + ListSessionsForRepoFunc func(ctx context.Context, owner string, repo string, limit int) ([]*Session, error) + + // ListSessionsForViewerFunc mocks the ListSessionsForViewer method. + ListSessionsForViewerFunc func(ctx context.Context, limit int) ([]*Session, error) + + // calls tracks calls to the methods. + calls struct { + // CreateJob holds details about calls to the CreateJob method. + CreateJob []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Owner is the owner argument value. + Owner string + // Repo is the repo argument value. + Repo string + // ProblemStatement is the problemStatement argument value. + ProblemStatement string + // BaseBranch is the baseBranch argument value. + BaseBranch string + } + // GetJob holds details about calls to the GetJob method. + GetJob []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Owner is the owner argument value. + Owner string + // Repo is the repo argument value. + Repo string + // JobID is the jobID argument value. + JobID string + } + // ListSessionsForRepo holds details about calls to the ListSessionsForRepo method. + ListSessionsForRepo []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Owner is the owner argument value. + Owner string + // Repo is the repo argument value. + Repo string + // Limit is the limit argument value. + Limit int + } + // ListSessionsForViewer holds details about calls to the ListSessionsForViewer method. + ListSessionsForViewer []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Limit is the limit argument value. + Limit int + } + } + lockCreateJob sync.RWMutex + lockGetJob sync.RWMutex + lockListSessionsForRepo sync.RWMutex + lockListSessionsForViewer sync.RWMutex +} + +// CreateJob calls CreateJobFunc. +func (mock *CapiClientMock) CreateJob(ctx context.Context, owner string, repo string, problemStatement string, baseBranch string) (*Job, error) { + if mock.CreateJobFunc == nil { + panic("CapiClientMock.CreateJobFunc: method is nil but CapiClient.CreateJob was just called") + } + callInfo := struct { + Ctx context.Context + Owner string + Repo string + ProblemStatement string + BaseBranch string + }{ + Ctx: ctx, + Owner: owner, + Repo: repo, + ProblemStatement: problemStatement, + BaseBranch: baseBranch, + } + mock.lockCreateJob.Lock() + mock.calls.CreateJob = append(mock.calls.CreateJob, callInfo) + mock.lockCreateJob.Unlock() + return mock.CreateJobFunc(ctx, owner, repo, problemStatement, baseBranch) +} + +// CreateJobCalls gets all the calls that were made to CreateJob. +// Check the length with: +// +// len(mockedCapiClient.CreateJobCalls()) +func (mock *CapiClientMock) CreateJobCalls() []struct { + Ctx context.Context + Owner string + Repo string + ProblemStatement string + BaseBranch string +} { + var calls []struct { + Ctx context.Context + Owner string + Repo string + ProblemStatement string + BaseBranch string + } + mock.lockCreateJob.RLock() + calls = mock.calls.CreateJob + mock.lockCreateJob.RUnlock() + return calls +} + +// GetJob calls GetJobFunc. +func (mock *CapiClientMock) GetJob(ctx context.Context, owner string, repo string, jobID string) (*Job, error) { + if mock.GetJobFunc == nil { + panic("CapiClientMock.GetJobFunc: method is nil but CapiClient.GetJob was just called") + } + callInfo := struct { + Ctx context.Context + Owner string + Repo string + JobID string + }{ + Ctx: ctx, + Owner: owner, + Repo: repo, + JobID: jobID, + } + mock.lockGetJob.Lock() + mock.calls.GetJob = append(mock.calls.GetJob, callInfo) + mock.lockGetJob.Unlock() + return mock.GetJobFunc(ctx, owner, repo, jobID) +} + +// GetJobCalls gets all the calls that were made to GetJob. +// Check the length with: +// +// len(mockedCapiClient.GetJobCalls()) +func (mock *CapiClientMock) GetJobCalls() []struct { + Ctx context.Context + Owner string + Repo string + JobID string +} { + var calls []struct { + Ctx context.Context + Owner string + Repo string + JobID string + } + mock.lockGetJob.RLock() + calls = mock.calls.GetJob + mock.lockGetJob.RUnlock() + return calls +} + +// ListSessionsForRepo calls ListSessionsForRepoFunc. +func (mock *CapiClientMock) ListSessionsForRepo(ctx context.Context, owner string, repo string, limit int) ([]*Session, error) { + if mock.ListSessionsForRepoFunc == nil { + panic("CapiClientMock.ListSessionsForRepoFunc: method is nil but CapiClient.ListSessionsForRepo was just called") + } + callInfo := struct { + Ctx context.Context + Owner string + Repo string + Limit int + }{ + Ctx: ctx, + Owner: owner, + Repo: repo, + Limit: limit, + } + mock.lockListSessionsForRepo.Lock() + mock.calls.ListSessionsForRepo = append(mock.calls.ListSessionsForRepo, callInfo) + mock.lockListSessionsForRepo.Unlock() + return mock.ListSessionsForRepoFunc(ctx, owner, repo, limit) +} + +// ListSessionsForRepoCalls gets all the calls that were made to ListSessionsForRepo. +// Check the length with: +// +// len(mockedCapiClient.ListSessionsForRepoCalls()) +func (mock *CapiClientMock) ListSessionsForRepoCalls() []struct { + Ctx context.Context + Owner string + Repo string + Limit int +} { + var calls []struct { + Ctx context.Context + Owner string + Repo string + Limit int + } + mock.lockListSessionsForRepo.RLock() + calls = mock.calls.ListSessionsForRepo + mock.lockListSessionsForRepo.RUnlock() + return calls +} + +// ListSessionsForViewer calls ListSessionsForViewerFunc. +func (mock *CapiClientMock) ListSessionsForViewer(ctx context.Context, limit int) ([]*Session, error) { + if mock.ListSessionsForViewerFunc == nil { + panic("CapiClientMock.ListSessionsForViewerFunc: method is nil but CapiClient.ListSessionsForViewer was just called") + } + callInfo := struct { + Ctx context.Context + Limit int + }{ + Ctx: ctx, + Limit: limit, + } + mock.lockListSessionsForViewer.Lock() + mock.calls.ListSessionsForViewer = append(mock.calls.ListSessionsForViewer, callInfo) + mock.lockListSessionsForViewer.Unlock() + return mock.ListSessionsForViewerFunc(ctx, limit) +} + +// ListSessionsForViewerCalls gets all the calls that were made to ListSessionsForViewer. +// Check the length with: +// +// len(mockedCapiClient.ListSessionsForViewerCalls()) +func (mock *CapiClientMock) ListSessionsForViewerCalls() []struct { + Ctx context.Context + Limit int +} { + var calls []struct { + Ctx context.Context + Limit int + } + mock.lockListSessionsForViewer.RLock() + calls = mock.calls.ListSessionsForViewer + mock.lockListSessionsForViewer.RUnlock() + return calls +} From 19e17c54fd4c882ff2e96b93654fd6d49cd00864 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 18:47:24 +0100 Subject: [PATCH 10/20] refactor(agent-task/capi): drop embedding of unexported struct Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 46 ++++++- pkg/cmd/agent-task/capi/sessions_test.go | 145 +++++++++++------------ 2 files changed, 107 insertions(+), 84 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index eee4ca649..8bf29575c 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -58,8 +58,23 @@ type sessionPullRequest struct { // Session is a hydrated in-flight agent task type Session struct { - session - PullRequest *api.PullRequest `json:"-"` + ID string + Name string + UserID uint64 + AgentID int64 + Logs string + State string + OwnerID uint64 + RepoID uint64 + ResourceType string + ResourceID int64 + LastUpdatedAt time.Time + CreatedAt time.Time + CompletedAt time.Time + EventURL string + EventType string + + PullRequest *api.PullRequest } // ListSessionsForViewer lists all agent sessions for the @@ -235,10 +250,9 @@ func (c *CAPIClient) hydrateSessionPullRequests(sessions []session) ([]*Session, newSessions := make([]*Session, 0, len(sessions)) for _, s := range sessions { - newSessions = append(newSessions, &Session{ - session: s, - PullRequest: prMap[strconv.FormatInt(s.ResourceID, 10)], - }) + newSession := fromAPISession(s) + newSession.PullRequest = prMap[strconv.FormatInt(s.ResourceID, 10)] + newSessions = append(newSessions, newSession) } return newSessions, nil @@ -261,3 +275,23 @@ func generatePullRequestNodeID(repoID, pullRequestID int64) string { return "PR_" + encoded } + +func fromAPISession(s session) *Session { + return &Session{ + ID: s.ID, + Name: s.Name, + UserID: s.UserID, + AgentID: s.AgentID, + Logs: s.Logs, + State: s.State, + OwnerID: s.OwnerID, + RepoID: s.RepoID, + ResourceType: s.ResourceType, + ResourceID: s.ResourceID, + LastUpdatedAt: s.LastUpdatedAt, + CreatedAt: s.CreatedAt, + CompletedAt: s.CompletedAt, + EventURL: s.EventURL, + EventType: s.EventType, + } +} diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 2ac2dab6e..20b670d56 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -117,19 +117,18 @@ func TestListSessionsForViewer(t *testing.T) { }, wantOut: []*Session{ { - session: session{ - ID: "sess1", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "pull", - ResourceID: 2000, - CreatedAt: sampleDate, - }, + + ID: "sess1", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2000, + CreatedAt: sampleDate, PullRequest: &api.PullRequest{ ID: "PR_node", FullDatabaseID: "2000", @@ -260,19 +259,17 @@ func TestListSessionsForViewer(t *testing.T) { }, wantOut: []*Session{ { - session: session{ - ID: "sess1", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "pull", - ResourceID: 2000, - CreatedAt: sampleDate, - }, + ID: "sess1", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2000, + CreatedAt: sampleDate, PullRequest: &api.PullRequest{ ID: "PR_node", FullDatabaseID: "2000", @@ -289,19 +286,17 @@ func TestListSessionsForViewer(t *testing.T) { }, }, { - session: session{ - ID: "sess2", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "pull", - ResourceID: 2001, - CreatedAt: sampleDate, - }, + ID: "sess2", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2001, + CreatedAt: sampleDate, PullRequest: &api.PullRequest{ ID: "PR_node", FullDatabaseID: "2001", @@ -525,19 +520,17 @@ func TestListSessionsForRepo(t *testing.T) { }, wantOut: []*Session{ { - session: session{ - ID: "sess1", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "pull", - ResourceID: 2000, - CreatedAt: sampleDate, - }, + ID: "sess1", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2000, + CreatedAt: sampleDate, PullRequest: &api.PullRequest{ ID: "PR_node", FullDatabaseID: "2000", @@ -668,19 +661,17 @@ func TestListSessionsForRepo(t *testing.T) { }, wantOut: []*Session{ { - session: session{ - ID: "sess1", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "pull", - ResourceID: 2000, - CreatedAt: sampleDate, - }, + ID: "sess1", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2000, + CreatedAt: sampleDate, PullRequest: &api.PullRequest{ ID: "PR_node", FullDatabaseID: "2000", @@ -697,19 +688,17 @@ func TestListSessionsForRepo(t *testing.T) { }, }, { - session: session{ - ID: "sess2", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "pull", - ResourceID: 2001, - CreatedAt: sampleDate, - }, + ID: "sess2", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2001, + CreatedAt: sampleDate, PullRequest: &api.PullRequest{ ID: "PR_node", FullDatabaseID: "2001", From 27d9a0d5fc344b1af2034ffc46523a7a6a29c607 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 19:33:19 +0100 Subject: [PATCH 11/20] test(agent-task list): update `TestNewCmdList` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/list/list_test.go | 41 +++++++++++++++++++++------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index 178704dc7..2185c5393 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -1,31 +1,31 @@ package list import ( + "context" "errors" - "net/http" - "strings" "testing" "time" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/browser" - "github.com/cli/cli/v2/internal/config" - "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/httpmock" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestNewCmdList(t *testing.T) { tests := []struct { - name string - args string - wantOpts ListOptions - wantErr string + name string + args string + wantOpts ListOptions + wantBaseRepo ghrepo.Interface + wantErr string }{ { name: "no arguments", @@ -33,6 +33,14 @@ func TestNewCmdList(t *testing.T) { Limit: defaultLimit, }, }, + { + name: "base repo specified", + args: "--repo OWNER/REPO", + wantOpts: ListOptions{ + Limit: defaultLimit, + }, + wantBaseRepo: ghrepo.New("OWNER", "REPO"), + }, { name: "custom limit", args: "--limit 15", @@ -62,11 +70,18 @@ func TestNewCmdList(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: ios, + } + var gotOpts *ListOptions cmd := NewCmdList(f, func(opts *ListOptions) error { gotOpts = opts; return nil }) + if tt.args != "" { - cmd.SetArgs(strings.Split(tt.args, " ")) + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) } _, err := cmd.ExecuteC() if tt.wantErr != "" { @@ -77,6 +92,12 @@ func TestNewCmdList(t *testing.T) { require.NoError(t, err) assert.Equal(t, tt.wantOpts.Limit, gotOpts.Limit) assert.Equal(t, tt.wantOpts.Web, gotOpts.Web) + + if tt.wantBaseRepo != nil { + baseRepo, err := gotOpts.BaseRepo() + require.NoError(t, err) + assert.True(t, ghrepo.IsSame(tt.wantBaseRepo, baseRepo)) + } }) } } From c708e58f694090ea21be84ee9fd1c54b9a9637e2 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 19:33:47 +0100 Subject: [PATCH 12/20] test(agent-task list): use `CapiClientMock` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/list/list_test.go | 906 ++++++++++----------------- 1 file changed, 324 insertions(+), 582 deletions(-) diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index 2185c5393..5ed0d097d 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -103,12 +103,14 @@ func TestNewCmdList(t *testing.T) { } func Test_listRun(t *testing.T) { - createdAt := time.Now().Add(-6 * time.Hour).Format(time.RFC3339) // 6h ago + sampleDate := time.Now().Add(-6 * time.Hour) // 6h ago + sampleDateString := sampleDate.Format(time.RFC3339) tests := []struct { name string tty bool stubs func(*httpmock.Registry) + capiStubs func(*testing.T, *capi.CapiClientMock) baseRepo ghrepo.Interface baseRepoErr error limit int @@ -119,90 +121,347 @@ func Test_listRun(t *testing.T) { wantBrowserURL string }{ { - name: "no sessions", - tty: true, - stubs: func(reg *httpmock.Registry) { registerEmptySessionsMock(reg) }, + name: "viewer-scoped no sessions", + tty: true, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + return nil, nil + } + }, wantErr: cmdutil.NewNoResultsError("no agent tasks found"), }, { - name: "limit truncates sessions", + name: "viewer-scoped respects --limit", tty: true, - limit: 3, - stubs: func(reg *httpmock.Registry) { registerManySessionsMock(reg, createdAt) }, + limit: 999, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + assert.Equal(t, 999, limit) + return nil, nil + } + }, + wantErr: cmdutil.NewNoResultsError("no agent tasks found"), // not important + }, + { + name: "viewer-scoped single session (tty)", + tty: true, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + return []*capi.Session{ + { + ID: "s1", + State: "completed", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 101, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, nil + } + }, wantOut: heredoc.Doc(` - SESSION ID PULL REQUEST REPO SESSION STATE CREATED - s1 #101 OWNER/REPO completed about 6 hours ago - s2 #102 OWNER/REPO failed about 6 hours ago - s3 #103 OWNER/REPO in_progress about 6 hours ago + SESSION ID PULL REQUEST REPO SESSION STATE CREATED + s1 #101 OWNER/REPO completed about 6 hours ago `), }, { - name: "single session (tty)", - tty: true, - stubs: func(reg *httpmock.Registry) { registerSingleSessionMock(reg, createdAt) }, + name: "viewer-scoped single session (nontty)", + tty: false, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + return []*capi.Session{ + { + ID: "s1", + State: "completed", + ResourceType: "pull", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Number: 101, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, nil + } + }, + wantOut: "s1\t#101\tOWNER/REPO\tcompleted\t" + sampleDateString + "\n", // header omitted for non-tty + }, + { + name: "viewer-scoped many sessions (tty)", + tty: true, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + return []*capi.Session{ + { + ID: "s1", + State: "completed", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 101, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s2", + State: "failed", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 102, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s3", + State: "in_progress", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 103, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s4", + State: "queued", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 104, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s5", + State: "canceled", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 105, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s6", + State: "mystery", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 106, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, nil + } + }, wantOut: heredoc.Doc(` - SESSION ID PULL REQUEST REPO SESSION STATE CREATED - sess1 #42 OWNER/REPO completed about 6 hours ago + SESSION ID PULL REQUEST REPO SESSION STATE CREATED + s1 #101 OWNER/REPO completed about 6 hours ago + s2 #102 OWNER/REPO failed about 6 hours ago + s3 #103 OWNER/REPO in_progress about 6 hours ago + s4 #104 OWNER/REPO queued about 6 hours ago + s5 #105 OWNER/REPO canceled about 6 hours ago + s6 #106 OWNER/REPO mystery about 6 hours ago `), }, { - name: "single session (nontty)", - tty: false, - stubs: func(reg *httpmock.Registry) { registerSingleSessionMock(reg, createdAt) }, - wantOut: "sess1\t#42\tOWNER/REPO\tcompleted\t" + createdAt + "\n", // header omitted for non-tty - }, - { - name: "many sessions (tty)", - tty: true, - stubs: func(reg *httpmock.Registry) { registerManySessionsMock(reg, createdAt) }, - wantOut: heredoc.Doc(` - SESSION ID PULL REQUEST REPO SESSION STATE CREATED - s1 #101 OWNER/REPO completed about 6 hours ago - s2 #102 OWNER/REPO failed about 6 hours ago - s3 #103 OWNER/REPO in_progress about 6 hours ago - s4 #104 OWNER/REPO queued about 6 hours ago - s5 #105 OWNER/REPO canceled about 6 hours ago - s6 #106 OWNER/REPO mystery about 6 hours ago - `), - }, - { - name: "repo scoped single session", + name: "repo-scoped no sessions", tty: true, - stubs: func(reg *httpmock.Registry) { registerRepoSingleSessionMock(reg, createdAt, "OWNER", "REPO") }, baseRepo: ghrepo.New("OWNER", "REPO"), + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForRepoFunc = func(ctx context.Context, owner, repo string, limit int) ([]*capi.Session, error) { + return nil, nil + } + }, + wantErr: cmdutil.NewNoResultsError("no agent tasks found"), + }, + { + name: "repo-scoped respects --limit/--repo", + tty: true, + limit: 999, + baseRepo: ghrepo.New("OWNER", "REPO"), + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForRepoFunc = func(ctx context.Context, owner, repo string, limit int) ([]*capi.Session, error) { + assert.Equal(t, 999, limit) + assert.Equal(t, "OWNER", owner) + assert.Equal(t, "REPO", repo) + return nil, nil + } + }, + wantErr: cmdutil.NewNoResultsError("no agent tasks found"), // not important + }, + { + name: "repo-scoped single session (tty)", + tty: true, + baseRepo: ghrepo.New("OWNER", "REPO"), + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForRepoFunc = func(ctx context.Context, owner, repo string, limit int) ([]*capi.Session, error) { + return []*capi.Session{ + { + ID: "s1", + State: "completed", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 101, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, nil + } + }, wantOut: heredoc.Doc(` - SESSION ID PULL REQUEST REPO SESSION STATE CREATED - sessR1 #55 OWNER/REPO completed about 6 hours ago + SESSION ID PULL REQUEST REPO SESSION STATE CREATED + s1 #101 OWNER/REPO completed about 6 hours ago `), }, { - name: "repo scoped no sessions", - tty: true, - stubs: func(reg *httpmock.Registry) { registerRepoEmptySessionsMock(reg, "OWNER", "REPO") }, + name: "repo-scoped single session (nontty)", + tty: false, baseRepo: ghrepo.New("OWNER", "REPO"), - wantErr: cmdutil.NewNoResultsError("no agent tasks found"), + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForRepoFunc = func(ctx context.Context, owner, repo string, limit int) ([]*capi.Session, error) { + return []*capi.Session{ + { + ID: "s1", + State: "completed", + ResourceType: "pull", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Number: 101, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, nil + } + }, + wantOut: "s1\t#101\tOWNER/REPO\tcompleted\t" + sampleDateString + "\n", // header omitted for non-tty + }, + { + name: "repo-scoped many sessions (tty)", + tty: true, + baseRepo: ghrepo.New("OWNER", "REPO"), + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListSessionsForRepoFunc = func(ctx context.Context, owner, repo string, limit int) ([]*capi.Session, error) { + return []*capi.Session{ + { + ID: "s1", + State: "completed", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 101, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s2", + State: "failed", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 102, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s3", + State: "in_progress", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 103, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s4", + State: "queued", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 104, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s5", + State: "canceled", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 105, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + { + ID: "s6", + State: "mystery", + CreatedAt: sampleDate, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 106, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, nil + } + }, + wantOut: heredoc.Doc(` + SESSION ID PULL REQUEST REPO SESSION STATE CREATED + s1 #101 OWNER/REPO completed about 6 hours ago + s2 #102 OWNER/REPO failed about 6 hours ago + s3 #103 OWNER/REPO in_progress about 6 hours ago + s4 #104 OWNER/REPO queued about 6 hours ago + s5 #105 OWNER/REPO canceled about 6 hours ago + s6 #106 OWNER/REPO mystery about 6 hours ago + `), }, { name: "repo resolution error does not surface", tty: true, baseRepoErr: errors.New("ambiguous repo"), - wantErr: cmdutil.NewNoResultsError("no agent tasks found"), - stubs: func(reg *httpmock.Registry) { registerEmptySessionsMock(reg) }, - }, - { - name: "repo scoped many sessions (tty)", - tty: true, - stubs: func(reg *httpmock.Registry) { registerRepoManySessionsMock(reg, createdAt, "OWNER", "REPO") }, - baseRepo: ghrepo.New("OWNER", "REPO"), - wantOut: heredoc.Doc(` - SESSION ID PULL REQUEST REPO SESSION STATE CREATED - r1 #301 OWNER/REPO completed about 6 hours ago - r2 #302 OWNER/REPO failed about 6 hours ago - r3 #303 OWNER/REPO in_progress about 6 hours ago - r4 #304 OWNER/REPO queued about 6 hours ago - r5 #305 OWNER/REPO canceled about 6 hours ago - r6 #306 OWNER/REPO mystery about 6 hours ago - `), + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + // We expect a viewer-scoped fetch request: + m.ListSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + return nil, nil + } + }, + wantErr: cmdutil.NewNoResultsError("no agent tasks found"), }, { name: "web mode", @@ -225,15 +484,11 @@ func Test_listRun(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - reg := &httpmock.Registry{} - if tt.stubs != nil { - tt.stubs(reg) + capiClientMock := &capi.CapiClientMock{} + if tt.capiStubs != nil { + tt.capiStubs(t, capiClientMock) } - cfg := config.NewBlankConfig() - cfg.Set("github.com", "oauth_token", "OTOKEN") - authCfg := cfg.Authentication() - ios, _, stdout, stderr := iostreams.Test() ios.SetStdoutTTY(tt.tty) @@ -242,11 +497,8 @@ func Test_listRun(t *testing.T) { br = &browser.Stub{} } - httpClient := &http.Client{Transport: reg} - capiClient := capi.NewCAPIClient(httpClient, authCfg) opts := &ListOptions{ IO: ios, - Config: func() (gh.Config, error) { return cfg, nil }, Limit: tt.limit, Web: tt.web, Browser: br, @@ -254,7 +506,7 @@ func Test_listRun(t *testing.T) { if tt.web { require.FailNow(t, "CapiClient was called with --web") } - return capiClient, nil + return capiClientMock, nil }, } if tt.baseRepo != nil || tt.baseRepoErr != nil { @@ -276,516 +528,6 @@ func Test_listRun(t *testing.T) { if tt.web { br.Verify(t, tt.wantBrowserURL) } - reg.Verify(t) }) } } - -// registerRepoSingleSessionMock mocks repo-scoped endpoint with one session and hydration. -func registerRepoSingleSessionMock(reg *httpmock.Registry, createdAt, owner, repo string) { - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions/nwo/"+owner+"/"+repo), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Docf(`{ - "sessions": [ - { - "id": "sessR1", - "name": "Repo build", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "completed", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 3000, - "created_at": "%[1]s" - } - ] - }`, createdAt)), - ) - // Second page empty (pagination end) - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions/nwo/"+owner+"/"+repo), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Doc(`{ - "sessions": [] - }`)), - ) - // Hydration - reg.Register( - httpmock.GraphQL(`query FetchPRs`), - httpmock.StringResponse(heredoc.Docf(`{ - "data": { - "nodes": [ - { - "id": "PR_nodeR1", - "fullDatabaseId": "3000", - "number": 55, - "title": "Improve build", - "state": "OPEN", - "url": "https://github.com/%[2]s/%[3]s/pull/55", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { "nameWithOwner": "%[2]s/%[3]s" } - } - ] - } -}`, createdAt, owner, repo)), - ) -} - -// registerRepoEmptySessionsMock mocks repo-scoped endpoint returning no sessions. -func registerRepoEmptySessionsMock(reg *httpmock.Registry, owner, repo string) { - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions/nwo/"+owner+"/"+repo), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Doc(`{ - "sessions": [] -}`)), - ) -} - -// registerRepoManySessionsMock mirrors registerManySessionsMock but for repo-scoped endpoint -func registerRepoManySessionsMock(reg *httpmock.Registry, createdAt, owner, repo string) { - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions/nwo/"+owner+"/"+repo), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Docf(`{ - "sessions": [ - { - "id": "r1", - "name": "A", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "completed", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 3001, - "created_at": "%[1]s" - }, - { - "id": "r2", - "name": "B", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "failed", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 3002, - "created_at": "%[1]s" - }, - { - "id": "r3", - "name": "C", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "in_progress", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 3003, - "created_at": "%[1]s" - }, - { - "id": "r4", - "name": "D", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "queued", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 3004, - "created_at": "%[1]s" - }, - { - "id": "r5", - "name": "E", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "canceled", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 3005, - "created_at": "%[1]s" - }, - { - "id": "r6", - "name": "F", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "mystery", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 3006, - "created_at": "%[1]s" - } - ] - }`, createdAt)), - ) - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions/nwo/"+owner+"/"+repo), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Doc(`{ - "sessions": [] - }`)), - ) - reg.Register( - httpmock.GraphQL(`query FetchPRs`), - httpmock.StringResponse(heredoc.Docf(`{ - "data": { - "nodes": [ - { - "id": "PR_r1", - "fullDatabaseId": "3001", - "number": 301, - "title": "PR 301", - "state": "OPEN", - "url": "https://github.com/%[2]s/%[3]s/pull/301", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "%[2]s/%[3]s" - } - }, - { - "id": "PR_r2", - "fullDatabaseId": "3002", - "number": 302, - "title": "PR 302", - "state": "OPEN", - "url": "https://github.com/%[2]s/%[3]s/pull/302", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "%[2]s/%[3]s" - } - }, - { - "id": "PR_r3", - "fullDatabaseId": "3003", - "number": 303, - "title": "PR 303", - "state": "OPEN", - "url": "https://github.com/%[2]s/%[3]s/pull/303", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "%[2]s/%[3]s" - } - }, - { - "id": "PR_r4", - "fullDatabaseId": "3004", - "number": 304, - "title": "PR 304", - "state": "OPEN", - "url": "https://github.com/%[2]s/%[3]s/pull/304", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "%[2]s/%[3]s" - } - }, - { - "id": "PR_r5", - "fullDatabaseId": "3005", - "number": 305, - "title": "PR 305", - "state": "OPEN", - "url": "https://github.com/%[2]s/%[3]s/pull/305", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "%[2]s/%[3]s" - } - }, - { - "id": "PR_r6", - "fullDatabaseId": "3006", - "number": 306, - "title": "PR 306", - "state": "OPEN", - "url": "https://github.com/%[2]s/%[3]s/pull/306", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "%[2]s/%[3]s" - } - } - ] - } - }`, createdAt, owner, repo)), - ) -} - -// registerEmptySessionsMock registers a single empty page of sessions -func registerEmptySessionsMock(reg *httpmock.Registry) { - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions"), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Doc(`{ - "sessions": [] - }`)), - ) -} - -// registerSingleSessionMock registers two REST pages (one with a session, one empty) and GraphQL hydration for that session's PR -func registerSingleSessionMock(reg *httpmock.Registry, createdAt string) { - // First page with one session - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions"), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Docf(`{ - "sessions": [ - { - "id": "sess1", - "name": "Build artifacts", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "completed", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 2000, - "created_at": "%[1]s" - } - ] -}`, createdAt)), - ) - // Second page empty to terminate pagination - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions"), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Doc(`{ - "sessions": [] - }`)), - ) - // GraphQL hydration - reg.Register( - httpmock.GraphQL(`query FetchPRs`), - httpmock.StringResponse(heredoc.Docf(`{ - "data": { - "nodes": [ - { - "id": "PR_node", - "fullDatabaseId": "2000", - "number": 42, - "title": "Improve docs", - "state": "OPEN", - "url": "https://github.com/OWNER/REPO/pull/42", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } - } - ] - } - }`, createdAt)), - ) -} - -// registerManySessionsMock registers multiple sessions covering various states -// States covered: completed, failed, in_progress, queued, canceled, (unknown -> treated as muted) -func registerManySessionsMock(reg *httpmock.Registry, createdAt string) { - // First page returns six sessions - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions"), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Docf(`{ - "sessions": [ - { - "id": "s1", - "name": "A", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "completed", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 2000, - "created_at": "%[1]s" - }, - { - "id": "s2", - "name": "B", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "failed", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 2001, - "created_at": "%[1]s" - }, - { - "id": "s3", - "name": "C", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "in_progress", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 2002, - "created_at": "%[1]s" - }, - { - "id": "s4", - "name": "D", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "queued", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 2003, - "created_at": "%[1]s" - }, - { - "id": "s5", - "name": "E", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "canceled", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 2004, - "created_at": "%[1]s" - }, - { - "id": "s6", - "name": "F", - "user_id": 1, - "agent_id": 2, - "logs": "", - "state": "mystery", - "owner_id": 10, - "repo_id": 1000, - "resource_type": "pull", - "resource_id": 2005, - "created_at": "%[1]s" - } - ] -}`, createdAt)), - ) - // Second page empty - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/sessions"), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Doc(`{ - "sessions": [] - }`)), - ) - // GraphQL hydration for 6 PRs - reg.Register( - httpmock.GraphQL(`query FetchPRs`), - httpmock.StringResponse(heredoc.Docf(`{ - "data": { - "nodes": [ - { - "id": "PR_node1", - "fullDatabaseId": "2000", - "number": 101, - "title": "PR 101", - "state": "OPEN", - "url": "https://github.com/OWNER/REPO/pull/101", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } - }, - { - "id": "PR_node2", - "fullDatabaseId": "2001", - "number": 102, - "title": "PR 102", - "state": "OPEN", - "url": "https://github.com/OWNER/REPO/pull/102", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } - }, - { - "id": "PR_node3", - "fullDatabaseId": "2002", - "number": 103, - "title": "PR 103", - "state": "OPEN", - "url": "https://github.com/OWNER/REPO/pull/103", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } - }, - { - "id": "PR_node4", - "fullDatabaseId": "2003", - "number": 104, - "title": "PR 104", - "state": "OPEN", - "url": "https://github.com/OWNER/REPO/pull/104", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } - }, - { - "id": "PR_node5", - "fullDatabaseId": "2004", - "number": 105, - "title": "PR 105", - "state": "OPEN", - "url": "https://github.com/OWNER/REPO/pull/105", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } - }, - { - "id": "PR_node6", - "fullDatabaseId": "2005", - "number": 106, - "title": "PR 106", - "state": "OPEN", - "url": "https://github.com/OWNER/REPO/pull/106", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } - } - ] - } - }`, createdAt)), - ) -} From 3762d978948100f7f1b2eeabd538aa1c3796aa84 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 19:54:33 +0100 Subject: [PATCH 13/20] test(agent-task list): apply test args anyway Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/list/list_test.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index 5ed0d097d..ce0fd323f 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -1,8 +1,10 @@ package list import ( + "bytes" "context" "errors" + "io" "testing" "time" @@ -78,12 +80,15 @@ func TestNewCmdList(t *testing.T) { var gotOpts *ListOptions cmd := NewCmdList(f, func(opts *ListOptions) error { gotOpts = opts; return nil }) - if tt.args != "" { - argv, err := shlex.Split(tt.args) - require.NoError(t, err) - cmd.SetArgs(argv) - } - _, err := cmd.ExecuteC() + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() if tt.wantErr != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) From b0ac06e4f449aef5fde1b8c91025c20e6406b933 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 20:17:43 +0100 Subject: [PATCH 14/20] fix(agent-task create): allow no positional arg Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/create/create.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index 6a5d169be..c7f6d3ad9 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -47,12 +47,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return err } - // 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 task description is required") - } - // Populate ProblemStatement from either arg or file if len(args) > 0 { opts.ProblemStatement = args[0] From d17fdb3e8c16b843195b492053d608b5596093c4 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 20:19:35 +0100 Subject: [PATCH 15/20] test(agent-task list): update `TestNewCmdCreate` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/create/create_test.go | 123 ++++++++++++----------- 1 file changed, 65 insertions(+), 58 deletions(-) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index a7bb8a166..8cd82ac8c 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -1,10 +1,11 @@ package create import ( + "fmt" + "io" "net/http" "os" "path/filepath" - "slices" "testing" "github.com/MakeNowJust/heredoc" @@ -15,103 +16,109 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" "github.com/stretchr/testify/require" ) -// Test basic option parsing & repository requirement -func TestNewCmdCreate_Args(t *testing.T) { +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 - fileContent string // if non-empty, create temp file and substitute {{FILE}} token in args - wantOpts *CreateOptions // nil when expecting error - expectedErr string + name string + args string + stdin string + wantOpts *CreateOptions // nil when expecting error + wantErr string }{ { - name: "no args nor file", - args: []string{}, - expectedErr: "a task description is required", + name: "no args nor file", + wantErr: "a task description is required", }, { name: "arg only success", - args: []string{"task description from args"}, + args: "'task description from args'", wantOpts: &CreateOptions{ ProblemStatement: "task description from args", }, }, { - name: "from-file success", - args: []string{"-F", "{{FILE}}"}, - fileContent: "task description from file", + name: "from-file success", + args: fmt.Sprintf("-F %s", tmpFile), wantOpts: &CreateOptions{ ProblemStatement: "task description from file", }, }, { - name: "file content from stdin success", - args: []string{"-F", "-"}, - fileContent: "task from stdin", - wantOpts: &CreateOptions{ProblemStatement: "task from stdin"}, + name: "file content from stdin success", + args: "-F -", + stdin: "task description from stdin", + wantOpts: &CreateOptions{ + ProblemStatement: "task description from stdin", + }, }, { - name: "mutually exclusive arg and file", - args: []string{"Some task inline", "-F", "{{FILE}}"}, - fileContent: "Some task", - expectedErr: "only one of -F or arg can be provided", + 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", - args: []string{"-F", "does-not-exist.md"}, - expectedErr: "could not read task description file: open does-not-exist.md: no such file or directory", + name: "missing file path", + args: "-F does-not-exist.md", + wantErr: "could not read task description file: open does-not-exist.md:", }, { - name: "empty file", - args: []string{"-F", "{{FILE}}"}, - fileContent: " \n\n", - expectedErr: "task description file is empty", + name: "empty file", + args: fmt.Sprintf("-F %s", tmpEmptyFile), + wantErr: "task description file is empty", + }, + { + name: "empty from stdin", + args: "-F -", + stdin: " \n\n", + wantErr: "task description file is empty", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ios, stdinBuf, _, _ := iostreams.Test() - - // Provide file content either via stdin ( -F - ) or by creating a temp file - if tt.fileContent != "" { - isStdin := len(tt.args) == 2 && tt.args[0] == "-F" && tt.args[1] == "-" - hasFileToken := slices.Contains(tt.args, "{{FILE}}") - - switch { - case isStdin: - stdinBuf.WriteString(tt.fileContent) - case hasFileToken: - dir := t.TempDir() - path := filepath.Join(dir, "task.md") - if err := os.WriteFile(path, []byte(tt.fileContent), 0o600); err != nil { - t.Fatalf("failed to write temp file: %v", err) - } - for i, a := range tt.args { - if a == "{{FILE}}" { - tt.args[i] = path - } - } - } + ios, stdin, _, _ := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: ios, } - f := &cmdutil.Factory{IOStreams: ios} var gotOpts *CreateOptions cmd := NewCmdCreate(f, func(o *CreateOptions) error { gotOpts = o return nil }) - cmd.SetArgs(tt.args) - _, err := cmd.ExecuteC() - if tt.expectedErr != "" { - require.Error(t, err) - require.Equal(t, tt.expectedErr, err.Error()) + 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.NoError(t, err) if tt.wantOpts != nil { require.Equal(t, tt.wantOpts.ProblemStatement, gotOpts.ProblemStatement) From 28bb0f62bbe402751056143b92f15d2c562930dc Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 20:59:33 +0100 Subject: [PATCH 16/20] fix(agent-task create): simplify command initialisation Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/create/create.go | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index c7f6d3ad9..7b081b4e5 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -33,7 +33,8 @@ type CreateOptions struct { func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { opts := &CreateOptions{ - IO: f.IOStreams, + IO: f.IOStreams, + CapiClient: shared.CapiClientFunc(f), } var fromFileName string @@ -43,6 +44,9 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co Short: "Create an agent task (preview)", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + // 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 { return err } @@ -61,17 +65,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co } opts.ProblemStatement = trimmed } + if opts.ProblemStatement == "" { return cmdutil.FlagErrorf("a task description is required") } - // Support -R/--repo override - if f != nil { - opts.BaseRepo = f.BaseRepo - - if opts.CapiClient == nil { - opts.CapiClient = shared.CapiClientFunc(f) - } - } if runF != nil { return runF(opts) @@ -103,12 +100,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co } func createRun(opts *CreateOptions) error { - if opts.ProblemStatement == "" { - 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 { // Not printing the error that came back from BaseRepo() here because we want From 07ec8c629d13e5f5c8a43d727fad4fdae2c987f6 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 21:01:56 +0100 Subject: [PATCH 17/20] test(agent-task create): use `CapiClientMock` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/create/create_test.go | 300 +++++++++++------------ 1 file changed, 150 insertions(+), 150 deletions(-) diff --git a/pkg/cmd/agent-task/create/create_test.go b/pkg/cmd/agent-task/create/create_test.go index 8cd82ac8c..3a244c0e7 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -1,20 +1,19 @@ package create import ( + "context" + "errors" "fmt" "io" - "net/http" "os" "path/filepath" "testing" + "time" - "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/google/shlex" "github.com/stretchr/testify/require" @@ -128,177 +127,182 @@ func TestNewCmdCreate(t *testing.T) { } 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" - }`) + sampleDateString := "2025-08-29T00:00:00Z" + sampleDate, err := time.Parse(time.RFC3339, sampleDateString) + require.NoError(t, err) + + createdJobSuccess := capi.Job{ + ID: "job123", + SessionID: "sess1", + Actor: &capi.JobActor{ + ID: 1, + Login: "octocat", + }, + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + } + createdJobSuccessWithPR := capi.Job{ + ID: "job123", + SessionID: "sess1", + Actor: &capi.JobActor{ + ID: 1, + Login: "octocat", + }, + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + PullRequest: &capi.JobPullRequest{ + ID: 101, + Number: 42, + }, + } tests := []struct { - name string - stubs func(*httpmock.Registry) - baseRepoFunc func() (ghrepo.Interface, error) - problemStatement string - baseBranch string - wantStdout string - wantStdErr string - wantErr string + name string + capiStubs func(*testing.T, *capi.CapiClientMock) + baseRepoFunc func() (ghrepo.Interface, error) + baseBranch string + wantStdout string + wantStdErr string + wantErr string }{ { - name: "base branch included in create payload", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - problemStatement: "Do the thing", - baseBranch: "feature", - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.WithHost(httpmock.REST("POST", "agents/swe/v1/jobs/OWNER/REPO"), "api.githubcopilot.com"), - httpmock.RESTPayload(201, createdJobSuccessWithPRResponse, func(payload map[string]interface{}) { - prRaw, ok := payload["pull_request"].(map[string]interface{}) - if !ok { - require.FailNow(t, "expected pull_request object in payload") - } - if prRaw["base_ref"] != "refs/heads/feature" { - require.FailNow(t, "expected pull_request.base_ref to be 'refs/heads/feature'") - } - if payload["problem_statement"] != "Do the thing" { - require.FailNow(t, "unexpected problem_statement value") - } - }), - ) - }, - wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", + 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: "get job API failure surfaces error", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - 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", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - 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", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - 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", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - 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), - ) - // 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"}`), - ) + name: "base branch included in create payload", + baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, + 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) + require.Equal(t, "REPO", repo) + require.Equal(t, "Do the thing", problemStatement) + require.Equal(t, "feature", baseBranch) + return &createdJobSuccess, nil + } + m.GetJobFunc = func(ctx context.Context, owner, repo, jobID string) (*capi.Job, error) { + require.Equal(t, "OWNER", owner) + require.Equal(t, "REPO", repo) + require.Equal(t, "job123", jobID) + return &createdJobSuccessWithPR, nil } }, - wantStdout: "job jobABC queued. View progress: https://github.com/copilot/agents\n", + wantStdout: "https://github.com/OWNER/REPO/pull/42/agent-sessions/sess1\n", }, { - name: "missing repo returns error", - problemStatement: "task", - 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", - baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }, - 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(500, `{"error":{"message":"some API error"}}`), - ) + name: "create task API failure returns error", + baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), 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, "OWNER", owner) + require.Equal(t, "REPO", repo) + require.Equal(t, "Do the thing", problemStatement) + require.Equal(t, "", baseBranch) + return nil, errors.New("some error") + } }, - wantErr: "failed to create job: some API error", + wantErr: "some 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", + name: "get job API failure surfaces error", + baseRepoFunc: func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), 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, "OWNER", owner) + require.Equal(t, "REPO", repo) + require.Equal(t, "Do the thing", problemStatement) + require.Equal(t, "", baseBranch) + return &createdJobSuccess, nil + } + m.GetJobFunc = func(ctx context.Context, owner, repo, jobID string) (*capi.Job, error) { + return nil, errors.New("some error") + } + }, + wantStdErr: "some error\n", + 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 }, + 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 &createdJobSuccessWithPR, nil + } + }, + 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 }, + 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 &createdJobSuccess, nil + } + m.GetJobFunc = func(ctx context.Context, owner, repo, jobID string) (*capi.Job, error) { + require.Equal(t, "OWNER", owner) + require.Equal(t, "REPO", repo) + require.Equal(t, "job123", jobID) + return &createdJobSuccessWithPR, nil + } + }, + 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 }, + 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 &createdJobSuccess, nil + } + + count := 0 + m.GetJobFunc = func(ctx context.Context, owner, repo, jobID string) (*capi.Job, error) { + if count++; count > 4 { + require.FailNow(t, "too many get calls") + } + return &createdJobSuccess, nil + } + }, + wantStdout: "job job123 queued. View progress: https://github.com/copilot/agents\n", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + capiClientMock := &capi.CapiClientMock{} + if tt.capiStubs != nil { + tt.capiStubs(t, capiClientMock) + } + ios, _, stdout, stderr := iostreams.Test() opts := &CreateOptions{ IO: ios, - ProblemStatement: tt.problemStatement, + ProblemStatement: "Do the thing", BaseRepo: tt.baseRepoFunc, BaseBranch: tt.baseBranch, + CapiClient: func() (capi.CapiClient, error) { + return capiClientMock, nil + }, } // 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 != "" { @@ -310,10 +314,6 @@ func Test_createRun(t *testing.T) { require.Equal(t, tt.wantStdout, stdout.String()) require.Equal(t, tt.wantStdErr, stderr.String()) - - if tt.stubs != nil { - reg.Verify(t) - } }) } } From 844c0ab56eec219cd237667098e899c9638f17c4 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 21:06:15 +0100 Subject: [PATCH 18/20] refactor(agent-task create): remove redundant `if` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/create/create.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/agent-task/create/create.go b/pkg/cmd/agent-task/create/create.go index 7b081b4e5..028740612 100644 --- a/pkg/cmd/agent-task/create/create.go +++ b/pkg/cmd/agent-task/create/create.go @@ -89,9 +89,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co $ gh agent-task create "fix errors" --base branch `), } - if f != nil { - cmdutil.EnableRepoOverride(cmd, f) - } + + 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.BaseBranch, "base", "b", "", "Base branch for the pull request (use default branch if not provided)") From 7824b43f8a23dea62c102d7916b3895a685b862d Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 4 Sep 2025 21:06:38 +0100 Subject: [PATCH 19/20] refactor(agent-task list): simplify cmd initialisation Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/list/list.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index 9c19c61f6..b9f892a63 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -32,9 +32,10 @@ type ListOptions struct { // NewCmdList creates the list command func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { opts := &ListOptions{ - IO: f.IOStreams, - Limit: defaultLimit, - Browser: f.Browser, + IO: f.IOStreams, + CapiClient: shared.CapiClientFunc(f), + Limit: defaultLimit, + Browser: f.Browser, } cmd := &cobra.Command{ @@ -45,10 +46,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman // Support -R/--repo override opts.BaseRepo = f.BaseRepo - if opts.CapiClient == nil { - opts.CapiClient = shared.CapiClientFunc(f) - } - if opts.Limit < 1 { return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit) } @@ -59,9 +56,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman }, } - if f != nil { - cmdutil.EnableRepoOverride(cmd, f) - } + cmdutil.EnableRepoOverride(cmd, f) cmd.Flags().IntVarP(&opts.Limit, "limit", "L", defaultLimit, fmt.Sprintf("Maximum number of agent tasks to fetch (default %d)", defaultLimit)) cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open agent tasks in the browser") From 004be9da2005289013167faf5c464cdc92b10dc0 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 5 Sep 2025 09:28:44 +0100 Subject: [PATCH 20/20] test(agent-task create): quote file paths to pass CI on Windows To keep the backslashes in Windows file paths and stop `shlex.Split` from interpreting them as escape characters, we need to quote the paths. Signed-off-by: Babak K. Shandiz --- 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 3a244c0e7..00134792f 100644 --- a/pkg/cmd/agent-task/create/create_test.go +++ b/pkg/cmd/agent-task/create/create_test.go @@ -50,7 +50,7 @@ func TestNewCmdCreate(t *testing.T) { }, { name: "from-file success", - args: fmt.Sprintf("-F %s", tmpFile), + args: fmt.Sprintf("-F '%s'", tmpFile), wantOpts: &CreateOptions{ ProblemStatement: "task description from file", }, @@ -75,7 +75,7 @@ func TestNewCmdCreate(t *testing.T) { }, { name: "empty file", - args: fmt.Sprintf("-F %s", tmpEmptyFile), + args: fmt.Sprintf("-F '%s'", tmpEmptyFile), wantErr: "task description file is empty", }, {