Skip to content

Commit ff15f68

Browse files
tommaso-morotommaso-moro
authored
Use minimal types for tool responses (#3055)
Return compact response shapes for pull request statuses, review comment replies, and individual workflow runs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6786153-698a-4563-97ad-a8221c40e306
1 parent eff4c3c commit ff15f68

5 files changed

Lines changed: 198 additions & 46 deletions

File tree

‎pkg/github/actions.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,7 @@ func getWorkflowRun(ctx context.Context, client *github.Client, owner, repo stri
802802
return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get workflow run", resp, err), nil, nil
803803
}
804804
defer func() { _ = resp.Body.Close() }()
805-
r, err := json.Marshal(workflowRun)
805+
r, err := json.Marshal(convertToMinimalWorkflowRun(workflowRun))
806806
if err != nil {
807807
return nil, nil, fmt.Errorf("failed to marshal workflow run: %w", err)
808808
}

‎pkg/github/actions_test.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -307,14 +307,9 @@ func Test_ActionsGet_GetWorkflowRun(t *testing.T) {
307307
toolDef := ActionsGet(translations.NullTranslationHelper)
308308

309309
t.Run("successful workflow run get", func(t *testing.T) {
310+
run := actionsTestWorkflowRun()
310311
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
311312
GetReposActionsRunsByOwnerByRepoByRunID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
312-
run := &github.WorkflowRun{
313-
ID: github.Ptr(int64(12345)),
314-
Name: github.Ptr("CI"),
315-
Status: github.Ptr("completed"),
316-
Conclusion: github.Ptr("success"),
317-
}
318313
w.WriteHeader(http.StatusOK)
319314
_ = json.NewEncoder(w).Encode(run)
320315
}),
@@ -338,11 +333,21 @@ func Test_ActionsGet_GetWorkflowRun(t *testing.T) {
338333
require.False(t, result.IsError)
339334

340335
textContent := getTextResult(t, result)
341-
var response github.WorkflowRun
336+
var response MinimalWorkflowRun
342337
err = json.Unmarshal([]byte(textContent.Text), &response)
343338
require.NoError(t, err)
344-
assert.NotNil(t, response.ID)
345-
assert.Equal(t, int64(12345), *response.ID)
339+
340+
expected := convertToMinimalWorkflowRun(run)
341+
assert.Equal(t, expected, response)
342+
343+
var payload map[string]any
344+
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &payload))
345+
assert.Equal(t, marshalActionsObject(t, expected), payload)
346+
assert.NotContains(t, payload, "node_id")
347+
assert.NotContains(t, payload, "repository")
348+
assert.NotContains(t, payload, "head_repository")
349+
assert.NotContains(t, payload, "url")
350+
assert.NotContains(t, payload, "jobs_url")
346351
})
347352
}
348353

‎pkg/github/minimal_types.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,24 @@ type MinimalPRBranchRepo struct {
699699
Description string `json:"description,omitempty"`
700700
}
701701

702+
// MinimalRepoStatus is the trimmed output type for an individual commit status.
703+
type MinimalRepoStatus struct {
704+
State string `json:"state"`
705+
Context string `json:"context"`
706+
Description string `json:"description,omitempty"`
707+
TargetURL string `json:"target_url,omitempty"`
708+
CreatedAt string `json:"created_at,omitempty"`
709+
UpdatedAt string `json:"updated_at,omitempty"`
710+
}
711+
712+
// MinimalCombinedStatus is the trimmed output type for a combined commit status.
713+
type MinimalCombinedStatus struct {
714+
State string `json:"state"`
715+
SHA string `json:"sha"`
716+
TotalCount int `json:"total_count"`
717+
Statuses []MinimalRepoStatus `json:"statuses"`
718+
}
719+
702720
type MinimalProjectStatusUpdate struct {
703721
ID string `json:"id"`
704722
Body string `json:"body,omitempty"`
@@ -1057,6 +1075,42 @@ func convertToMinimalPRBranch(branch *github.PullRequestBranch) *MinimalPRBranch
10571075
return b
10581076
}
10591077

1078+
func convertToMinimalCombinedStatus(status *github.CombinedStatus) MinimalCombinedStatus {
1079+
minimalStatus := MinimalCombinedStatus{
1080+
Statuses: make([]MinimalRepoStatus, 0),
1081+
}
1082+
if status == nil {
1083+
return minimalStatus
1084+
}
1085+
1086+
minimalStatus.State = status.GetState()
1087+
minimalStatus.SHA = status.GetSHA()
1088+
minimalStatus.TotalCount = status.GetTotalCount()
1089+
minimalStatus.Statuses = make([]MinimalRepoStatus, 0, len(status.GetStatuses()))
1090+
for _, repoStatus := range status.GetStatuses() {
1091+
if repoStatus != nil {
1092+
minimalStatus.Statuses = append(minimalStatus.Statuses, convertToMinimalRepoStatus(repoStatus))
1093+
}
1094+
}
1095+
1096+
return minimalStatus
1097+
}
1098+
1099+
func convertToMinimalRepoStatus(status *github.RepoStatus) MinimalRepoStatus {
1100+
if status == nil {
1101+
return MinimalRepoStatus{}
1102+
}
1103+
1104+
return MinimalRepoStatus{
1105+
State: status.GetState(),
1106+
Context: status.GetContext(),
1107+
Description: status.GetDescription(),
1108+
TargetURL: status.GetTargetURL(),
1109+
CreatedAt: formatMinimalTimestamp(status.CreatedAt),
1110+
UpdatedAt: formatMinimalTimestamp(status.UpdatedAt),
1111+
}
1112+
}
1113+
10601114
func convertToMinimalProject(fullProject *github.ProjectV2) *MinimalProject {
10611115
if fullProject == nil {
10621116
return nil

‎pkg/github/pullrequests.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ func GetPullRequestStatus(ctx context.Context, client *github.Client, owner, rep
307307
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get combined status", resp, body), nil
308308
}
309309

310-
r, err := json.Marshal(status)
310+
r, err := json.Marshal(convertToMinimalCombinedStatus(status))
311311
if err != nil {
312312
return nil, fmt.Errorf("failed to marshal response: %w", err)
313313
}
@@ -1281,10 +1281,9 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor
12811281
}
12821282
}
12831283

1284-
var comment *github.PullRequestComment
1284+
var commentResponse *MinimalResponse
12851285
if hasBody {
1286-
var resp *github.Response
1287-
comment, resp, err = client.PullRequests.CreateCommentInReplyTo(ctx, owner, repo, pullNumber, body, commentID)
1286+
comment, resp, err := client.PullRequests.CreateCommentInReplyTo(ctx, owner, repo, pullNumber, body, commentID)
12881287
if err != nil {
12891288
return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reply to pull request comment", resp, err), nil, nil
12901289
}
@@ -1297,19 +1296,24 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor
12971296
}
12981297
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add reply to pull request comment", resp, bodyBytes), nil, nil
12991298
}
1299+
1300+
commentResponse = &MinimalResponse{
1301+
ID: fmt.Sprintf("%d", comment.GetID()),
1302+
URL: comment.GetHTMLURL(),
1303+
}
13001304
}
13011305

13021306
var result any
13031307
switch {
13041308
case hasBody && hasReaction:
1305-
result = map[string]any{
1306-
"comment": comment,
1307-
"reaction": reactionResponse,
1309+
result = map[string]MinimalResponse{
1310+
"comment": *commentResponse,
1311+
"reaction": *reactionResponse,
13081312
}
13091313
case hasReaction:
13101314
result = reactionResponse
13111315
default:
1312-
result = comment
1316+
result = commentResponse
13131317
}
13141318

13151319
r, err := json.Marshal(result)

‎pkg/github/pullrequests_test.go‎

Lines changed: 117 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,42 +1575,58 @@ func Test_GetPullRequestStatus(t *testing.T) {
15751575
},
15761576
}
15771577

1578-
// Setup mock status for success case
1578+
statusCreatedAt := &github.Timestamp{Time: time.Date(2026, time.August, 11, 9, 30, 0, 0, time.UTC)}
1579+
statusUpdatedAt := &github.Timestamp{Time: time.Date(2026, time.August, 11, 9, 35, 0, 0, time.UTC)}
15791580
mockStatus := &github.CombinedStatus{
1581+
Name: github.Ptr("abcd1234"),
15801582
State: github.Ptr("success"),
1581-
TotalCount: github.Ptr(3),
1583+
SHA: github.Ptr("abcd1234"),
1584+
TotalCount: github.Ptr(2),
1585+
CommitURL: github.Ptr("https://api.github.com/repos/owner/repo/commits/abcd1234"),
1586+
RepositoryURL: github.Ptr(
1587+
"https://api.github.com/repos/owner/repo",
1588+
),
15821589
Statuses: []*github.RepoStatus{
15831590
{
1591+
ID: github.Ptr(int64(101)),
1592+
NodeID: github.Ptr("SC_kwDOStatus101"),
1593+
URL: github.Ptr("https://api.github.com/repos/owner/repo/statuses/abcd1234"),
15841594
State: github.Ptr("success"),
15851595
Context: github.Ptr("continuous-integration/travis-ci"),
15861596
Description: github.Ptr("Build succeeded"),
15871597
TargetURL: github.Ptr("https://travis-ci.org/owner/repo/builds/123"),
1598+
AvatarURL: github.Ptr("https://avatars.githubusercontent.com/in/123"),
1599+
Creator: &github.User{
1600+
Login: github.Ptr("ci-bot"),
1601+
},
1602+
CreatedAt: statusCreatedAt,
1603+
UpdatedAt: statusUpdatedAt,
15881604
},
15891605
{
15901606
State: github.Ptr("success"),
15911607
Context: github.Ptr("codecov/patch"),
15921608
Description: github.Ptr("Coverage increased"),
15931609
TargetURL: github.Ptr("https://codecov.io/gh/owner/repo/pull/42"),
15941610
},
1595-
{
1596-
State: github.Ptr("success"),
1597-
Context: github.Ptr("lint/golangci-lint"),
1598-
Description: github.Ptr("No issues found"),
1599-
TargetURL: github.Ptr("https://golangci.com/r/owner/repo/pull/42"),
1600-
},
16011611
},
16021612
}
1613+
emptyStatus := &github.CombinedStatus{
1614+
State: github.Ptr("pending"),
1615+
SHA: github.Ptr("abcd1234"),
1616+
TotalCount: github.Ptr(0),
1617+
Statuses: []*github.RepoStatus{nil},
1618+
}
16031619

16041620
tests := []struct {
16051621
name string
16061622
mockedClient *http.Client
16071623
requestArgs map[string]any
16081624
expectError bool
1609-
expectedStatus *github.CombinedStatus
1625+
expectedStatus *MinimalCombinedStatus
16101626
expectedErrMsg string
16111627
}{
16121628
{
1613-
name: "successful status fetch",
1629+
name: "successful status fetch with multiple statuses",
16141630
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
16151631
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
16161632
GetReposCommitsStatusByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockStatus),
@@ -1621,8 +1637,46 @@ func Test_GetPullRequestStatus(t *testing.T) {
16211637
"repo": "repo",
16221638
"pullNumber": float64(42),
16231639
},
1624-
expectError: false,
1625-
expectedStatus: mockStatus,
1640+
expectedStatus: &MinimalCombinedStatus{
1641+
State: "success",
1642+
SHA: "abcd1234",
1643+
TotalCount: 2,
1644+
Statuses: []MinimalRepoStatus{
1645+
{
1646+
State: "success",
1647+
Context: "continuous-integration/travis-ci",
1648+
Description: "Build succeeded",
1649+
TargetURL: "https://travis-ci.org/owner/repo/builds/123",
1650+
CreatedAt: "2026-08-11T09:30:00Z",
1651+
UpdatedAt: "2026-08-11T09:35:00Z",
1652+
},
1653+
{
1654+
State: "success",
1655+
Context: "codecov/patch",
1656+
Description: "Coverage increased",
1657+
TargetURL: "https://codecov.io/gh/owner/repo/pull/42",
1658+
},
1659+
},
1660+
},
1661+
},
1662+
{
1663+
name: "successful status fetch with no statuses",
1664+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1665+
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
1666+
GetReposCommitsStatusByOwnerByRepoByRef: mockResponse(t, http.StatusOK, emptyStatus),
1667+
}),
1668+
requestArgs: map[string]any{
1669+
"method": "get_status",
1670+
"owner": "owner",
1671+
"repo": "repo",
1672+
"pullNumber": float64(42),
1673+
},
1674+
expectedStatus: &MinimalCombinedStatus{
1675+
State: "pending",
1676+
SHA: "abcd1234",
1677+
TotalCount: 0,
1678+
Statuses: []MinimalRepoStatus{},
1679+
},
16261680
},
16271681
{
16281682
name: "PR fetch fails",
@@ -1691,20 +1745,33 @@ func Test_GetPullRequestStatus(t *testing.T) {
16911745
require.NoError(t, err)
16921746
require.False(t, result.IsError)
16931747

1694-
// Parse the result and get the text content if no error
16951748
textContent := getTextResult(t, result)
16961749

1697-
// Unmarshal and verify the result
1698-
var returnedStatus github.CombinedStatus
1750+
var returnedStatus MinimalCombinedStatus
16991751
err = json.Unmarshal([]byte(textContent.Text), &returnedStatus)
17001752
require.NoError(t, err)
1701-
assert.Equal(t, *tc.expectedStatus.State, *returnedStatus.State)
1702-
assert.Equal(t, *tc.expectedStatus.TotalCount, *returnedStatus.TotalCount)
1703-
assert.Len(t, returnedStatus.Statuses, len(tc.expectedStatus.Statuses))
1704-
for i, status := range returnedStatus.Statuses {
1705-
assert.Equal(t, *tc.expectedStatus.Statuses[i].State, *status.State)
1706-
assert.Equal(t, *tc.expectedStatus.Statuses[i].Context, *status.Context)
1707-
assert.Equal(t, *tc.expectedStatus.Statuses[i].Description, *status.Description)
1753+
assert.Equal(t, *tc.expectedStatus, returnedStatus)
1754+
1755+
expectedJSON, err := json.Marshal(tc.expectedStatus)
1756+
require.NoError(t, err)
1757+
assert.JSONEq(t, string(expectedJSON), textContent.Text)
1758+
1759+
var payload map[string]any
1760+
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &payload))
1761+
assert.NotContains(t, payload, "name")
1762+
assert.NotContains(t, payload, "commit_url")
1763+
assert.NotContains(t, payload, "repository_url")
1764+
1765+
statuses, ok := payload["statuses"].([]any)
1766+
require.True(t, ok)
1767+
for _, status := range statuses {
1768+
statusPayload, ok := status.(map[string]any)
1769+
require.True(t, ok)
1770+
assert.NotContains(t, statusPayload, "id")
1771+
assert.NotContains(t, statusPayload, "node_id")
1772+
assert.NotContains(t, statusPayload, "url")
1773+
assert.NotContains(t, statusPayload, "avatar_url")
1774+
assert.NotContains(t, statusPayload, "creator")
17081775
}
17091776
})
17101777
}
@@ -4145,6 +4212,13 @@ func TestAddReplyToPullRequestComment(t *testing.T) {
41454212
}
41464213
replyCreatedAfterReactionFailure := &atomic.Bool{}
41474214

4215+
assertMinimalResponse := func(t *testing.T, response map[string]any, expectedID, expectedURL string) {
4216+
t.Helper()
4217+
assert.Len(t, response, 2)
4218+
assert.Equal(t, expectedID, response["id"])
4219+
assert.Equal(t, expectedURL, response["url"])
4220+
}
4221+
41484222
tests := []struct {
41494223
name string
41504224
mockedClient *http.Client
@@ -4354,14 +4428,29 @@ func TestAddReplyToPullRequestComment(t *testing.T) {
43544428
return
43554429
}
43564430

4357-
// Parse the result and verify it's not an error
43584431
require.False(t, result.IsError)
43594432
textContent := getTextResult(t, result)
4360-
if _, ok := tc.requestArgs["body"]; ok {
4361-
assert.Contains(t, textContent.Text, "This is a reply to the comment")
4362-
}
4363-
if _, ok := tc.requestArgs["reaction"]; ok {
4364-
assert.Contains(t, textContent.Text, "789")
4433+
4434+
var response map[string]any
4435+
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response))
4436+
4437+
_, hasBody := tc.requestArgs["body"]
4438+
_, hasReaction := tc.requestArgs["reaction"]
4439+
reactionURL := client.BaseURL() + "repos/owner/repo/pulls/comments/123/reactions/789"
4440+
4441+
switch {
4442+
case hasBody && hasReaction:
4443+
assert.Len(t, response, 2)
4444+
commentResponse, ok := response["comment"].(map[string]any)
4445+
require.True(t, ok)
4446+
assertMinimalResponse(t, commentResponse, "456", "https://github.com/owner/repo/pull/42#discussion_r456")
4447+
reactionResponse, ok := response["reaction"].(map[string]any)
4448+
require.True(t, ok)
4449+
assertMinimalResponse(t, reactionResponse, "789", reactionURL)
4450+
case hasBody:
4451+
assertMinimalResponse(t, response, "456", "https://github.com/owner/repo/pull/42#discussion_r456")
4452+
default:
4453+
assertMinimalResponse(t, response, "789", reactionURL)
43654454
}
43664455
})
43674456
}

0 commit comments

Comments
 (0)