From 1e36e9f1e386b9b1bde7840afbd638c8d55a2a09 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 5 Sep 2025 10:55:42 +0100 Subject: [PATCH 1/8] refactor(agent-task/capi): hydrate user data Signed-off-by: Babak K. Shandiz --- api/queries_repo.go | 7 +- pkg/cmd/agent-task/capi/sessions.go | 84 ++++++++++++++++-------- pkg/cmd/agent-task/capi/sessions_test.go | 74 ++++++++++++++++++--- 3 files changed, 126 insertions(+), 39 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 324200afd..e797cead4 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -141,9 +141,10 @@ type RepositoryOwner struct { } type GitHubUser struct { - ID string `json:"id"` - Login string `json:"login"` - Name string `json:"name"` + ID string `json:"id"` + Login string `json:"login"` + Name string `json:"name"` + DatabaseID int64 `json:"databaseId"` } // Actor is a superset of User and Bot, among others. diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index 8bf29575c..b553fc932 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -22,7 +22,7 @@ var defaultSessionsPerPage = 50 type session struct { ID string `json:"id"` Name string `json:"name"` - UserID uint64 `json:"user_id"` + UserID int64 `json:"user_id"` AgentID int64 `json:"agent_id"` Logs string `json:"logs"` State string `json:"state"` @@ -60,7 +60,7 @@ type sessionPullRequest struct { type Session struct { ID string Name string - UserID uint64 + UserID int64 AgentID int64 Logs string State string @@ -75,6 +75,7 @@ type Session struct { EventType string PullRequest *api.PullRequest + User *api.GitHubUser } // ListSessionsForViewer lists all agent sessions for the @@ -127,7 +128,7 @@ func (c *CAPIClient) ListSessionsForViewer(ctx context.Context, limit int) ([]*S } // Hydrate the result with pull request data. - result, err := c.hydrateSessionPullRequests(sessions) + result, err := c.hydrateSessionPullRequestsAndUsers(sessions) if err != nil { return nil, fmt.Errorf("failed to fetch session resources: %w", err) } @@ -188,42 +189,50 @@ func (c *CAPIClient) ListSessionsForRepo(ctx context.Context, owner string, repo } // Hydrate the result with pull request data. - result, err := c.hydrateSessionPullRequests(sessions) + result, err := c.hydrateSessionPullRequestsAndUsers(sessions) if err != nil { return nil, fmt.Errorf("failed to fetch session resources: %w", err) } return result, nil } -// hydrateSessionPullRequests hydrates pull request information in sessions -func (c *CAPIClient) hydrateSessionPullRequests(sessions []session) ([]*Session, error) { +// hydrateSessionPullRequestsAndUsers hydrates pull request and user information in sessions +func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]*Session, error) { if len(sessions) == 0 { return nil, nil } prNodeIds := make([]string, 0, len(sessions)) - + userNodeIds := make([]string, 0, len(sessions)) for _, session := range sessions { prNodeID := generatePullRequestNodeID(int64(session.RepoID), session.ResourceID) - if slices.Contains(prNodeIds, prNodeID) { - continue + if !slices.Contains(prNodeIds, prNodeID) { + prNodeIds = append(prNodeIds, prNodeID) } - prNodeIds = append(prNodeIds, prNodeID) - } + userNodeId := generateUserNodeID(session.UserID) + if !slices.Contains(userNodeIds, userNodeId) { + userNodeIds = append(userNodeIds, userNodeId) + } + } apiClient := api.NewClientFromHTTP(c.httpClient) var resp struct { Nodes []struct { TypeName string `graphql:"__typename"` PullRequest sessionPullRequest `graphql:"... on PullRequest"` + User api.GitHubUser `graphql:"... on User"` } `graphql:"nodes(ids: $ids)"` } + ids := make([]string, 0, len(prNodeIds)+len(userNodeIds)) + ids = append(ids, prNodeIds...) + ids = append(ids, userNodeIds...) + // TODO handle pagination host, _ := c.authCfg.DefaultHost() - err := apiClient.Query(host, "FetchPRsForAgentTaskSessions", &resp, map[string]any{ - "ids": prNodeIds, + err := apiClient.Query(host, "FetchPRsAndUsersForAgentTaskSessions", &resp, map[string]any{ + "ids": ids, }) if err != nil { @@ -231,20 +240,26 @@ func (c *CAPIClient) hydrateSessionPullRequests(sessions []session) ([]*Session, } prMap := make(map[string]*api.PullRequest, len(prNodeIds)) + userMap := make(map[int64]*api.GitHubUser, len(userNodeIds)) for _, node := range resp.Nodes { - prMap[node.PullRequest.FullDatabaseID] = &api.PullRequest{ - ID: node.PullRequest.ID, - FullDatabaseID: node.PullRequest.FullDatabaseID, - Number: node.PullRequest.Number, - Title: node.PullRequest.Title, - State: node.PullRequest.State, - URL: node.PullRequest.URL, - Body: node.PullRequest.Body, - CreatedAt: node.PullRequest.CreatedAt, - UpdatedAt: node.PullRequest.UpdatedAt, - ClosedAt: node.PullRequest.ClosedAt, - MergedAt: node.PullRequest.MergedAt, - Repository: node.PullRequest.Repository, + switch node.TypeName { + case "User": + userMap[node.User.DatabaseID] = &node.User + case "PullRequest": + prMap[node.PullRequest.FullDatabaseID] = &api.PullRequest{ + ID: node.PullRequest.ID, + FullDatabaseID: node.PullRequest.FullDatabaseID, + Number: node.PullRequest.Number, + Title: node.PullRequest.Title, + State: node.PullRequest.State, + URL: node.PullRequest.URL, + Body: node.PullRequest.Body, + CreatedAt: node.PullRequest.CreatedAt, + UpdatedAt: node.PullRequest.UpdatedAt, + ClosedAt: node.PullRequest.ClosedAt, + MergedAt: node.PullRequest.MergedAt, + Repository: node.PullRequest.Repository, + } } } @@ -252,6 +267,7 @@ func (c *CAPIClient) hydrateSessionPullRequests(sessions []session) ([]*Session, for _, s := range sessions { newSession := fromAPISession(s) newSession.PullRequest = prMap[strconv.FormatInt(s.ResourceID, 10)] + newSession.User = userMap[s.UserID] newSessions = append(newSessions, newSession) } @@ -276,6 +292,22 @@ func generatePullRequestNodeID(repoID, pullRequestID int64) string { return "PR_" + encoded } +func generateUserNodeID(userID int64) string { + buf := bytes.Buffer{} + parts := []int64{0, userID} + + encoder := msgpack.NewEncoder(&buf) + encoder.UseCompactInts(true) + + if err := encoder.Encode(parts); err != nil { + panic(err) + } + + encoded := base64.RawURLEncoding.EncodeToString(buf.Bytes()) + + return "U_" + encoded +} + func fromAPISession(s session) *Session { return &Session{ ID: s.ID, diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 20b670d56..0ff3a46f8 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -86,7 +86,7 @@ func TestListSessionsForViewer(t *testing.T) { ) // GraphQL hydration reg.Register( - httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), httpmock.GraphQLQuery(heredoc.Docf(` { "data": { @@ -105,13 +105,19 @@ func TestListSessionsForViewer(t *testing.T) { "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"}, vars["ids"]) + assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "U_kgAB"}, vars["ids"]) }), ) }, @@ -143,6 +149,11 @@ func TestListSessionsForViewer(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, + User: &api.GitHubUser{ + Login: "octocat", + Name: "Octocat", + DatabaseID: 1, + }, }, }, }, @@ -213,7 +224,7 @@ func TestListSessionsForViewer(t *testing.T) { ) // GraphQL hydration reg.Register( - httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), httpmock.GraphQLQuery(heredoc.Docf(` { "data": { @@ -247,13 +258,19 @@ func TestListSessionsForViewer(t *testing.T) { "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"}, vars["ids"]) + assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "PR_kwDNA-jNB9E", "U_kgAB"}, vars["ids"]) }), ) }, @@ -284,6 +301,11 @@ func TestListSessionsForViewer(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, + User: &api.GitHubUser{ + Login: "octocat", + Name: "Octocat", + DatabaseID: 1, + }, }, { ID: "sess2", @@ -311,6 +333,11 @@ func TestListSessionsForViewer(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, + User: &api.GitHubUser{ + Login: "octocat", + Name: "Octocat", + DatabaseID: 1, + }, }, }, }, @@ -365,7 +392,7 @@ func TestListSessionsForViewer(t *testing.T) { ) // GraphQL hydration reg.Register( - httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), httpmock.StatusStringResponse(500, `{}`), ) }, @@ -489,7 +516,7 @@ func TestListSessionsForRepo(t *testing.T) { ) // GraphQL hydration reg.Register( - httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), httpmock.GraphQLQuery(heredoc.Docf(` { "data": { @@ -508,13 +535,19 @@ func TestListSessionsForRepo(t *testing.T) { "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"}, vars["ids"]) + assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "U_kgAB"}, vars["ids"]) }), ) }, @@ -545,6 +578,11 @@ func TestListSessionsForRepo(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, + User: &api.GitHubUser{ + Login: "octocat", + Name: "Octocat", + DatabaseID: 1, + }, }, }, }, @@ -615,7 +653,7 @@ func TestListSessionsForRepo(t *testing.T) { ) // GraphQL hydration reg.Register( - httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), httpmock.GraphQLQuery(heredoc.Docf(` { "data": { @@ -649,13 +687,19 @@ func TestListSessionsForRepo(t *testing.T) { "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"}, vars["ids"]) + assert.Equal(t, []interface{}{"PR_kwDNA-jNB9A", "PR_kwDNA-jNB9E", "U_kgAB"}, vars["ids"]) }), ) }, @@ -686,6 +730,11 @@ func TestListSessionsForRepo(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, + User: &api.GitHubUser{ + Login: "octocat", + Name: "Octocat", + DatabaseID: 1, + }, }, { ID: "sess2", @@ -713,6 +762,11 @@ func TestListSessionsForRepo(t *testing.T) { NameWithOwner: "OWNER/REPO", }, }, + User: &api.GitHubUser{ + Login: "octocat", + Name: "Octocat", + DatabaseID: 1, + }, }, }, }, @@ -767,7 +821,7 @@ func TestListSessionsForRepo(t *testing.T) { ) // GraphQL hydration reg.Register( - httpmock.GraphQL(`query FetchPRsForAgentTaskSessions\b`), + httpmock.GraphQL(`query FetchPRsAndUsersForAgentTaskSessions\b`), httpmock.StatusStringResponse(500, `{}`), ) }, From 4e1fcf1da93092dcee7e5d3ee774bd183d74a11e Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 5 Sep 2025 10:59:51 +0100 Subject: [PATCH 2/8] refactor(agent-task/capi): populate PRs `IsDraft` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 2 ++ pkg/cmd/agent-task/capi/sessions_test.go | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index b553fc932..f53028941 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -47,6 +47,7 @@ type sessionPullRequest struct { State string URL string Body string + IsDraft bool CreatedAt time.Time UpdatedAt time.Time @@ -252,6 +253,7 @@ func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]* Number: node.PullRequest.Number, Title: node.PullRequest.Title, State: node.PullRequest.State, + IsDraft: node.PullRequest.IsDraft, URL: node.PullRequest.URL, Body: node.PullRequest.Body, CreatedAt: node.PullRequest.CreatedAt, diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 0ff3a46f8..94991fb8b 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -98,6 +98,7 @@ func TestListSessionsForViewer(t *testing.T) { "number": 42, "title": "Improve docs", "state": "OPEN", + "isDraft": true, "url": "https://github.com/OWNER/REPO/pull/42", "body": "", "createdAt": "%[1]s", @@ -141,6 +142,7 @@ func TestListSessionsForViewer(t *testing.T) { Number: 42, Title: "Improve docs", State: "OPEN", + IsDraft: true, URL: "https://github.com/OWNER/REPO/pull/42", Body: "", CreatedAt: sampleDate, @@ -236,6 +238,7 @@ func TestListSessionsForViewer(t *testing.T) { "number": 42, "title": "Improve docs", "state": "OPEN", + "isDraft": true, "url": "https://github.com/OWNER/REPO/pull/42", "body": "", "createdAt": "%[1]s", @@ -251,6 +254,7 @@ func TestListSessionsForViewer(t *testing.T) { "number": 43, "title": "Improve docs", "state": "OPEN", + "isDraft": true, "url": "https://github.com/OWNER/REPO/pull/43", "body": "", "createdAt": "%[1]s", @@ -293,6 +297,7 @@ func TestListSessionsForViewer(t *testing.T) { Number: 42, Title: "Improve docs", State: "OPEN", + IsDraft: true, URL: "https://github.com/OWNER/REPO/pull/42", Body: "", CreatedAt: sampleDate, @@ -325,6 +330,7 @@ func TestListSessionsForViewer(t *testing.T) { Number: 43, Title: "Improve docs", State: "OPEN", + IsDraft: true, URL: "https://github.com/OWNER/REPO/pull/43", Body: "", CreatedAt: sampleDate, @@ -528,6 +534,7 @@ func TestListSessionsForRepo(t *testing.T) { "number": 42, "title": "Improve docs", "state": "OPEN", + "isDraft": true, "url": "https://github.com/OWNER/REPO/pull/42", "body": "", "createdAt": "%[1]s", @@ -570,6 +577,7 @@ func TestListSessionsForRepo(t *testing.T) { Number: 42, Title: "Improve docs", State: "OPEN", + IsDraft: true, URL: "https://github.com/OWNER/REPO/pull/42", Body: "", CreatedAt: sampleDate, @@ -665,6 +673,7 @@ func TestListSessionsForRepo(t *testing.T) { "number": 42, "title": "Improve docs", "state": "OPEN", + "isDraft": true, "url": "https://github.com/OWNER/REPO/pull/42", "body": "", "createdAt": "%[1]s", @@ -680,6 +689,7 @@ func TestListSessionsForRepo(t *testing.T) { "number": 43, "title": "Improve docs", "state": "OPEN", + "isDraft": true, "url": "https://github.com/OWNER/REPO/pull/43", "body": "", "createdAt": "%[1]s", @@ -722,6 +732,7 @@ func TestListSessionsForRepo(t *testing.T) { Number: 42, Title: "Improve docs", State: "OPEN", + IsDraft: true, URL: "https://github.com/OWNER/REPO/pull/42", Body: "", CreatedAt: sampleDate, @@ -754,6 +765,7 @@ func TestListSessionsForRepo(t *testing.T) { Number: 43, Title: "Improve docs", State: "OPEN", + IsDraft: true, URL: "https://github.com/OWNER/REPO/pull/43", Body: "", CreatedAt: sampleDate, From dcadeb75d49a71a3937272dfc6a64d5eb7414fdd Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 5 Sep 2025 11:17:13 +0100 Subject: [PATCH 3/8] feat(agent-task/capi): add `GetSession` method Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions.go | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index f53028941..e0252a8bb 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -5,6 +5,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -18,6 +19,8 @@ import ( var defaultSessionsPerPage = 50 +var ErrSessionNotFound = errors.New("not found") + // session is an in-flight agent task type session struct { ID string `json:"id"` @@ -197,6 +200,45 @@ func (c *CAPIClient) ListSessionsForRepo(ctx context.Context, owner string, repo return result, nil } +// GetSession retrieves a specific agent session by ID. +func (c *CAPIClient) GetSession(ctx context.Context, id string) (*Session, error) { + if id == "" { + return nil, fmt.Errorf("missing session ID") + } + + url := fmt.Sprintf("%s/agents/sessions/%s", baseCAPIURL, url.PathEscape(id)) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) + if err != nil { + return nil, err + } + + res, err := c.httpClient.Do(req) + if err != nil { + return nil, err + } + + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + if res.StatusCode == http.StatusNotFound { + return nil, ErrSessionNotFound + } + return nil, fmt.Errorf("failed to get session: %s", res.Status) + } + + var rawSession session + if err := json.NewDecoder(res.Body).Decode(&rawSession); err != nil { + return nil, fmt.Errorf("failed to decode session response: %w", err) + } + + sessions, err := c.hydrateSessionPullRequestsAndUsers([]session{rawSession}) + if err != nil { + return nil, fmt.Errorf("failed to fetch session resources: %w", err) + } + + return sessions[0], nil +} + // hydrateSessionPullRequestsAndUsers hydrates pull request and user information in sessions func (c *CAPIClient) hydrateSessionPullRequestsAndUsers(sessions []session) ([]*Session, error) { if len(sessions) == 0 { From ee8fc060776ef5c2faae5dc03aa8471f3b32352f Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 5 Sep 2025 11:18:58 +0100 Subject: [PATCH 4/8] test(agent-task/capi): add tests for `GetSession` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/sessions_test.go | 207 +++++++++++++++++++++++ 1 file changed, 207 insertions(+) diff --git a/pkg/cmd/agent-task/capi/sessions_test.go b/pkg/cmd/agent-task/capi/sessions_test.go index 94991fb8b..115c7ab9e 100644 --- a/pkg/cmd/agent-task/capi/sessions_test.go +++ b/pkg/cmd/agent-task/capi/sessions_test.go @@ -875,3 +875,210 @@ func TestListSessionsForRepo(t *testing.T) { }) } } + +func TestGetSessionRequiresID(t *testing.T) { + client := &CAPIClient{} + + _, err := client.GetSession(context.Background(), "") + assert.EqualError(t, err, "missing session ID") +} + +func TestGetSession(t *testing.T) { + sampleDateString := "2025-08-29T00:00:00Z" + sampleDate, err := time.Parse(time.RFC3339, sampleDateString) + require.NoError(t, err) + + tests := []struct { + name string + httpStubs func(*testing.T, *httpmock.Registry) + wantErr string + wantErrIs error + wantOut *Session + }{ + { + name: "session not found", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/sessions/some-uuid"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(404, "{}"), + ) + }, + wantErrIs: ErrSessionNotFound, + wantErr: "not found", + }, + { + name: "API error", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/sessions/some-uuid"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(500, "some error"), + ) + }, + wantErr: "failed to get session:", + }, + { + name: "invalid JSON response", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/sessions/some-uuid"), "api.githubcopilot.com"), + httpmock.StatusStringResponse(200, ""), + ) + }, + wantErr: "failed to decode session response: EOF", + }, + { + name: "success", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/sessions/some-uuid"), "api.githubcopilot.com"), + httpmock.StringResponse(heredoc.Docf(` + { + "id": "some-uuid", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2000, + "created_at": "%[1]s" + }`, + sampleDateString, + )), + ) + // GraphQL hydration + reg.Register( + httpmock.GraphQL(`query 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: "some-uuid", + Name: "Build artifacts", + UserID: 1, + AgentID: 2, + Logs: "", + State: "completed", + OwnerID: 10, + RepoID: 1000, + ResourceType: "pull", + ResourceID: 2000, + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + ID: "PR_node", + FullDatabaseID: "2000", + Number: 42, + Title: "Improve docs", + State: "OPEN", + 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: "API error at hydration", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.WithHost(httpmock.REST("GET", "agents/sessions/some-uuid"), "api.githubcopilot.com"), + httpmock.StringResponse(heredoc.Docf(` + { + "id": "some-uuid", + "name": "Build artifacts", + "user_id": 1, + "agent_id": 2, + "logs": "", + "state": "completed", + "owner_id": 10, + "repo_id": 1000, + "resource_type": "pull", + "resource_id": 2000, + "created_at": "%[1]s" + }`, + sampleDateString, + )), + ) + // GraphQL hydration + reg.Register( + httpmock.GraphQL(`query 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()) + + session, err := capiClient.GetSession(context.Background(), "some-uuid") + + if tt.wantErrIs != nil { + require.ErrorIs(t, err, tt.wantErrIs) + } + + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + require.Nil(t, session) + return + } + + require.NoError(t, err) + require.Equal(t, tt.wantOut, session) + }) + } +} From 87b772dc83fa5b133d1374ac6dc9abd253b2c11a Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 5 Sep 2025 11:20:06 +0100 Subject: [PATCH 5/8] feat(agent-task/capi): add `GetSession` method to `CapiClient` interface Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/capi/client.go | 1 + pkg/cmd/agent-task/capi/client_mock.go | 50 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/pkg/cmd/agent-task/capi/client.go b/pkg/cmd/agent-task/capi/client.go index ecec9a024..15765552b 100644 --- a/pkg/cmd/agent-task/capi/client.go +++ b/pkg/cmd/agent-task/capi/client.go @@ -19,6 +19,7 @@ type CapiClient interface { ListSessionsForRepo(ctx context.Context, owner string, repo string, 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) } // CAPIClient is a client for interacting with the Copilot API diff --git a/pkg/cmd/agent-task/capi/client_mock.go b/pkg/cmd/agent-task/capi/client_mock.go index 621585587..ba7c05ab0 100644 --- a/pkg/cmd/agent-task/capi/client_mock.go +++ b/pkg/cmd/agent-task/capi/client_mock.go @@ -24,6 +24,9 @@ var _ CapiClient = &CapiClientMock{} // GetJobFunc: func(ctx context.Context, owner string, repo string, jobID string) (*Job, error) { // panic("mock out the GetJob method") // }, +// GetSessionFunc: func(ctx context.Context, id string) (*Session, error) { +// panic("mock out the GetSession method") +// }, // ListSessionsForRepoFunc: func(ctx context.Context, owner string, repo string, limit int) ([]*Session, error) { // panic("mock out the ListSessionsForRepo method") // }, @@ -43,6 +46,9 @@ type CapiClientMock struct { // GetJobFunc mocks the GetJob method. GetJobFunc func(ctx context.Context, owner string, repo string, jobID string) (*Job, error) + // GetSessionFunc mocks the GetSession method. + GetSessionFunc func(ctx context.Context, id string) (*Session, error) + // ListSessionsForRepoFunc mocks the ListSessionsForRepo method. ListSessionsForRepoFunc func(ctx context.Context, owner string, repo string, limit int) ([]*Session, error) @@ -75,6 +81,13 @@ type CapiClientMock struct { // JobID is the jobID argument value. JobID string } + // GetSession holds details about calls to the GetSession method. + GetSession []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // ID is the id argument value. + ID string + } // ListSessionsForRepo holds details about calls to the ListSessionsForRepo method. ListSessionsForRepo []struct { // Ctx is the ctx argument value. @@ -96,6 +109,7 @@ type CapiClientMock struct { } lockCreateJob sync.RWMutex lockGetJob sync.RWMutex + lockGetSession sync.RWMutex lockListSessionsForRepo sync.RWMutex lockListSessionsForViewer sync.RWMutex } @@ -192,6 +206,42 @@ func (mock *CapiClientMock) GetJobCalls() []struct { return calls } +// GetSession calls GetSessionFunc. +func (mock *CapiClientMock) GetSession(ctx context.Context, id string) (*Session, error) { + if mock.GetSessionFunc == nil { + panic("CapiClientMock.GetSessionFunc: method is nil but CapiClient.GetSession was just called") + } + callInfo := struct { + Ctx context.Context + ID string + }{ + Ctx: ctx, + ID: id, + } + mock.lockGetSession.Lock() + mock.calls.GetSession = append(mock.calls.GetSession, callInfo) + mock.lockGetSession.Unlock() + return mock.GetSessionFunc(ctx, id) +} + +// GetSessionCalls gets all the calls that were made to GetSession. +// Check the length with: +// +// len(mockedCapiClient.GetSessionCalls()) +func (mock *CapiClientMock) GetSessionCalls() []struct { + Ctx context.Context + ID string +} { + var calls []struct { + Ctx context.Context + ID string + } + mock.lockGetSession.RLock() + calls = mock.calls.GetSession + mock.lockGetSession.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 { From 476b636810563b68a95e1a3aabddce67aacdc8e9 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 5 Sep 2025 11:30:33 +0100 Subject: [PATCH 6/8] test(agent-task list): remove unused `stubs` Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/list/list_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index ce0fd323f..d096ab3be 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -14,7 +14,6 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" "github.com/cli/cli/v2/pkg/cmdutil" - "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" @@ -114,7 +113,6 @@ func Test_listRun(t *testing.T) { tests := []struct { name string tty bool - stubs func(*httpmock.Registry) capiStubs func(*testing.T, *capi.CapiClientMock) baseRepo ghrepo.Interface baseRepoErr error From 92c7a56b828654720b9089ef3b02d364aa56cdaf Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 5 Sep 2025 12:45:51 +0100 Subject: [PATCH 7/8] feat(agent-task view): add `view` command Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/agent_task.go | 2 + pkg/cmd/agent-task/shared/display.go | 23 ++++++ pkg/cmd/agent-task/view/view.go | 108 +++++++++++++++++++++++++++ 3 files changed, 133 insertions(+) create mode 100644 pkg/cmd/agent-task/view/view.go diff --git a/pkg/cmd/agent-task/agent_task.go b/pkg/cmd/agent-task/agent_task.go index e732e31de..4eeed3719 100644 --- a/pkg/cmd/agent-task/agent_task.go +++ b/pkg/cmd/agent-task/agent_task.go @@ -7,6 +7,7 @@ import ( cmdCreate "github.com/cli/cli/v2/pkg/cmd/agent-task/create" cmdList "github.com/cli/cli/v2/pkg/cmd/agent-task/list" + cmdView "github.com/cli/cli/v2/pkg/cmd/agent-task/view" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/go-gh/v2/pkg/auth" "github.com/spf13/cobra" @@ -31,6 +32,7 @@ func NewCmdAgentTask(f *cmdutil.Factory) *cobra.Command { // register subcommands cmd.AddCommand(cmdList.NewCmdList(f, nil)) cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) + cmd.AddCommand(cmdView.NewCmdView(f, nil)) return cmd } diff --git a/pkg/cmd/agent-task/shared/display.go b/pkg/cmd/agent-task/shared/display.go index 675143140..dd114b049 100644 --- a/pkg/cmd/agent-task/shared/display.go +++ b/pkg/cmd/agent-task/shared/display.go @@ -23,3 +23,26 @@ func ColorFuncForSessionState(s capi.Session, cs *iostreams.ColorScheme) func(st return stateColor } + +func SessionStateString(state string) string { + switch state { + case "queued": + return "Queued" + case "in_progress": + return "In Progress" + case "completed": + return "Completed" + case "failed": + return "Failed" + case "idle": + return "Idle" + case "waiting_for_user": + return "Waiting for User" + case "timed_out": + return "Timed Out" + case "cancelled": + return "Cancelled" + default: + return state + } +} diff --git a/pkg/cmd/agent-task/view/view.go b/pkg/cmd/agent-task/view/view.go new file mode 100644 index 000000000..f6ce4d468 --- /dev/null +++ b/pkg/cmd/agent-task/view/view.go @@ -0,0 +1,108 @@ +package view + +import ( + "context" + "errors" + "fmt" + "net/url" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/text" + "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" + "github.com/cli/cli/v2/pkg/cmd/agent-task/shared" + prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +type ViewOptions struct { + IO *iostreams.IOStreams + CapiClient func() (capi.CapiClient, error) + + SelectorArg string +} + +func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { + opts := &ViewOptions{ + IO: f.IOStreams, + CapiClient: shared.CapiClientFunc(f), + } + + cmd := &cobra.Command{ + Use: "view ", + Short: "View an agent task session", + Long: heredoc.Doc(` + View an agent task session. + `), + Args: cmdutil.ExactArgs(1, "a session ID is required"), + RunE: func(cmd *cobra.Command, args []string) error { + opts.SelectorArg = args[0] + + if runF != nil { + return runF(opts) + } + return viewRun(opts) + }, + } + + return cmd +} + +func viewRun(opts *ViewOptions) error { + capiClient, err := opts.CapiClient() + if err != nil { + return err + } + + ctx := context.Background() + + opts.IO.StartProgressIndicatorWithLabel("Fetching agent session...") + defer opts.IO.StopProgressIndicator() + + session, err := capiClient.GetSession(ctx, opts.SelectorArg) + opts.IO.StopProgressIndicator() + + if err != nil { + if errors.Is(err, capi.ErrSessionNotFound) { + fmt.Fprintln(opts.IO.ErrOut, "session not found") + return cmdutil.SilentError + } + return err + } + + out := opts.IO.Out + cs := opts.IO.ColorScheme() + + if session.PullRequest != nil { + fmt.Fprintf(out, "%s • %s • %s%s\n", + shared.ColorFuncForSessionState(*session, cs)(shared.SessionStateString(session.State)), + cs.Bold(session.PullRequest.Title), + session.PullRequest.Repository.NameWithOwner, + cs.ColorFromString(prShared.ColorForPRState(*session.PullRequest))(fmt.Sprintf("#%d", session.PullRequest.Number)), + ) + } else { + // Should never happen, but we need to cover the path + fmt.Fprintf(out, "%s\n", shared.ColorFuncForSessionState(*session, cs)(shared.SessionStateString(session.State))) + } + + if session.User != nil { + fmt.Fprintf(out, "Started on behalf of %s %s\n", session.User.Login, text.FuzzyAgo(time.Now(), session.CreatedAt)) + } else { + // Should never happen, but we need to cover the path + fmt.Fprintf(out, "Started %s\n", text.FuzzyAgo(time.Now(), session.CreatedAt)) + } + + // TODO(babakks): uncomment when we have the --logs option ready + // fmt.Fprintln(out, "") + // fmt.Fprintf(out, "For the detailed session logs, try: gh agent-task view '%s' --logs\n", opts.SelectorArg) + + if session.PullRequest != nil { + fmt.Fprintln(out, "") + fmt.Fprintln(out, cs.Muted("View this session on GitHub:")) + fmt.Fprintln(out, cs.Muted(fmt.Sprintf("%s/agent-sessions/%s", session.PullRequest.URL, url.PathEscape(session.ID)))) + } + + return nil +} From d3fa0a70bcaad7f700db7d2bc63ca01a76b8f56b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Fri, 5 Sep 2025 12:55:15 +0100 Subject: [PATCH 8/8] test(agent-task view): add tests for the `view` command Signed-off-by: Babak K. Shandiz --- pkg/cmd/agent-task/view/view_test.go | 261 +++++++++++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 pkg/cmd/agent-task/view/view_test.go diff --git a/pkg/cmd/agent-task/view/view_test.go b/pkg/cmd/agent-task/view/view_test.go new file mode 100644 index 000000000..97304c399 --- /dev/null +++ b/pkg/cmd/agent-task/view/view_test.go @@ -0,0 +1,261 @@ +package view + +import ( + "bytes" + "context" + "errors" + "io" + "testing" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/pkg/cmd/agent-task/capi" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewCmdList(t *testing.T) { + tests := []struct { + name string + args string + wantOpts ViewOptions + wantErr string + }{ + { + name: "no arguments", + wantErr: "a session ID is required", + }, + { + name: "session ID arg", + args: "some-uuid", + wantOpts: ViewOptions{ + SelectorArg: "some-uuid", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: ios, + } + + var gotOpts *ViewOptions + cmd := NewCmdView(f, func(opts *ViewOptions) error { gotOpts = opts; return nil }) + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.wantOpts.SelectorArg, gotOpts.SelectorArg) + }) + } +} + +func Test_viewRun(t *testing.T) { + sampleDate := time.Now().Add(-6 * time.Hour) // 6h ago + + tests := []struct { + name string + selectorArg string + tty bool + capiStubs func(*testing.T, *capi.CapiClientMock) + wantOut string + wantErr error + wantStderr string + }{ + { + name: "not found (tty)", + tty: true, + selectorArg: "some-session-id", + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(ctx context.Context, selector string) (*capi.Session, error) { + return nil, capi.ErrSessionNotFound + } + }, + wantStderr: "session not found\n", + wantErr: cmdutil.SilentError, + }, + { + name: "not found (nontty)", + selectorArg: "some-session-id", + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(ctx context.Context, selector string) (*capi.Session, error) { + return nil, capi.ErrSessionNotFound + } + }, + wantStderr: "session not found\n", + wantErr: cmdutil.SilentError, + }, + { + name: "API error (tty)", + tty: true, + selectorArg: "some-session-id", + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(ctx context.Context, selector string) (*capi.Session, error) { + return nil, errors.New("some error") + } + }, + wantErr: errors.New("some error"), + }, + { + name: "API error (nontty)", + selectorArg: "some-session-id", + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(ctx context.Context, selector string) (*capi.Session, error) { + return nil, errors.New("some error") + } + }, + wantErr: errors.New("some error"), + }, + { + name: "success, with PR and user data (tty)", + tty: true, + selectorArg: "some-session-id", + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(ctx context.Context, selector string) (*capi.Session, error) { + return &capi.Session{ + ID: "some-session-id", + State: "completed", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + User: &api.GitHubUser{ + Login: "octocat", + }, + }, nil + } + }, + wantOut: heredoc.Doc(` + Completed • fix something • OWNER/REPO#101 + Started on behalf of octocat about 6 hours ago + + View this session on GitHub: + https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id + `), + }, + { + name: "success, without user data (tty)", + tty: true, + selectorArg: "some-session-id", + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(ctx context.Context, selector string) (*capi.Session, error) { + return &capi.Session{ + ID: "some-session-id", + State: "completed", + CreatedAt: sampleDate, + PullRequest: &api.PullRequest{ + Title: "fix something", + Number: 101, + URL: "https://github.com/OWNER/REPO/pull/101", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, nil + } + }, + wantOut: heredoc.Doc(` + Completed • fix something • OWNER/REPO#101 + Started about 6 hours ago + + View this session on GitHub: + https://github.com/OWNER/REPO/pull/101/agent-sessions/some-session-id + `), + }, + { + name: "success, without PR data (tty)", + tty: true, + selectorArg: "some-session-id", + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(ctx context.Context, selector string) (*capi.Session, error) { + return &capi.Session{ + ID: "some-session-id", + State: "completed", + CreatedAt: sampleDate, + User: &api.GitHubUser{ + Login: "octocat", + }, + }, nil + } + }, + wantOut: heredoc.Doc(` + Completed + Started on behalf of octocat about 6 hours ago + `), + }, + { + name: "success, without PR nor user data (tty)", + tty: true, + selectorArg: "some-session-id", + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(ctx context.Context, selector string) (*capi.Session, error) { + return &capi.Session{ + ID: "some-session-id", + State: "completed", + CreatedAt: sampleDate, + }, nil + } + }, + wantOut: heredoc.Doc(` + Completed + Started about 6 hours ago + `), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + capiClientMock := &capi.CapiClientMock{} + if tt.capiStubs != nil { + tt.capiStubs(t, capiClientMock) + } + + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.tty) + + opts := &ViewOptions{ + IO: ios, + CapiClient: func() (capi.CapiClient, error) { + return capiClientMock, nil + }, + SelectorArg: tt.selectorArg, + } + + err := viewRun(opts) + if tt.wantErr != nil { + assert.Error(t, err) + require.EqualError(t, err, tt.wantErr.Error()) + } else { + require.NoError(t, err) + } + + got := stdout.String() + require.Equal(t, tt.wantOut, got) + require.Equal(t, tt.wantStderr, stderr.String()) + }) + } +}