From 185e36c60f6c512a9880cc3f4a1a44b84efacd47 Mon Sep 17 00:00:00 2001 From: Ariel Deitcher Date: Thu, 18 Sep 2025 15:40:06 -0700 Subject: [PATCH 1/7] update agents resource route --- pkg/cmd/agent-task/capi/sessions.go | 129 +++++---- pkg/cmd/agent-task/capi/sessions_test.go | 354 ++++++++++------------- pkg/cmd/agent-task/view/view.go | 4 - 3 files changed, 222 insertions(+), 265 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index a3989a862..6f64d5a1e 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -27,24 +27,25 @@ var ErrSessionNotFound = errors.New("not found") // session is an in-flight agent task type session struct { - ID string `json:"id"` - Name string `json:"name"` - UserID int64 `json:"user_id"` - AgentID int64 `json:"agent_id"` - Logs string `json:"logs"` - State string `json:"state"` - OwnerID uint64 `json:"owner_id"` - RepoID uint64 `json:"repo_id"` - ResourceType string `json:"resource_type"` - ResourceID int64 `json:"resource_id"` - LastUpdatedAt time.Time `json:"last_updated_at,omitempty"` - CreatedAt time.Time `json:"created_at,omitempty"` - CompletedAt time.Time `json:"completed_at,omitempty"` - EventURL string `json:"event_url"` - EventType string `json:"event_type"` - PremiumRequests float64 `json:"premium_requests"` - WorkflowRunID uint64 `json:"workflow_run_id,omitempty"` - Error *struct { + ID string `json:"id"` + Name string `json:"name"` + UserID int64 `json:"user_id"` + AgentID int64 `json:"agent_id"` + Logs string `json:"logs"` + State string `json:"state"` + OwnerID uint64 `json:"owner_id"` + RepoID uint64 `json:"repo_id"` + ResourceType string `json:"resource_type"` + ResourceID int64 `json:"resource_id"` + ResourceGlobalID string `json:"resource_global_id"` + LastUpdatedAt time.Time `json:"last_updated_at,omitempty"` + CreatedAt time.Time `json:"created_at,omitempty"` + CompletedAt time.Time `json:"completed_at,omitempty"` + EventURL string `json:"event_url"` + EventType string `json:"event_type"` + PremiumRequests float64 `json:"premium_requests"` + WorkflowRunID uint64 `json:"workflow_run_id,omitempty"` + Error *struct { Code string `json:"code"` Message string `json:"message"` } `json:"error,omitempty"` @@ -100,6 +101,26 @@ type SessionError struct { Message string } +type Resource struct { + ID string `json:"id"` + UserID uint64 `json:"user_id"` + ResourceType string `json:"resource_type"` + ResourceID int64 `json:"resource_id"` + ResourceGlobalID string `json:"resource_global_id"` + SessionCount int `json:"session_count"` + SessionLastUpdatedAt int64 `json:"last_updated_at"` + SessionState string `json:"state,omitempty"` + ResourceState string `json:"resource_state"` + Sessions []resourceSession `json:"sessions"` +} + +type resourceSession struct { + SessionID string `json:"id"` + Name string `json:"name"` + SessionState string `json:"state,omitempty"` + SessionLastUpdatedAt int64 `json:"last_updated_at"` +} + // ListLatestSessionsForViewer lists all agent sessions for the // authenticated user up to limit. func (c *CAPIClient) ListLatestSessionsForViewer(ctx context.Context, limit int) ([]*Session, error) { @@ -258,46 +279,39 @@ func (c *CAPIClient) ListSessionsByResourceID(ctx context.Context, resourceType return nil, nil } - url := fmt.Sprintf("%s/agents/sessions/resource/%s/%d", baseCAPIURL, url.PathEscape(resourceType), resourceID) - pageSize := defaultSessionsPerPage + url := fmt.Sprintf("%s/agents/resource/%s/%d", baseCAPIURL, url.PathEscape(resourceType), resourceID) - 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 - } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) + if err != nil { + return nil, err } - // Drop any above the limit - if len(sessions) > limit { - sessions = sessions[:limit] + 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 Resource + if err := json.NewDecoder(res.Body).Decode(&response); err != nil { + return nil, fmt.Errorf("failed to decode sessions response: %w", err) + } + + sessions := make([]session, 0, len(response.Sessions)) + for _, s := range response.Sessions { + sessions = append(sessions, session{ + ID: s.SessionID, + Name: s.Name, + UserID: int64(response.UserID), + ResourceType: response.ResourceType, + ResourceID: response.ResourceID, + ResourceGlobalID: response.ResourceGlobalID, + LastUpdatedAt: time.Unix(s.SessionLastUpdatedAt, 0), + State: s.SessionState, + }) } result, err := c.hydrateSessionPullRequestsAndUsers(sessions) @@ -317,7 +331,10 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* userNodeIds := make([]string, 0, len(sessions)) for _, session := range sessions { if session.ResourceType == "pull" { - prNodeID := generatePullRequestNodeID(int64(session.RepoID), session.ResourceID) + prNodeID := session.ResourceGlobalID + if session.ResourceGlobalID == "" { + prNodeID = generatePullRequestNodeID(int64(session.RepoID), session.ResourceID) + } if !slices.Contains(prNodeIds, prNodeID) { prNodeIds = append(prNodeIds, prNodeID) } diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index daccbeaed..8c13610a2 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -1200,7 +1200,7 @@ func TestListSessionsByResourceIDRequiresResource(t *testing.T) { } func TestListSessionsByResourceID(t *testing.T) { - sampleDateString := "2025-08-29T00:00:00Z" + sampleDateString := "2025-08-29T07:00:00Z" sampleDate, err := time.Parse(time.RFC3339, sampleDateString) require.NoError(t, err) @@ -1226,16 +1226,14 @@ func TestListSessionsByResourceID(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/sessions/resource/pull/999", url.Values{ - "page_number": {"1"}, - "page_size": {"50"}, - }), + httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), "api.githubcopilot.com", ), - httpmock.StringResponse(`{"sessions":[]}`), + + httpmock.StatusStringResponse(404, "{}"), ) }, - wantOut: nil, + wantErr: "failed to list sessions", }, { name: "single session", @@ -1243,14 +1241,123 @@ func TestListSessionsByResourceID(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/sessions/resource/pull/999", url.Values{ - "page_number": {"1"}, - "page_size": {"50"}, - }), + httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), "api.githubcopilot.com", ), httpmock.StringResponse(heredoc.Docf(` { + "id": "resource:pull:2000", + "user_id": 1, + "resource_global_id": "PR_kwDNA-jNB9A", + "resource_type": "pull", + "resource_id": 2000, + "session_count": 1, + "last_updated_at": 1756450800, + "state": "completed", + "resource_state": "draft", + "sessions": [ + { + "id": "sess1", + "name": "Build artifacts", + "state": "completed", + "last_updated_at": 1756450800 + } + ] + }`, + 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", + LastUpdatedAt: sampleDate, + Name: "Build artifacts", + UserID: 1, + State: "completed", + ResourceType: "pull", + ResourceID: 2000, + 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, + }, + }, + }, + }, + { + name: "multiple sessions", + perPage: 1, // to enforce pagination + limit: 2, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost( + httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), + "api.githubcopilot.com", + ), + httpmock.StringResponse(heredoc.Docf(` + { + "id": "resource:pull:2000", + "user_id": 1, + "resource_global_id": "PR_kwDNA-jNB9A", + "resource_type": "pull", + "resource_id": 2000, + "session_count": 1, + "last_updated_at": 1756450800, + "state": "completed", + "resource_state": "draft", "sessions": [ { "id": "sess1", @@ -1265,6 +1372,20 @@ func TestListSessionsByResourceID(t *testing.T) { "resource_id": 2000, "created_at": "%[1]s", "premium_requests": 0.1 + }, + { + "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 } ] }`, @@ -1311,177 +1432,12 @@ func TestListSessionsByResourceID(t *testing.T) { }, 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, - }, - }, - }, - }, - { - 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/resource/pull/999", 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/resource/pull/999", 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, + ID: "sess1", + Name: "Build artifacts", + UserID: 1, + State: "completed", + ResourceType: "pull", + ResourceID: 2000, PullRequest: &api.PullRequest{ ID: "PR_node", FullDatabaseID: "2000", @@ -1504,26 +1460,20 @@ func TestListSessionsByResourceID(t *testing.T) { }, }, { - 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, + ID: "sess2", + Name: "Build artifacts", + UserID: 1, + State: "completed", + ResourceType: "pull", + ResourceID: 2000, PullRequest: &api.PullRequest{ ID: "PR_node", - FullDatabaseID: "2001", - Number: 43, + FullDatabaseID: "2000", + Number: 42, Title: "Improve docs", State: "OPEN", IsDraft: true, - URL: "https://github.com/OWNER/REPO/pull/43", + URL: "https://github.com/OWNER/REPO/pull/42", Body: "", CreatedAt: sampleDate, UpdatedAt: sampleDate, @@ -1545,10 +1495,7 @@ func TestListSessionsByResourceID(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/sessions/resource/pull/999", url.Values{ - "page_number": {"1"}, - "page_size": {"50"}, - }), + httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), "api.githubcopilot.com", ), httpmock.StatusStringResponse(500, "{}"), @@ -1561,10 +1508,7 @@ func TestListSessionsByResourceID(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/sessions/resource/pull/999", url.Values{ - "page_number": {"1"}, - "page_size": {"50"}, - }), + httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), "api.githubcopilot.com", ), httpmock.StringResponse(heredoc.Docf(` diff --git a/pkg/cmd/agent-task/view/view.go b/pkg/cmd/agent-task/view/view.go index b002c2a3d..60fa892bf 100644 --- a/pkg/cmd/agent-task/view/view.go +++ b/pkg/cmd/agent-task/view/view.go @@ -224,10 +224,6 @@ func viewRun(opts *ViewOptions) error { prURL = pr.URL } - // TODO(babakks): currently we just fetch a pre-defined number of - // matching sessions to avoid hitting the API too many times, but it's - // technically possible for a PR to be associated with lots of sessions - // (i.e. above our selected limit). sessions, err := capiClient.ListSessionsByResourceID(ctx, "pull", prID, defaultLimit) if err != nil { return fmt.Errorf("failed to list sessions for pull request: %w", err) From 546ab1cf4b3fd19c933ab3a3a94edd81b801bda0 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 19 Sep 2025 14:27:08 +0100 Subject: [PATCH 2/7] refactor(agent-task/capi): keep `resource` private Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index 6f64d5a1e..b789d1f8b 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -101,7 +101,7 @@ type SessionError struct { Message string } -type Resource struct { +type resource struct { ID string `json:"id"` UserID uint64 `json:"user_id"` ResourceType string `json:"resource_type"` @@ -295,7 +295,7 @@ func (c *CAPIClient) ListSessionsByResourceID(ctx context.Context, resourceType return nil, fmt.Errorf("failed to list sessions: %s", res.Status) } - var response Resource + var response resource if err := json.NewDecoder(res.Body).Decode(&response); err != nil { return nil, fmt.Errorf("failed to decode sessions response: %w", err) } From 9148f41cc6d0022e02d345caa5610e5d16509c13 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 19 Sep 2025 14:28:46 +0100 Subject: [PATCH 3/7] fix(agent-task/capi): handle unpopulated resource session `LastUpdatedAt` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index b789d1f8b..96f887cdf 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -302,16 +302,19 @@ func (c *CAPIClient) ListSessionsByResourceID(ctx context.Context, resourceType sessions := make([]session, 0, len(response.Sessions)) for _, s := range response.Sessions { - sessions = append(sessions, session{ + session := session{ ID: s.SessionID, Name: s.Name, UserID: int64(response.UserID), ResourceType: response.ResourceType, ResourceID: response.ResourceID, ResourceGlobalID: response.ResourceGlobalID, - LastUpdatedAt: time.Unix(s.SessionLastUpdatedAt, 0), State: s.SessionState, - }) + } + if s.SessionLastUpdatedAt != 0 { + session.LastUpdatedAt = time.Unix(s.SessionLastUpdatedAt, 0).UTC() + } + sessions = append(sessions, session) } result, err := c.hydrateSessionPullRequestsAndUsers(sessions) From a108b4ee95038ae3e82fb9ea33ac9a702c7738f7 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 19 Sep 2025 14:29:44 +0100 Subject: [PATCH 4/7] fix(agent-task/capi): fetch viewer when resource session user id is zero Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index 96f887cdf..e8fac4784 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -330,6 +330,10 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* return nil, nil } + // When the session is fetched via the resources endpoint, the session user + // ID can be zero, which means it's the viewer user. + var includeViewer bool + prNodeIds := make([]string, 0, len(sessions)) userNodeIds := make([]string, 0, len(sessions)) for _, session := range sessions { @@ -343,9 +347,13 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* } } - userNodeId := generateUserNodeID(session.UserID) - if !slices.Contains(userNodeIds, userNodeId) { - userNodeIds = append(userNodeIds, userNodeId) + if session.UserID != 0 { + userNodeId := generateUserNodeID(session.UserID) + if !slices.Contains(userNodeIds, userNodeId) { + userNodeIds = append(userNodeIds, userNodeId) + } + } else { + includeViewer = true } } apiClient := api.NewClientFromHTTP(c.httpClient) @@ -356,6 +364,7 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* PullRequest sessionPullRequest `graphql:"... on PullRequest"` User api.GitHubUser `graphql:"... on User"` } `graphql:"nodes(ids: $ids)"` + Viewer api.GitHubUser `graphql:"viewer @include(if: $includeViewer)"` } ids := make([]string, 0, len(prNodeIds)+len(userNodeIds)) @@ -365,7 +374,8 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* // TODO handle pagination host, _ := c.authCfg.DefaultHost() err := apiClient.Query(host, "FetchPRsAndUsersForAgentTaskSessions", &resp, map[string]any{ - "ids": ids, + "ids": ids, + "includeViewer": includeViewer, }) if err != nil { @@ -401,7 +411,12 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* for _, s := range sessions { newSession := fromAPISession(s) newSession.PullRequest = prMap[strconv.FormatInt(s.ResourceID, 10)] - newSession.User = userMap[s.UserID] + if s.UserID != 0 { + newSession.User = userMap[s.UserID] + } else { + newSession.UserID = resp.Viewer.DatabaseID + newSession.User = &resp.Viewer + } newSessions = append(newSessions, newSession) } From 2fe60d9105c95dd8d96c556dc613517285dd2008 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 19 Sep 2025 14:30:29 +0100 Subject: [PATCH 5/7] docs(agent-task/capi): add TODO note for dropping local ID generation Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index e8fac4784..8835dcdaa 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -339,6 +339,8 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* for _, session := range sessions { if session.ResourceType == "pull" { prNodeID := session.ResourceGlobalID + // TODO: probably this can be dropped since the API should always + // keep returning the resource global ID. if session.ResourceGlobalID == "" { prNodeID = generatePullRequestNodeID(int64(session.RepoID), session.ResourceID) } From eba31343a858e2f3c7fb898b55d745a91f076f58 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 19 Sep 2025 14:31:03 +0100 Subject: [PATCH 6/7] test(agent-tassk/capi): update `ListSessionsByResourceID` tests Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions_test.go | 149 ++++++++++++++++++----- 1 file changed, 121 insertions(+), 28 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 8c13610a2..272b32cbe 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -1203,6 +1203,7 @@ func TestListSessionsByResourceID(t *testing.T) { sampleDateString := "2025-08-29T07:00:00Z" sampleDate, err := time.Parse(time.RFC3339, sampleDateString) require.NoError(t, err) + sampleDateTimestamp := sampleDate.Unix() resourceID := int64(999) resourceType := "pull" @@ -1221,14 +1222,14 @@ func TestListSessionsByResourceID(t *testing.T) { wantOut: nil, }, { - name: "no sessions", + // If the given pull request does not exist or the pull request has no sessions, + // the API endpoint returns 404 with different messages. We should treat them + // the same though. + name: "no sessions or no pull request", limit: 10, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( - httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), - "api.githubcopilot.com", - ), + httpmock.WithHost(httpmock.REST("GET", "agents/resource/pull/999"), "api.githubcopilot.com"), httpmock.StatusStringResponse(404, "{}"), ) @@ -1240,10 +1241,7 @@ func TestListSessionsByResourceID(t *testing.T) { limit: 10, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( - httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), - "api.githubcopilot.com", - ), + httpmock.WithHost(httpmock.REST("GET", "agents/resource/pull/999"), "api.githubcopilot.com"), httpmock.StringResponse(heredoc.Docf(` { "id": "resource:pull:2000", @@ -1252,7 +1250,7 @@ func TestListSessionsByResourceID(t *testing.T) { "resource_type": "pull", "resource_id": 2000, "session_count": 1, - "last_updated_at": 1756450800, + "last_updated_at": %[1]d, "state": "completed", "resource_state": "draft", "sessions": [ @@ -1260,11 +1258,11 @@ func TestListSessionsByResourceID(t *testing.T) { "id": "sess1", "name": "Build artifacts", "state": "completed", - "last_updated_at": 1756450800 + "last_updated_at": %[1]d } ] }`, - sampleDateString, + sampleDateTimestamp, )), ) // GraphQL hydration @@ -1302,12 +1300,115 @@ func TestListSessionsByResourceID(t *testing.T) { sampleDateString, ), func(q string, vars map[string]interface{}) { assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "U_kgAB"}, vars["ids"]) + assert.Equal(t, false, vars["includeViewer"]) }), ) }, wantOut: []*Session{ { ID: "sess1", + CreatedAt: time.Time{}, + LastUpdatedAt: sampleDate, + Name: "Build artifacts", + UserID: 1, + State: "completed", + ResourceType: "pull", + ResourceID: 2000, + 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, + }, + }, + }, + }, + { + name: "single session with zero user ID", + limit: 10, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/resource/pull/999"), "api.githubcopilot.com"), + httpmock.StringResponse(heredoc.Docf(` + { + "id": "resource:pull:2000", + "user_id": 0, + "resource_global_id": "PR_kwDNA-jNB9A", + "resource_type": "pull", + "resource_id": 2000, + "session_count": 1, + "last_updated_at": %[1]d, + "state": "completed", + "resource_state": "draft", + "sessions": [ + { + "id": "sess1", + "name": "Build artifacts", + "state": "completed", + "last_updated_at": %[1]d + } + ] + }`, + sampleDateTimestamp, + )), + ) + // 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" + } + } + ], + "viewer": { + "login": "octocat", + "name": "Octocat", + "databaseId": 1 + } + } + }`, + sampleDateString, + ), func(q string, vars map[string]interface{}) { + // Should not fetch user by ID since it's zero + assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A"}, vars["ids"]) + assert.Equal(t, true, vars["includeViewer"]) + }), + ) + }, + wantOut: []*Session{ + { + ID: "sess1", + CreatedAt: time.Time{}, LastUpdatedAt: sampleDate, Name: "Build artifacts", UserID: 1, @@ -1343,10 +1444,7 @@ func TestListSessionsByResourceID(t *testing.T) { limit: 2, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( - httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), - "api.githubcopilot.com", - ), + httpmock.WithHost(httpmock.REST("GET", "agents/resource/pull/999"), "api.githubcopilot.com"), httpmock.StringResponse(heredoc.Docf(` { "id": "resource:pull:2000", @@ -1355,7 +1453,7 @@ func TestListSessionsByResourceID(t *testing.T) { "resource_type": "pull", "resource_id": 2000, "session_count": 1, - "last_updated_at": 1756450800, + "last_updated_at": %[1]d, "state": "completed", "resource_state": "draft", "sessions": [ @@ -1370,7 +1468,7 @@ func TestListSessionsByResourceID(t *testing.T) { "repo_id": 1000, "resource_type": "pull", "resource_id": 2000, - "created_at": "%[1]s", + "created_at": %[1]d, "premium_requests": 0.1 }, { @@ -1384,12 +1482,12 @@ func TestListSessionsByResourceID(t *testing.T) { "repo_id": 1000, "resource_type": "pull", "resource_id": 2001, - "created_at": "%[1]s", + "created_at": %[1]d, "premium_requests": 0.1 } ] }`, - sampleDateString, + sampleDateTimestamp, )), ) // GraphQL hydration @@ -1427,6 +1525,7 @@ func TestListSessionsByResourceID(t *testing.T) { sampleDateString, ), func(q string, vars map[string]interface{}) { assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "U_kgAB"}, vars["ids"]) + assert.Equal(t, false, vars["includeViewer"]) }), ) }, @@ -1494,10 +1593,7 @@ func TestListSessionsByResourceID(t *testing.T) { limit: 10, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( - httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), - "api.githubcopilot.com", - ), + httpmock.WithHost(httpmock.REST("GET", "agents/resource/pull/999"), "api.githubcopilot.com"), httpmock.StatusStringResponse(500, "{}"), ) }, @@ -1507,10 +1603,7 @@ func TestListSessionsByResourceID(t *testing.T) { limit: 10, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( - httpmock.WithHost( - httpmock.QueryMatcher("GET", "agents/resource/pull/999", url.Values{}), - "api.githubcopilot.com", - ), + httpmock.WithHost(httpmock.REST("GET", "agents/resource/pull/999"), "api.githubcopilot.com"), httpmock.StringResponse(heredoc.Docf(` { "sessions": [ From 597cdaf08161ec1e99c076ea2d065821475a3cf3 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 19 Sep 2025 15:37:46 +0100 Subject: [PATCH 7/7] fix(agent-task/capi): remove fallback to viewer when user id is zero Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 25 ++---- pkg/cmd/agent-task/capi/sessions_test.go | 103 ----------------------- 2 files changed, 5 insertions(+), 123 deletions(-) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index 8835dcdaa..4d109ea26 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -330,10 +330,6 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* return nil, nil } - // When the session is fetched via the resources endpoint, the session user - // ID can be zero, which means it's the viewer user. - var includeViewer bool - prNodeIds := make([]string, 0, len(sessions)) userNodeIds := make([]string, 0, len(sessions)) for _, session := range sessions { @@ -349,13 +345,9 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* } } - if session.UserID != 0 { - userNodeId := generateUserNodeID(session.UserID) - if !slices.Contains(userNodeIds, userNodeId) { - userNodeIds = append(userNodeIds, userNodeId) - } - } else { - includeViewer = true + userNodeId := generateUserNodeID(session.UserID) + if !slices.Contains(userNodeIds, userNodeId) { + userNodeIds = append(userNodeIds, userNodeId) } } apiClient := api.NewClientFromHTTP(c.httpClient) @@ -366,7 +358,6 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* PullRequest sessionPullRequest `graphql:"... on PullRequest"` User api.GitHubUser `graphql:"... on User"` } `graphql:"nodes(ids: $ids)"` - Viewer api.GitHubUser `graphql:"viewer @include(if: $includeViewer)"` } ids := make([]string, 0, len(prNodeIds)+len(userNodeIds)) @@ -376,8 +367,7 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* // TODO handle pagination host, _ := c.authCfg.DefaultHost() err := apiClient.Query(host, "FetchPRsAndUsersForAgentTaskSessions", &resp, map[string]any{ - "ids": ids, - "includeViewer": includeViewer, + "ids": ids, }) if err != nil { @@ -413,12 +403,7 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* for _, s := range sessions { newSession := fromAPISession(s) newSession.PullRequest = prMap[strconv.FormatInt(s.ResourceID, 10)] - if s.UserID != 0 { - newSession.User = userMap[s.UserID] - } else { - newSession.UserID = resp.Viewer.DatabaseID - newSession.User = &resp.Viewer - } + newSession.User = userMap[s.UserID] newSessions = append(newSessions, newSession) } diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 272b32cbe..848fd716a 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -1300,108 +1300,6 @@ func TestListSessionsByResourceID(t *testing.T) { sampleDateString, ), func(q string, vars map[string]interface{}) { assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "U_kgAB"}, vars["ids"]) - assert.Equal(t, false, vars["includeViewer"]) - }), - ) - }, - wantOut: []*Session{ - { - ID: "sess1", - CreatedAt: time.Time{}, - LastUpdatedAt: sampleDate, - Name: "Build artifacts", - UserID: 1, - State: "completed", - ResourceType: "pull", - ResourceID: 2000, - 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, - }, - }, - }, - }, - { - name: "single session with zero user ID", - limit: 10, - httpStubs: func(t *testing.T, reg *httpmock.Registry) { - reg.Register( - httpmock.WithHost(httpmock.REST("GET", "agents/resource/pull/999"), "api.githubcopilot.com"), - httpmock.StringResponse(heredoc.Docf(` - { - "id": "resource:pull:2000", - "user_id": 0, - "resource_global_id": "PR_kwDNA-jNB9A", - "resource_type": "pull", - "resource_id": 2000, - "session_count": 1, - "last_updated_at": %[1]d, - "state": "completed", - "resource_state": "draft", - "sessions": [ - { - "id": "sess1", - "name": "Build artifacts", - "state": "completed", - "last_updated_at": %[1]d - } - ] - }`, - sampleDateTimestamp, - )), - ) - // 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" - } - } - ], - "viewer": { - "login": "octocat", - "name": "Octocat", - "databaseId": 1 - } - } - }`, - sampleDateString, - ), func(q string, vars map[string]interface{}) { - // Should not fetch user by ID since it's zero - assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A"}, vars["ids"]) - assert.Equal(t, true, vars["includeViewer"]) }), ) }, @@ -1525,7 +1423,6 @@ func TestListSessionsByResourceID(t *testing.T) { sampleDateString, ), func(q string, vars map[string]interface{}) { assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "U_kgAB"}, vars["ids"]) - assert.Equal(t, false, vars["includeViewer"]) }), ) },