diff --git a/pkg/cmd/agent-task/capi/client.go b/pkg/cmd/agent-task/capi/client.go index 6d35f11c2..c25549121 100644 --- a/pkg/cmd/agent-task/capi/client.go +++ b/pkg/cmd/agent-task/capi/client.go @@ -15,8 +15,7 @@ const capiHost = "api.githubcopilot.com" // CapiClient defines the methods used by the caller. Implementations // may be replaced with test doubles in unit tests. type CapiClient interface { - ListSessionsForViewer(ctx context.Context, limit int) ([]*Session, error) - ListSessionsForRepo(ctx context.Context, owner string, repo string, limit int) ([]*Session, error) + ListLatestSessionsForViewer(ctx context.Context, limit int) ([]*Session, error) CreateJob(ctx context.Context, owner, repo, problemStatement, baseBranch string) (*Job, error) GetJob(ctx context.Context, owner, repo, jobID string) (*Job, error) GetSession(ctx context.Context, id string) (*Session, error) diff --git a/pkg/cmd/agent-task/capi/client_mock.go b/pkg/cmd/agent-task/capi/client_mock.go index fa59ffab7..325a8d513 100644 --- a/pkg/cmd/agent-task/capi/client_mock.go +++ b/pkg/cmd/agent-task/capi/client_mock.go @@ -33,15 +33,12 @@ var _ CapiClient = &CapiClientMock{} // GetSessionLogsFunc: func(ctx context.Context, id string) ([]byte, error) { // panic("mock out the GetSessionLogs method") // }, +// ListLatestSessionsForViewerFunc: func(ctx context.Context, limit int) ([]*Session, error) { +// panic("mock out the ListLatestSessionsForViewer method") +// }, // ListSessionsByResourceIDFunc: func(ctx context.Context, resourceType string, resourceID int64, limit int) ([]*Session, error) { // panic("mock out the ListSessionsByResourceID 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 @@ -64,15 +61,12 @@ type CapiClientMock struct { // GetSessionLogsFunc mocks the GetSessionLogs method. GetSessionLogsFunc func(ctx context.Context, id string) ([]byte, error) + // ListLatestSessionsForViewerFunc mocks the ListLatestSessionsForViewer method. + ListLatestSessionsForViewerFunc func(ctx context.Context, limit int) ([]*Session, error) + // ListSessionsByResourceIDFunc mocks the ListSessionsByResourceID method. ListSessionsByResourceIDFunc func(ctx context.Context, resourceType string, resourceID int64, limit int) ([]*Session, 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. @@ -126,6 +120,13 @@ type CapiClientMock struct { // ID is the id argument value. ID string } + // ListLatestSessionsForViewer holds details about calls to the ListLatestSessionsForViewer method. + ListLatestSessionsForViewer []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Limit is the limit argument value. + Limit int + } // ListSessionsByResourceID holds details about calls to the ListSessionsByResourceID method. ListSessionsByResourceID []struct { // Ctx is the ctx argument value. @@ -137,33 +138,14 @@ type CapiClientMock struct { // Limit is the limit argument value. Limit int } - // 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 - lockGetPullRequestDatabaseID sync.RWMutex - lockGetSession sync.RWMutex - lockGetSessionLogs sync.RWMutex - lockListSessionsByResourceID sync.RWMutex - lockListSessionsForRepo sync.RWMutex - lockListSessionsForViewer sync.RWMutex + lockCreateJob sync.RWMutex + lockGetJob sync.RWMutex + lockGetPullRequestDatabaseID sync.RWMutex + lockGetSession sync.RWMutex + lockGetSessionLogs sync.RWMutex + lockListLatestSessionsForViewer sync.RWMutex + lockListSessionsByResourceID sync.RWMutex } // CreateJob calls CreateJobFunc. @@ -378,6 +360,42 @@ func (mock *CapiClientMock) GetSessionLogsCalls() []struct { return calls } +// ListLatestSessionsForViewer calls ListLatestSessionsForViewerFunc. +func (mock *CapiClientMock) ListLatestSessionsForViewer(ctx context.Context, limit int) ([]*Session, error) { + if mock.ListLatestSessionsForViewerFunc == nil { + panic("CapiClientMock.ListLatestSessionsForViewerFunc: method is nil but CapiClient.ListLatestSessionsForViewer was just called") + } + callInfo := struct { + Ctx context.Context + Limit int + }{ + Ctx: ctx, + Limit: limit, + } + mock.lockListLatestSessionsForViewer.Lock() + mock.calls.ListLatestSessionsForViewer = append(mock.calls.ListLatestSessionsForViewer, callInfo) + mock.lockListLatestSessionsForViewer.Unlock() + return mock.ListLatestSessionsForViewerFunc(ctx, limit) +} + +// ListLatestSessionsForViewerCalls gets all the calls that were made to ListLatestSessionsForViewer. +// Check the length with: +// +// len(mockedCapiClient.ListLatestSessionsForViewerCalls()) +func (mock *CapiClientMock) ListLatestSessionsForViewerCalls() []struct { + Ctx context.Context + Limit int +} { + var calls []struct { + Ctx context.Context + Limit int + } + mock.lockListLatestSessionsForViewer.RLock() + calls = mock.calls.ListLatestSessionsForViewer + mock.lockListLatestSessionsForViewer.RUnlock() + return calls +} + // ListSessionsByResourceID calls ListSessionsByResourceIDFunc. func (mock *CapiClientMock) ListSessionsByResourceID(ctx context.Context, resourceType string, resourceID int64, limit int) ([]*Session, error) { if mock.ListSessionsByResourceIDFunc == nil { @@ -421,83 +439,3 @@ func (mock *CapiClientMock) ListSessionsByResourceIDCalls() []struct { mock.lockListSessionsByResourceID.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 -} diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index f4f12254d..b379f6543 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -88,9 +88,9 @@ type Session struct { User *api.GitHubUser } -// ListSessionsForViewer lists all agent sessions for the +// ListLatestSessionsForViewer lists all agent sessions for the // authenticated user up to limit. -func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*Session, error) { +func (c *CAPIClient) ListLatestSessionsForViewer(ctx context.Context, limit int) ([]*Session, error) { if limit == 0 { return nil, nil } @@ -99,7 +99,8 @@ func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*S pageSize := defaultSessionsPerPage sessions := make([]session, 0, limit+pageSize) - + seenResources := make(map[int64]struct{}) + latestSessions := make([]session, 0, limit) for page := 1; ; page++ { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) if err != nil { @@ -109,6 +110,7 @@ func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*S q := req.URL.Query() q.Set("page_size", strconv.Itoa(pageSize)) q.Set("page_number", strconv.Itoa(page)) + q.Set("sort", "last_updated_at,desc") req.URL.RawQuery = q.Encode() res, err := c.httpClient.Do(req) @@ -126,18 +128,41 @@ func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*S return nil, fmt.Errorf("failed to decode sessions response: %w", err) } - sessions = append(sessions, response.Sessions...) - if len(response.Sessions) < pageSize || len(sessions) >= limit { + // Process only the newly fetched page worth of sessions. + pageSessions := response.Sessions + sessions = append(sessions, pageSessions...) + + // De-duplicate sessions by resource ID. + // Because the API returns newest first, once we've seen + // a resource ID we can ignore any older sessions for it. + for _, s := range pageSessions { + if _, exists := seenResources[s.ResourceID]; exists { + continue + } + + // A zero resource ID is a temporary situation before a PR/resource + // is associated with the session. We should not mark such case as seen. + if s.ResourceID != 0 { + seenResources[s.ResourceID] = struct{}{} + } + + latestSessions = append(latestSessions, s) + if len(latestSessions) >= limit { + break + } + } + + if len(response.Sessions) < pageSize || len(latestSessions) >= limit { break } } // Drop any above the limit - if len(sessions) > limit { - sessions = sessions[:limit] + if len(latestSessions) > limit { + latestSessions = latestSessions[:limit] } - result, err := c.hydrateSessionPullRequestsAndUsers(sessions) + result, err := c.hydrateSessionPullRequestsAndUsers(latestSessions) if err != nil { return nil, fmt.Errorf("failed to fetch session resources: %w", err) } @@ -145,65 +170,6 @@ func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*S return result, nil } -// ListSessionsForRepo lists agent sessions for a specific repository identified by owner/name up to limit. -func (c *CAPIClient) ListSessionsForRepo(ctx context.Context, owner string, repo string, limit int) ([]*Session, error) { - if owner == "" || 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 - - sessions := make([]session, 0, limit+pageSize) - - 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(pageSize)) - q.Set("page_number", strconv.Itoa(page)) - req.URL.RawQuery = q.Encode() - - res, err := c.httpClient.Do(req) - if err != nil { - return nil, err - } - defer res.Body.Close() - if res.StatusCode != http.StatusOK { - return nil, fmt.Errorf("failed to list sessions: %s", res.Status) - } - var response struct { - Sessions []session `json:"sessions"` - } - if err := json.NewDecoder(res.Body).Decode(&response); err != nil { - return nil, fmt.Errorf("failed to decode sessions response: %w", err) - } - - sessions = append(sessions, response.Sessions...) - if len(response.Sessions) < pageSize || len(sessions) >= limit { - break - } - } - - // Drop any above the limit - if len(sessions) > limit { - sessions = sessions[:limit] - } - - result, err := c.hydrateSessionPullRequestsAndUsers(sessions) - if err != nil { - return nil, fmt.Errorf("failed to fetch session resources: %w", err) - } - return result, nil -} - // GetSession retrieves a specific agent session by ID. func (c *CAPIClient) GetSession(ctx context.Context, id string) (*Session, error) { if id == "" { diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 45984dfec..b91a8b401 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -16,7 +16,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestListSessionsForViewer(t *testing.T) { +func TestListLatestSessionsForViewer(t *testing.T) { sampleDateString := "2025-08-29T00:00:00Z" sampleDate, err := time.Parse(time.RFC3339, sampleDateString) require.NoError(t, err) @@ -434,30 +434,17 @@ func TestListSessionsForViewer(t *testing.T) { }, }, { - name: "API error", - limit: 10, + name: "multiple pages with duplicates per PR only newest kept", + perPage: 2, + limit: 3, httpStubs: func(t *testing.T, reg *httpmock.Registry) { + // Page 1 returns newest sessions (ordered newest first overall) 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"}, + "page_size": {"2"}, + "sort": {"last_updated_at,desc"}, }), "api.githubcopilot.com", ), @@ -465,7 +452,7 @@ func TestListSessionsForViewer(t *testing.T) { { "sessions": [ { - "id": "sess1", + "id": "sessA-new", "name": "Build artifacts", "user_id": 1, "agent_id": 2, @@ -474,123 +461,11 @@ func TestListSessionsForViewer(t *testing.T) { "owner_id": 10, "repo_id": 1000, "resource_type": "pull", - "resource_id": 2000, - "created_at": "%[1]s", - "premium_requests": 0.1 - } - ] - }`, - sampleDateString, - )), - ) - // GraphQL hydration - reg.Register( - httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\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": [ + "resource_id": 3000, + "created_at": "%[1]s" + }, { - "id": "sess1", + "id": "sessB-new", "name": "Build artifacts", "user_id": 1, "agent_id": 2, @@ -599,16 +474,61 @@ func TestListSessionsForRepo(t *testing.T) { "owner_id": 10, "repo_id": 1000, "resource_type": "pull", - "resource_id": 2000, - "created_at": "%[1]s", - "premium_requests": 0.1 + "resource_id": 3001, + "created_at": "%[1]s" } ] }`, sampleDateString, )), ) - // GraphQL hydration + + // Page 2 returns older duplicate sessions for 3000, plus another new PR 3002 + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ + "page_number": {"2"}, + "page_size": {"2"}, + "sort": {"last_updated_at,desc"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sessA-old", + "name": "Build artifacts", + "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" + }, + { + "id": "sessC-new", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 3002, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + + // GraphQL hydration for PRs 3000, 3001, 3002 and user 1 reg.Register( httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), httpmock.GraphQLQuery(heredoc.Docf(` @@ -617,19 +537,45 @@ func TestListSessionsForRepo(t *testing.T) { "nodes": [ { "__typename": "PullRequest", - "id": "PR_node", - "fullDatabaseId": "2000", - "number": 42, + "id": "PR_node3000", + "fullDatabaseId": "3000", + "number": 100, "title": "Improve docs", "state": "OPEN", "isDraft": true, - "url": "https://github.com/OWNER/REPO/pull/42", + "url": "https://github.com/OWNER/REPO/pull/100", "body": "", "createdAt": "%[1]s", "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } + "repository": {"nameWithOwner": "OWNER/REPO"} + }, + { + "__typename": "PullRequest", + "id": "PR_node3001", + "fullDatabaseId": "3001", + "number": 101, + "title": "Improve docs", + "state": "OPEN", + "isDraft": true, + "url": "https://github.com/OWNER/REPO/pull/101", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "repository": {"nameWithOwner": "OWNER/REPO"} + }, + { + "__typename": "PullRequest", + "id": "PR_node3002", + "fullDatabaseId": "3002", + "number": 102, + "title": "Improve docs", + "state": "OPEN", + "isDraft": true, + "url": "https://github.com/OWNER/REPO/pull/102", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "repository": {"nameWithOwner": "OWNER/REPO"} }, { "__typename": "User", @@ -642,32 +588,32 @@ func TestListSessionsForRepo(t *testing.T) { }`, sampleDateString, ), func(q string, vars map[string]interface{}) { - assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "U_kgAB"}, vars["ids"]) + // Expected encoded node IDs for resource IDs 3000,3001,3002 and user octocat + assert.Equal(t, []interface{}{"PR_kwDNA-jNC7g", "PR_kwDNA-jNC7k", "PR_kwDNA-jNC7o", "U_kgAB"}, vars["ids"]) }), ) }, wantOut: []*Session{ { - ID: "sess1", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "pull", - ResourceID: 2000, - CreatedAt: sampleDate, - PremiumRequests: 0.1, + ID: "sessA-new", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 3000, + CreatedAt: sampleDate, PullRequest: &api.PullRequest{ - ID: "PR_node", - FullDatabaseID: "2000", - Number: 42, + ID: "PR_node3000", + FullDatabaseID: "3000", + Number: 100, Title: "Improve docs", State: "OPEN", IsDraft: true, - URL: "https://github.com/OWNER/REPO/pull/42", + URL: "https://github.com/OWNER/REPO/pull/100", Body: "", CreatedAt: sampleDate, UpdatedAt: sampleDate, @@ -675,24 +621,80 @@ func TestListSessionsForRepo(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, - User: &api.GitHubUser{ - Login: "octocat", - Name: "Octocat", - DatabaseID: 1, + User: &api.GitHubUser{Login: "octocat", Name: "Octocat", DatabaseID: 1}, + }, + { + ID: "sessB-new", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 3001, + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + ID: "PR_node3001", + FullDatabaseID: "3001", + Number: 101, + Title: "Improve docs", + State: "OPEN", + IsDraft: true, + URL: "https://github.com/OWNER/REPO/pull/101", + Body: "", + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, }, + User: &api.GitHubUser{Login: "octocat", Name: "Octocat", DatabaseID: 1}, + }, + { + ID: "sessC-new", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 3002, + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + ID: "PR_node3002", + FullDatabaseID: "3002", + Number: 102, + Title: "Improve docs", + State: "OPEN", + IsDraft: true, + URL: "https://github.com/OWNER/REPO/pull/102", + Body: "", + CreatedAt: sampleDate, + UpdatedAt: sampleDate, + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + User: &api.GitHubUser{Login: "octocat", Name: "Octocat", DatabaseID: 1}, }, }, }, { - // This happens at the early moments of a session lifecycle, before a PR is created and associated with it. - name: "single session, no pull request resource", - limit: 10, + name: "multiple pages with zero resource IDs all kept", + perPage: 2, + limit: 3, httpStubs: func(t *testing.T, reg *httpmock.Registry) { + // Page 1 returns newest sessions, one with a zero resource ID reg.Register( httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ "page_number": {"1"}, - "page_size": {"50"}, + "page_size": {"2"}, + "sort": {"last_updated_at,desc"}, }), "api.githubcopilot.com", ), @@ -700,7 +702,7 @@ func TestListSessionsForRepo(t *testing.T) { { "sessions": [ { - "id": "sess1", + "id": "sessA-new", "name": "Build artifacts", "user_id": 1, "agent_id": 2, @@ -708,90 +710,22 @@ func TestListSessionsForRepo(t *testing.T) { "state": "completed", "owner_id": 10, "repo_id": 1000, + "resource_type": "pull", + "resource_id": 3000, + "created_at": "%[1]s" + }, + { + "id": "sessB-new", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "queued", + "owner_id": 10, + "repo_id": 1000, "resource_type": "", "resource_id": 0, - "created_at": "%[1]s", - "premium_requests": 0.1 - } - ] - }`, - sampleDateString, - )), - ) - // GraphQL hydration - reg.Register( - httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), - httpmock.GraphQLQuery(heredoc.Docf(` - { - "data": { - "nodes": [ - { - "__typename": "User", - "login": "octocat", - "name": "Octocat", - "databaseId": 1 - } - ] - } - }`, - sampleDateString, - ), func(q string, vars map[string]interface{}) { - assert.Equal(t, []interface{}{"U_kgAB"}, vars["ids"]) - }), - ) - }, - wantOut: []*Session{ - { - - ID: "sess1", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "", - ResourceID: 0, - CreatedAt: sampleDate, - PremiumRequests: 0.1, - User: &api.GitHubUser{ - Login: "octocat", - Name: "Octocat", - DatabaseID: 1, - }, - }, - }, - }, - { - 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", - "premium_requests": 0.1 + "created_at": "%[1]s" } ] }`, @@ -799,12 +733,13 @@ func TestListSessionsForRepo(t *testing.T) { )), ) - // Second page + // Page 2 returns older duplicate sessions for 3000, plus another new session with zero resource ID reg.Register( httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ "page_number": {"2"}, - "page_size": {"1"}, + "page_size": {"2"}, + "sort": {"last_updated_at,desc"}, }), "api.githubcopilot.com", ), @@ -812,7 +747,7 @@ func TestListSessionsForRepo(t *testing.T) { { "sessions": [ { - "id": "sess2", + "id": "sessA-old", "name": "Build artifacts", "user_id": 1, "agent_id": 2, @@ -821,16 +756,29 @@ func TestListSessionsForRepo(t *testing.T) { "owner_id": 10, "repo_id": 1000, "resource_type": "pull", - "resource_id": 2001, - "created_at": "%[1]s", - "premium_requests": 0.1 + "resource_id": 3000, + "created_at": "%[1]s" + }, + { + "id": "sessC-new", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "queued", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "", + "resource_id": 0, + "created_at": "%[1]s" } ] }`, sampleDateString, )), ) - // GraphQL hydration + + // GraphQL hydration for PRs 3000 and user 1 reg.Register( httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), httpmock.GraphQLQuery(heredoc.Docf(` @@ -839,35 +787,17 @@ func TestListSessionsForRepo(t *testing.T) { "nodes": [ { "__typename": "PullRequest", - "id": "PR_node", - "fullDatabaseId": "2000", - "number": 42, + "id": "PR_node3000", + "fullDatabaseId": "3000", + "number": 100, "title": "Improve docs", "state": "OPEN", "isDraft": true, - "url": "https://github.com/OWNER/REPO/pull/42", + "url": "https://github.com/OWNER/REPO/pull/100", "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", - "isDraft": true, - "url": "https://github.com/OWNER/REPO/pull/43", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } + "repository": {"nameWithOwner": "OWNER/REPO"} }, { "__typename": "User", @@ -880,32 +810,32 @@ func TestListSessionsForRepo(t *testing.T) { }`, sampleDateString, ), func(q string, vars map[string]interface{}) { - assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "PR_kwDNA-jNB9E", "U_kgAB"}, vars["ids"]) + // Expected encoded node IDs for resource IDs 3000 and user octocat + assert.Equal(t, []interface{}{"PR_kwDNA-jNC7g", "U_kgAB"}, vars["ids"]) }), ) }, wantOut: []*Session{ { - ID: "sess1", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "pull", - ResourceID: 2000, - CreatedAt: sampleDate, - PremiumRequests: 0.1, + ID: "sessA-new", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 3000, + CreatedAt: sampleDate, PullRequest: &api.PullRequest{ - ID: "PR_node", - FullDatabaseID: "2000", - Number: 42, + ID: "PR_node3000", + FullDatabaseID: "3000", + Number: 100, Title: "Improve docs", State: "OPEN", IsDraft: true, - URL: "https://github.com/OWNER/REPO/pull/42", + URL: "https://github.com/OWNER/REPO/pull/100", Body: "", CreatedAt: sampleDate, UpdatedAt: sampleDate, @@ -913,45 +843,35 @@ func TestListSessionsForRepo(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, - User: &api.GitHubUser{ - Login: "octocat", - Name: "Octocat", - DatabaseID: 1, - }, + User: &api.GitHubUser{Login: "octocat", Name: "Octocat", DatabaseID: 1}, }, { - ID: "sess2", - Name: "Build artifacts", - UserID: 1, - AgentID: 2, - Logs: "", - State: "completed", - OwnerID: 10, - RepoID: 1000, - ResourceType: "pull", - ResourceID: 2001, - CreatedAt: sampleDate, - PremiumRequests: 0.1, - PullRequest: &api.PullRequest{ - ID: "PR_node", - FullDatabaseID: "2001", - Number: 43, - Title: "Improve docs", - State: "OPEN", - IsDraft: true, - URL: "https://github.com/OWNER/REPO/pull/43", - Body: "", - CreatedAt: sampleDate, - UpdatedAt: sampleDate, - Repository: &api.PRRepository{ - NameWithOwner: "OWNER/REPO", - }, - }, - User: &api.GitHubUser{ - Login: "octocat", - Name: "Octocat", - DatabaseID: 1, - }, + ID: "sessB-new", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "queued", + OwnerID: 10, + RepoID: 1000, + ResourceType: "", + ResourceID: 0, + CreatedAt: sampleDate, + User: &api.GitHubUser{Login: "octocat", Name: "Octocat", DatabaseID: 1}, + }, + { + ID: "sessC-new", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "queued", + OwnerID: 10, + RepoID: 1000, + ResourceType: "", + ResourceID: 0, + CreatedAt: sampleDate, + User: &api.GitHubUser{Login: "octocat", Name: "Octocat", DatabaseID: 1}, }, }, }, @@ -961,7 +881,7 @@ func TestListSessionsForRepo(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ "page_number": {"1"}, "page_size": {"50"}, }), @@ -977,7 +897,7 @@ func TestListSessionsForRepo(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/sessions/nwo/OWNER/REPO", url.Values{ + httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ "page_number": {"1"}, "page_size": {"50"}, }), @@ -1036,7 +956,7 @@ func TestListSessionsForRepo(t *testing.T) { }() } - sessions, err := capiClient.ListSessionsForRepo(context.Background(), "OWNER", "REPO", tt.limit) + sessions, err := capiClient.ListLatestSessionsForViewer(context.Background(), tt.limit) if tt.wantErr != "" { require.ErrorContains(t, err, tt.wantErr) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index 5886955e6..4370a0170 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/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" @@ -24,7 +23,6 @@ type ListOptions struct { IO *iostreams.IOStreams Limit int CapiClient func() (capi.CapiClient, error) - BaseRepo func() (ghrepo.Interface, error) Web bool Browser browser.Browser } @@ -40,12 +38,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "list", - Short: "List agent tasks", + Short: "List agent tasks (preview)", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - // Support -R/--repo override - opts.BaseRepo = f.BaseRepo - if opts.Limit < 1 { return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit) } @@ -56,8 +51,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman }, } - 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") @@ -66,10 +59,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman func listRun(opts *ListOptions) error { if opts.Web { - // Currently the web GUI does not have a page that supports filtering - // based on repo, so we just open the agents dashboard with no args. - // If that page is ever added in the future, we should route to that - // page instead of the global one when --repo is set. webURL := capi.AgentsHomeURL if opts.IO.IsStdoutTTY() { fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(webURL)) @@ -91,24 +80,11 @@ func listRun(opts *ListOptions) error { var sessions []*capi.Session ctx := context.Background() - var repo ghrepo.Interface - if opts.BaseRepo != nil { - // We swallow this error because when CWD is not a repo and - // the --repo flag is not set, we use the global/user session listing. - repo, _ = opts.BaseRepo() + sessions, err = capiClient.ListLatestSessionsForViewer(ctx, opts.Limit) + if err != nil { + return err } - if repo != nil && repo.RepoOwner() != "" && repo.RepoName() != "" { - sessions, err = capiClient.ListSessionsForRepo(ctx, repo.RepoOwner(), repo.RepoName(), opts.Limit) - if err != nil { - return err - } - } else { - sessions, err = capiClient.ListSessionsForViewer(ctx, opts.Limit) - if err != nil { - return err - } - } opts.IO.StopProgressIndicator() if len(sessions) == 0 { @@ -121,6 +97,12 @@ func listRun(opts *ListOptions) error { fmt.Fprintf(opts.IO.ErrOut, "error starting pager: %v\n", err) } + if opts.IO.IsStdoutTTY() { + count := len(sessions) + header := fmt.Sprintf("Showing %s", text.Pluralize(count, "session")) + fmt.Fprintf(opts.IO.Out, "%s\n\n", header) + } + cs := opts.IO.ColorScheme() tp := tableprinter.New(opts.IO, tableprinter.WithHeader("Session Name", "Pull Request", "Repo", "Session State", "Created")) for _, s := range sessions { @@ -145,9 +127,9 @@ func listRun(opts *ListOptions) error { // State if tp.IsTTY() { - tp.AddField(s.State, tableprinter.WithColor(shared.ColorFuncForSessionState(*s, cs))) + tp.AddField(shared.SessionStateString(s.State), tableprinter.WithColor(shared.ColorFuncForSessionState(*s, cs))) } else { - tp.AddField(s.State) + tp.AddField(shared.SessionStateString(s.State)) } // Created diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index 9b981f28a..747650283 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -3,7 +3,6 @@ package list import ( "bytes" "context" - "errors" "io" "testing" "time" @@ -11,7 +10,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/browser" - "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -22,11 +20,10 @@ import ( func TestNewCmdList(t *testing.T) { tests := []struct { - name string - args string - wantOpts ListOptions - wantBaseRepo ghrepo.Interface - wantErr string + name string + args string + wantOpts ListOptions + wantErr string }{ { name: "no arguments", @@ -34,14 +31,6 @@ 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", @@ -96,12 +85,6 @@ 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)) - } }) } } @@ -114,8 +97,6 @@ func Test_listRun(t *testing.T) { name string tty bool capiStubs func(*testing.T, *capi.CapiClientMock) - baseRepo ghrepo.Interface - baseRepoErr error limit int web bool wantOut string @@ -127,7 +108,7 @@ func Test_listRun(t *testing.T) { 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) { + m.ListLatestSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { return nil, nil } }, @@ -138,7 +119,7 @@ func Test_listRun(t *testing.T) { tty: true, limit: 999, capiStubs: func(t *testing.T, m *capi.CapiClientMock) { - m.ListSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + m.ListLatestSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { assert.Equal(t, 999, limit) return nil, nil } @@ -149,7 +130,7 @@ func Test_listRun(t *testing.T) { 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) { + m.ListLatestSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { return []*capi.Session{ { ID: "id1", @@ -168,15 +149,17 @@ func Test_listRun(t *testing.T) { } }, wantOut: heredoc.Doc(` - SESSION NAME PULL REQUEST REPO SESSION STATE CREATED - s1 #101 OWNER/REPO completed about 6 hours ago + Showing 1 session + + SESSION NAME PULL REQUEST REPO SESSION STATE CREATED + s1 #101 OWNER/REPO Ready for review about 6 hours ago `), }, { 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) { + m.ListLatestSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { return []*capi.Session{ { ID: "id1", @@ -194,13 +177,13 @@ func Test_listRun(t *testing.T) { }, nil } }, - wantOut: "s1\t#101\tOWNER/REPO\tcompleted\t" + sampleDateString + "\n", // header omitted for non-tty + wantOut: "s1\t#101\tOWNER/REPO\tReady for review\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) { + m.ListLatestSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { return []*capi.Session{ { ID: "id1", @@ -257,7 +240,7 @@ func Test_listRun(t *testing.T) { { ID: "id5", Name: "s5", - State: "canceled", + State: "cancelled", CreatedAt: sampleDate, ResourceType: "pull", PullRequest: &api.PullRequest{ @@ -284,204 +267,17 @@ func Test_listRun(t *testing.T) { } }, wantOut: heredoc.Doc(` - SESSION NAME 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 + Showing 6 sessions + + SESSION NAME PULL REQUEST REPO SESSION STATE CREATED + s1 #101 OWNER/REPO Ready for review 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 Cancelled about 6 hours ago + s6 #106 OWNER/REPO mystery about 6 hours ago `), }, - { - name: "repo-scoped no sessions", - 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 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: "id1", - Name: "s1", - State: "completed", - CreatedAt: sampleDate, - ResourceType: "pull", - PullRequest: &api.PullRequest{ - Number: 101, - Repository: &api.PRRepository{ - NameWithOwner: "OWNER/REPO", - }, - }, - }, - }, nil - } - }, - wantOut: heredoc.Doc(` - SESSION NAME PULL REQUEST REPO SESSION STATE CREATED - s1 #101 OWNER/REPO completed about 6 hours ago - `), - }, - { - name: "repo-scoped single session (nontty)", - tty: false, - 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: "id1", - Name: "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: "id1", - Name: "s1", - State: "completed", - CreatedAt: sampleDate, - ResourceType: "pull", - PullRequest: &api.PullRequest{ - Number: 101, - Repository: &api.PRRepository{ - NameWithOwner: "OWNER/REPO", - }, - }, - }, - { - ID: "id2", - Name: "s2", - State: "failed", - CreatedAt: sampleDate, - ResourceType: "pull", - PullRequest: &api.PullRequest{ - Number: 102, - Repository: &api.PRRepository{ - NameWithOwner: "OWNER/REPO", - }, - }, - }, - { - ID: "id3", - Name: "s3", - State: "in_progress", - CreatedAt: sampleDate, - ResourceType: "pull", - PullRequest: &api.PullRequest{ - Number: 103, - Repository: &api.PRRepository{ - NameWithOwner: "OWNER/REPO", - }, - }, - }, - { - ID: "id4", - Name: "s4", - State: "queued", - CreatedAt: sampleDate, - ResourceType: "pull", - PullRequest: &api.PullRequest{ - Number: 104, - Repository: &api.PRRepository{ - NameWithOwner: "OWNER/REPO", - }, - }, - }, - { - ID: "id5", - Name: "s5", - State: "canceled", - CreatedAt: sampleDate, - ResourceType: "pull", - PullRequest: &api.PullRequest{ - Number: 105, - Repository: &api.PRRepository{ - NameWithOwner: "OWNER/REPO", - }, - }, - }, - { - ID: "id6", - Name: "s6", - State: "mystery", - CreatedAt: sampleDate, - ResourceType: "pull", - PullRequest: &api.PullRequest{ - Number: 106, - Repository: &api.PRRepository{ - NameWithOwner: "OWNER/REPO", - }, - }, - }, - }, nil - } - }, - wantOut: heredoc.Doc(` - SESSION NAME 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"), - 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", tty: true, @@ -490,15 +286,6 @@ func Test_listRun(t *testing.T) { wantStderr: "Opening https://github.com/copilot/agents in your browser.\n", wantBrowserURL: "https://github.com/copilot/agents", }, - { - name: "web mode with repo still uses global URL, even when --repo is set", - tty: true, - web: true, - baseRepo: ghrepo.New("OWNER", "REPO"), - wantOut: "", - wantStderr: "Opening https://github.com/copilot/agents in your browser.\n", - wantBrowserURL: "https://github.com/copilot/agents", - }, } for _, tt := range tests { @@ -528,11 +315,6 @@ func Test_listRun(t *testing.T) { return capiClientMock, nil }, } - if tt.baseRepo != nil || tt.baseRepoErr != nil { - baseRepo := tt.baseRepo - baseRepoErr := tt.baseRepoErr - opts.BaseRepo = func() (ghrepo.Interface, error) { return baseRepo, baseRepoErr } - } err := listRun(opts) if tt.wantErr != nil { diff --git a/pkg/cmd/agent-task/shared/display.go b/pkg/cmd/agent-task/shared/display.go index 99ae91c43..3859a0e75 100644 --- a/pkg/cmd/agent-task/shared/display.go +++ b/pkg/cmd/agent-task/shared/display.go @@ -11,7 +11,7 @@ func ColorFuncForSessionState(s capi.Session, cs *iostreams.ColorScheme) func(st switch s.State { case "completed": stateColor = cs.Green - case "canceled": + case "cancelled": stateColor = cs.Muted case "in_progress", "queued": stateColor = cs.Yellow diff --git a/pkg/cmd/agent-task/shared/log_mock.go b/pkg/cmd/agent-task/shared/log_mock.go index e4096e273..f12cf6bf3 100644 --- a/pkg/cmd/agent-task/shared/log_mock.go +++ b/pkg/cmd/agent-task/shared/log_mock.go @@ -19,10 +19,10 @@ var _ LogRenderer = &LogRendererMock{} // // // make and configure a mocked LogRenderer // mockedLogRenderer := &LogRendererMock{ -// FollowFunc: func(fetcher func() ([]byte, error), w io.Writer, cs *iostreams.IOStreams) error { +// FollowFunc: func(fetcher func() ([]byte, error), w io.Writer, ioMoqParam *iostreams.IOStreams) error { // panic("mock out the Follow method") // }, -// RenderFunc: func(logs []byte, w io.Writer, cs *iostreams.IOStreams) (bool, error) { +// RenderFunc: func(logs []byte, w io.Writer, ioMoqParam *iostreams.IOStreams) (bool, error) { // panic("mock out the Render method") // }, // } @@ -33,10 +33,10 @@ var _ LogRenderer = &LogRendererMock{} // } type LogRendererMock struct { // FollowFunc mocks the Follow method. - FollowFunc func(fetcher func() ([]byte, error), w io.Writer, cs *iostreams.IOStreams) error + FollowFunc func(fetcher func() ([]byte, error), w io.Writer, ioMoqParam *iostreams.IOStreams) error // RenderFunc mocks the Render method. - RenderFunc func(logs []byte, w io.Writer, cs *iostreams.IOStreams) (bool, error) + RenderFunc func(logs []byte, w io.Writer, ioMoqParam *iostreams.IOStreams) (bool, error) // calls tracks calls to the methods. calls struct { @@ -46,8 +46,8 @@ type LogRendererMock struct { Fetcher func() ([]byte, error) // W is the w argument value. W io.Writer - // Cs is the cs argument value. - Cs *iostreams.IOStreams + // IoMoqParam is the ioMoqParam argument value. + IoMoqParam *iostreams.IOStreams } // Render holds details about calls to the Render method. Render []struct { @@ -55,8 +55,8 @@ type LogRendererMock struct { Logs []byte // W is the w argument value. W io.Writer - // Cs is the cs argument value. - Cs *iostreams.IOStreams + // IoMoqParam is the ioMoqParam argument value. + IoMoqParam *iostreams.IOStreams } } lockFollow sync.RWMutex @@ -64,23 +64,23 @@ type LogRendererMock struct { } // Follow calls FollowFunc. -func (mock *LogRendererMock) Follow(fetcher func() ([]byte, error), w io.Writer, cs *iostreams.IOStreams) error { +func (mock *LogRendererMock) Follow(fetcher func() ([]byte, error), w io.Writer, ioMoqParam *iostreams.IOStreams) error { if mock.FollowFunc == nil { panic("LogRendererMock.FollowFunc: method is nil but LogRenderer.Follow was just called") } callInfo := struct { - Fetcher func() ([]byte, error) - W io.Writer - Cs *iostreams.IOStreams + Fetcher func() ([]byte, error) + W io.Writer + IoMoqParam *iostreams.IOStreams }{ - Fetcher: fetcher, - W: w, - Cs: cs, + Fetcher: fetcher, + W: w, + IoMoqParam: ioMoqParam, } mock.lockFollow.Lock() mock.calls.Follow = append(mock.calls.Follow, callInfo) mock.lockFollow.Unlock() - return mock.FollowFunc(fetcher, w, cs) + return mock.FollowFunc(fetcher, w, ioMoqParam) } // FollowCalls gets all the calls that were made to Follow. @@ -88,14 +88,14 @@ func (mock *LogRendererMock) Follow(fetcher func() ([]byte, error), w io.Writer, // // len(mockedLogRenderer.FollowCalls()) func (mock *LogRendererMock) FollowCalls() []struct { - Fetcher func() ([]byte, error) - W io.Writer - Cs *iostreams.IOStreams + Fetcher func() ([]byte, error) + W io.Writer + IoMoqParam *iostreams.IOStreams } { var calls []struct { - Fetcher func() ([]byte, error) - W io.Writer - Cs *iostreams.IOStreams + Fetcher func() ([]byte, error) + W io.Writer + IoMoqParam *iostreams.IOStreams } mock.lockFollow.RLock() calls = mock.calls.Follow @@ -104,23 +104,23 @@ func (mock *LogRendererMock) FollowCalls() []struct { } // Render calls RenderFunc. -func (mock *LogRendererMock) Render(logs []byte, w io.Writer, cs *iostreams.IOStreams) (bool, error) { +func (mock *LogRendererMock) Render(logs []byte, w io.Writer, ioMoqParam *iostreams.IOStreams) (bool, error) { if mock.RenderFunc == nil { panic("LogRendererMock.RenderFunc: method is nil but LogRenderer.Render was just called") } callInfo := struct { - Logs []byte - W io.Writer - Cs *iostreams.IOStreams + Logs []byte + W io.Writer + IoMoqParam *iostreams.IOStreams }{ - Logs: logs, - W: w, - Cs: cs, + Logs: logs, + W: w, + IoMoqParam: ioMoqParam, } mock.lockRender.Lock() mock.calls.Render = append(mock.calls.Render, callInfo) mock.lockRender.Unlock() - return mock.RenderFunc(logs, w, cs) + return mock.RenderFunc(logs, w, ioMoqParam) } // RenderCalls gets all the calls that were made to Render. @@ -128,14 +128,14 @@ func (mock *LogRendererMock) Render(logs []byte, w io.Writer, cs *iostreams.IOSt // // len(mockedLogRenderer.RenderCalls()) func (mock *LogRendererMock) RenderCalls() []struct { - Logs []byte - W io.Writer - Cs *iostreams.IOStreams + Logs []byte + W io.Writer + IoMoqParam *iostreams.IOStreams } { var calls []struct { - Logs []byte - W io.Writer - Cs *iostreams.IOStreams + Logs []byte + W io.Writer + IoMoqParam *iostreams.IOStreams } mock.lockRender.RLock() calls = mock.calls.Render