From 680ebade594c3dc80bbdc6872a78369b6ad842cd Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 17 Sep 2025 17:47:50 -0600 Subject: [PATCH 01/10] Remove repo-scoped session listing and related code This commit removes support for listing agent sessions scoped to a specific repository, including the ListSessionsForRepo method and related tests. The list command now always lists the latest sessions for the viewer, simplifying the code and user experience. Test and mock code have been updated accordingly. --- pkg/cmd/agent-task/capi/client.go | 3 +- pkg/cmd/agent-task/capi/client_mock.go | 174 +++----- pkg/cmd/agent-task/capi/sessions.go | 88 +--- pkg/cmd/agent-task/capi/sessions_test.go | 524 +---------------------- pkg/cmd/agent-task/list/list.go | 31 +- pkg/cmd/agent-task/list/list_test.go | 244 +---------- 6 files changed, 102 insertions(+), 962 deletions(-) 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..f6825e674 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) - + var seenResources []int64 + var latestSessions []session 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) @@ -127,17 +129,30 @@ func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*S } sessions = append(sessions, response.Sessions...) - if len(response.Sessions) < pageSize || len(sessions) >= limit { + + // De-duplicate sessions by resource ID. + // Because the API returns newest first, + // we can safely skip any additional sessions + // for a resource we have already seen. + for _, s := range sessions { + if slices.Contains(seenResources, s.ResourceID) { + continue + } + seenResources = append(seenResources, s.ResourceID) + latestSessions = append(latestSessions, s) + } + + 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 +160,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..1c49444db 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -514,529 +514,7 @@ func TestListSessionsForViewer(t *testing.T) { }() } - 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", - "premium_requests": 0.1 - } - ] - }`, - sampleDateString, - )), - ) - // GraphQL hydration - reg.Register( - httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), - httpmock.GraphQLQuery(heredoc.Docf(` - { - "data": { - "nodes": [ - { - "__typename": "PullRequest", - "id": "PR_node", - "fullDatabaseId": "2000", - "number": 42, - "title": "Improve docs", - "state": "OPEN", - "isDraft": true, - "url": "https://github.com/OWNER/REPO/pull/42", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } - }, - { - "__typename": "User", - "login": "octocat", - "name": "Octocat", - "databaseId": 1 - } - ] - } - }`, - sampleDateString, - ), func(q string, vars map[string]interface{}) { - assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "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, - PullRequest: &api.PullRequest{ - ID: "PR_node", - FullDatabaseID: "2000", - Number: 42, - Title: "Improve docs", - State: "OPEN", - IsDraft: true, - URL: "https://github.com/OWNER/REPO/pull/42", - 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, - 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": "", - "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 - } - ] - }`, - 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", - "premium_requests": 0.1 - } - ] - }`, - sampleDateString, - )), - ) - // GraphQL hydration - reg.Register( - httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), - httpmock.GraphQLQuery(heredoc.Docf(` - { - "data": { - "nodes": [ - { - "__typename": "PullRequest", - "id": "PR_node", - "fullDatabaseId": "2000", - "number": 42, - "title": "Improve docs", - "state": "OPEN", - "isDraft": true, - "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", - "isDraft": true, - "url": "https://github.com/OWNER/REPO/pull/43", - "body": "", - "createdAt": "%[1]s", - "updatedAt": "%[1]s", - "repository": { - "nameWithOwner": "OWNER/REPO" - } - }, - { - "__typename": "User", - "login": "octocat", - "name": "Octocat", - "databaseId": 1 - } - ] - } - }`, - sampleDateString, - ), func(q string, vars map[string]interface{}) { - assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "PR_kwDNA-jNB9E", "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, - PullRequest: &api.PullRequest{ - ID: "PR_node", - FullDatabaseID: "2000", - Number: 42, - Title: "Improve docs", - State: "OPEN", - IsDraft: true, - URL: "https://github.com/OWNER/REPO/pull/42", - Body: "", - CreatedAt: sampleDate, - UpdatedAt: sampleDate, - Repository: &api.PRRepository{ - NameWithOwner: "OWNER/REPO", - }, - }, - 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, - }, - }, - }, - }, - { - 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", - "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.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..0eb7fd34a 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -56,8 +56,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 +64,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 +85,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 +102,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 { diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index 9b981f28a..b9a750a6d 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,6 +149,8 @@ func Test_listRun(t *testing.T) { } }, wantOut: heredoc.Doc(` + Showing 1 session + SESSION NAME PULL REQUEST REPO SESSION STATE CREATED s1 #101 OWNER/REPO completed about 6 hours ago `), @@ -176,7 +159,7 @@ func Test_listRun(t *testing.T) { 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", @@ -200,7 +183,7 @@ func Test_listRun(t *testing.T) { 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", @@ -284,6 +267,8 @@ func Test_listRun(t *testing.T) { } }, wantOut: heredoc.Doc(` + Showing 6 sessions + 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 @@ -293,195 +278,6 @@ func Test_listRun(t *testing.T) { 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 { From 08bc1a8d29088418407427ff9802fb39327af861 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 17 Sep 2025 20:48:09 -0600 Subject: [PATCH 02/10] Fix session deduplication and add test for paging Refactor ListLatestSessionsForViewer to use a map for tracking seen resource IDs, ensuring only the newest session per resource is kept. Add a test case to verify correct deduplication across paginated API responses. --- pkg/cmd/agent-task/capi/sessions.go | 22 +- pkg/cmd/agent-task/capi/sessions_test.go | 252 ++++++++++++++++++++++- 2 files changed, 264 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index f6825e674..c34732d9b 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -99,8 +99,8 @@ func (c *CAPIClient) ListLatestSessionsForViewer(ctx context.Context, limit int) pageSize := defaultSessionsPerPage sessions := make([]session, 0, limit+pageSize) - var seenResources []int64 - var latestSessions []session + 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 { @@ -128,18 +128,22 @@ func (c *CAPIClient) ListLatestSessionsForViewer(ctx context.Context, limit int) return nil, fmt.Errorf("failed to decode sessions response: %w", err) } - sessions = append(sessions, response.Sessions...) + // 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, - // we can safely skip any additional sessions - // for a resource we have already seen. - for _, s := range sessions { - if slices.Contains(seenResources, s.ResourceID) { + // 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 } - seenResources = append(seenResources, s.ResourceID) + seenResources[s.ResourceID] = struct{}{} latestSessions = append(latestSessions, s) + if len(latestSessions) >= limit { + break + } } if len(response.Sessions) < pageSize || len(latestSessions) >= limit { diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 1c49444db..26a079501 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) @@ -433,6 +433,256 @@ func TestListSessionsForViewer(t *testing.T) { }, }, }, + { + 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": {"2"}, + "sort": {"last_updated_at,desc"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sessA-new", + "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": "sessB-new", + "name": "Build artifacts", + "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" + } + ] + }`, + sampleDateString, + )), + ) + + // Page 2 returns older duplicate sessions for 3000 and 3001, 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(` + { + "data": { + "nodes": [ + { + "__typename": "PullRequest", + "id": "PR_node3000", + "fullDatabaseId": "3000", + "number": 100, + "title": "Improve docs", + "state": "OPEN", + "isDraft": true, + "url": "https://github.com/OWNER/REPO/pull/100", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "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", + "login": "octocat", + "name": "Octocat", + "databaseId": 1 + } + ] + } + }`, + sampleDateString, + ), func(q string, vars map[string]interface{}) { + // Expected encoded node IDs for resource IDs 3000,3001,3002 + assert.Equal(t, []interface{}{"PR_kwDNA-jNC7g", "PR_kwDNA-jNC7k", "PR_kwDNA-jNC7o", "U_kgAB"}, vars["ids"]) + }), + ) + }, + wantOut: []*Session{ + { + 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_node3000", + FullDatabaseID: "3000", + Number: 100, + Title: "Improve docs", + State: "OPEN", + IsDraft: true, + URL: "https://github.com/OWNER/REPO/pull/100", + 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: "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}, + }, + }, + }, { name: "API error", limit: 10, From 6ec4324d6d1dd93545bef697b1aac0efef9e903b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 11:40:51 +0100 Subject: [PATCH 03/10] docs(agent-task/capi): fix test case explanation Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 26a079501..4eb0bc199 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -483,7 +483,7 @@ func TestListLatestSessionsForViewer(t *testing.T) { )), ) - // Page 2 returns older duplicate sessions for 3000 and 3001, plus another new PR 3002 + // Page 2 returns older duplicate sessions for 3000, plus another new PR 3002 reg.Register( httpmock.WithHost( httpmock.QueryMatcher("GET", "agents/sessions", url.Values{ From 5f73d3b43848e20e3f6b1f065cceada5bf1eb21b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 11:42:02 +0100 Subject: [PATCH 04/10] refactor(agent-task list): remove unused `BaseRepo` from `ListOptions` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/list/list.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index 0eb7fd34a..f1fca365d 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 } @@ -43,9 +41,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Short: "List agent tasks", 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) } From c68d1a31c05febd8ff803e3a0961e307a3c148e0 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 11:53:44 +0100 Subject: [PATCH 05/10] fix(agent-task/capi): keep sessions with no resource attached Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index c34732d9b..b379f6543 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -139,7 +139,13 @@ func (c *CAPIClient) ListLatestSessionsForViewer(ctx context.Context, limit int) if _, exists := seenResources[s.ResourceID]; exists { continue } - seenResources[s.ResourceID] = struct{}{} + + // 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 From 6235c33ff000225b12de8d7c7e29fcc18b527ea4 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 11:54:20 +0100 Subject: [PATCH 06/10] test(agent-task/capi): assert session with zero resource ID are kept Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions_test.go | 194 ++++++++++++++++++++++- 1 file changed, 193 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 4eb0bc199..b91a8b401 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -588,7 +588,7 @@ func TestListLatestSessionsForViewer(t *testing.T) { }`, sampleDateString, ), func(q string, vars map[string]interface{}) { - // Expected encoded node IDs for resource IDs 3000,3001,3002 + // 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"]) }), ) @@ -683,6 +683,198 @@ func TestListLatestSessionsForViewer(t *testing.T) { }, }, }, + { + 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", url.Values{ + "page_number": {"1"}, + "page_size": {"2"}, + "sort": {"last_updated_at,desc"}, + }), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "sessions": [ + { + "id": "sessA-new", + "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": "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" + } + ] + }`, + sampleDateString, + )), + ) + + // 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", 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": "queued", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "", + "resource_id": 0, + "created_at": "%[1]s" + } + ] + }`, + sampleDateString, + )), + ) + + // GraphQL hydration for PRs 3000 and user 1 + reg.Register( + httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), + httpmock.GraphQLQuery(heredoc.Docf(` + { + "data": { + "nodes": [ + { + "__typename": "PullRequest", + "id": "PR_node3000", + "fullDatabaseId": "3000", + "number": 100, + "title": "Improve docs", + "state": "OPEN", + "isDraft": true, + "url": "https://github.com/OWNER/REPO/pull/100", + "body": "", + "createdAt": "%[1]s", + "updatedAt": "%[1]s", + "repository": {"nameWithOwner": "OWNER/REPO"} + }, + { + "__typename": "User", + "login": "octocat", + "name": "Octocat", + "databaseId": 1 + } + ] + } + }`, + sampleDateString, + ), func(q string, vars map[string]interface{}) { + // 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: "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_node3000", + FullDatabaseID: "3000", + Number: 100, + Title: "Improve docs", + State: "OPEN", + IsDraft: true, + URL: "https://github.com/OWNER/REPO/pull/100", + 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}, + }, + }, + }, { name: "API error", limit: 10, From 169f45daf9940b0c28c732dd845cbfd372eeef1b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 11:57:28 +0100 Subject: [PATCH 07/10] chore(agent-task/shared): regenerate `LogRenderer` mock Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/shared/log_mock.go | 72 +++++++++++++-------------- 1 file changed, 36 insertions(+), 36 deletions(-) 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 From 7d163eaf27d019a573d227c1ab5756497f6740f3 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 12:20:34 +0100 Subject: [PATCH 08/10] docs(agent-task list): add "(preview)" note to cmd docs Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/list/list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index f1fca365d..c2897acc1 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -38,7 +38,7 @@ 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 { if opts.Limit < 1 { From 764376836ca08b6202dba3bdf4819a176148b7f1 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 12:33:47 +0100 Subject: [PATCH 09/10] fix(agent-task/shared): fix session state value Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/shared/display.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 4a0c32ef9f8fd6ac173d653a62fb25d77e846134 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 18 Sep 2025 12:34:27 +0100 Subject: [PATCH 10/10] fix(agent-task list): show capitalised session state Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/list/list.go | 4 ++-- pkg/cmd/agent-task/list/list_test.go | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index c2897acc1..4370a0170 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -127,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 b9a750a6d..747650283 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -151,8 +151,8 @@ func Test_listRun(t *testing.T) { wantOut: heredoc.Doc(` Showing 1 session - SESSION NAME PULL REQUEST REPO SESSION STATE CREATED - s1 #101 OWNER/REPO completed about 6 hours ago + SESSION NAME PULL REQUEST REPO SESSION STATE CREATED + s1 #101 OWNER/REPO Ready for review about 6 hours ago `), }, { @@ -177,7 +177,7 @@ 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)", @@ -240,7 +240,7 @@ func Test_listRun(t *testing.T) { { ID: "id5", Name: "s5", - State: "canceled", + State: "cancelled", CreatedAt: sampleDate, ResourceType: "pull", PullRequest: &api.PullRequest{ @@ -269,13 +269,13 @@ func Test_listRun(t *testing.T) { wantOut: heredoc.Doc(` Showing 6 sessions - 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 + 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 `), }, {