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] 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,