Skip to content

Commit d2ca606

Browse files
MayorFajMayorFaj
authored
Merge branch 'main' into feat/git-blame-tool
2 parents 72fbace + 5259513 commit d2ca606

5 files changed

Lines changed: 294 additions & 8 deletions

File tree

‎pkg/github/helper_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ const (
3131
GetReposByOwnerByRepo = "GET /repos/{owner}/{repo}"
3232
GetReposBranchesByOwnerByRepo = "GET /repos/{owner}/{repo}/branches"
3333
GetReposTagsByOwnerByRepo = "GET /repos/{owner}/{repo}/tags"
34+
GetReposCollaboratorsByOwnerByRepo = "GET /repos/{owner}/{repo}/collaborators"
3435
GetReposCommitsByOwnerByRepo = "GET /repos/{owner}/{repo}/commits"
3536
GetReposCommitsByOwnerByRepoByRef = "GET /repos/{owner}/{repo}/commits/{ref}"
3637
GetReposContentsByOwnerByRepoByPath = "GET /repos/{owner}/{repo}/contents/{path}"

‎pkg/github/issues.go‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
ghErrors "github.com/github/github-mcp-server/pkg/errors"
13+
"github.com/github/github-mcp-server/pkg/ifc"
1314
"github.com/github/github-mcp-server/pkg/inventory"
1415
"github.com/github/github-mcp-server/pkg/sanitize"
1516
"github.com/github/github-mcp-server/pkg/scopes"
@@ -130,6 +131,7 @@ type IssueFragment struct {
130131
// Common interface for all issue query types
131132
type IssueQueryResult interface {
132133
GetIssueFragment() IssueQueryFragment
134+
GetIsPrivate() bool
133135
}
134136

135137
type IssueQueryFragment struct {
@@ -146,28 +148,32 @@ type IssueQueryFragment struct {
146148
// ListIssuesQuery is the root query structure for fetching issues with optional label filtering.
147149
type ListIssuesQuery struct {
148150
Repository struct {
149-
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})"`
151+
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})"`
152+
IsPrivate githubv4.Boolean
150153
} `graphql:"repository(owner: $owner, name: $repo)"`
151154
}
152155

153156
// ListIssuesQueryTypeWithLabels is the query structure for fetching issues with optional label filtering.
154157
type ListIssuesQueryTypeWithLabels struct {
155158
Repository struct {
156-
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})"`
159+
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})"`
160+
IsPrivate githubv4.Boolean
157161
} `graphql:"repository(owner: $owner, name: $repo)"`
158162
}
159163

160164
// ListIssuesQueryWithSince is the query structure for fetching issues without label filtering but with since filtering.
161165
type ListIssuesQueryWithSince struct {
162166
Repository struct {
163-
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"`
167+
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"`
168+
IsPrivate githubv4.Boolean
164169
} `graphql:"repository(owner: $owner, name: $repo)"`
165170
}
166171

167172
// ListIssuesQueryTypeWithLabelsWithSince is the query structure for fetching issues with both label and since filtering.
168173
type ListIssuesQueryTypeWithLabelsWithSince struct {
169174
Repository struct {
170-
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"`
175+
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"`
176+
IsPrivate githubv4.Boolean
171177
} `graphql:"repository(owner: $owner, name: $repo)"`
172178
}
173179

@@ -176,18 +182,28 @@ func (q *ListIssuesQueryTypeWithLabels) GetIssueFragment() IssueQueryFragment {
176182
return q.Repository.Issues
177183
}
178184

185+
func (q *ListIssuesQueryTypeWithLabels) GetIsPrivate() bool { return bool(q.Repository.IsPrivate) }
186+
179187
func (q *ListIssuesQuery) GetIssueFragment() IssueQueryFragment {
180188
return q.Repository.Issues
181189
}
182190

191+
func (q *ListIssuesQuery) GetIsPrivate() bool { return bool(q.Repository.IsPrivate) }
192+
183193
func (q *ListIssuesQueryWithSince) GetIssueFragment() IssueQueryFragment {
184194
return q.Repository.Issues
185195
}
186196

197+
func (q *ListIssuesQueryWithSince) GetIsPrivate() bool { return bool(q.Repository.IsPrivate) }
198+
187199
func (q *ListIssuesQueryTypeWithLabelsWithSince) GetIssueFragment() IssueQueryFragment {
188200
return q.Repository.Issues
189201
}
190202

203+
func (q *ListIssuesQueryTypeWithLabelsWithSince) GetIsPrivate() bool {
204+
return bool(q.Repository.IsPrivate)
205+
}
206+
191207
func getIssueQueryType(hasLabels bool, hasSince bool) any {
192208
switch {
193209
case hasLabels && hasSince:
@@ -1568,11 +1584,35 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
15681584
}
15691585

15701586
var resp MinimalIssuesResponse
1587+
var isPrivate bool
15711588
if queryResult, ok := issueQuery.(IssueQueryResult); ok {
15721589
resp = convertToMinimalIssuesResponse(queryResult.GetIssueFragment())
1590+
isPrivate = queryResult.GetIsPrivate()
15731591
}
15741592

1575-
return MarshalledTextResult(resp), nil, nil
1593+
result := MarshalledTextResult(resp)
1594+
if deps.GetFlags(ctx).InsidersMode {
1595+
if result.Meta == nil {
1596+
result.Meta = mcp.Meta{}
1597+
}
1598+
var readers []string
1599+
if isPrivate {
1600+
restClient, err := deps.GetClient(ctx)
1601+
if err == nil {
1602+
if collaborators, err := FetchRepoCollaborators(ctx, restClient, owner, repo); err == nil {
1603+
readers = collaborators
1604+
}
1605+
}
1606+
// Fall back to the repository owner so the reader set is
1607+
// never empty for a private repository even if the
1608+
// collaborators lookup fails.
1609+
if len(readers) == 0 {
1610+
readers = []string{owner}
1611+
}
1612+
}
1613+
result.Meta["ifc"] = ifc.LabelListIssues(isPrivate, readers)
1614+
}
1615+
return result, nil, nil
15761616
})
15771617
}
15781618

‎pkg/github/issues_test.go‎

Lines changed: 171 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,6 +1117,7 @@ func Test_ListIssues(t *testing.T) {
11171117
},
11181118
"totalCount": 2,
11191119
},
1120+
"isPrivate": false,
11201121
},
11211122
})
11221123

@@ -1132,6 +1133,7 @@ func Test_ListIssues(t *testing.T) {
11321133
},
11331134
"totalCount": 2,
11341135
},
1136+
"isPrivate": false,
11351137
},
11361138
})
11371139

@@ -1147,6 +1149,7 @@ func Test_ListIssues(t *testing.T) {
11471149
},
11481150
"totalCount": 1,
11491151
},
1152+
"isPrivate": false,
11501153
},
11511154
})
11521155

@@ -1272,8 +1275,8 @@ func Test_ListIssues(t *testing.T) {
12721275
}
12731276

12741277
// Define the actual query strings that match the implementation
1275-
qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
1276-
qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
1278+
qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}"
1279+
qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}"
12771280

12781281
for _, tc := range tests {
12791282
t.Run(tc.name, func(t *testing.T) {
@@ -1349,6 +1352,172 @@ func Test_ListIssues(t *testing.T) {
13491352
}
13501353
}
13511354

1355+
func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
1356+
t.Parallel()
1357+
1358+
serverTool := ListIssues(translations.NullTranslationHelper)
1359+
1360+
mockIssues := []map[string]any{
1361+
{
1362+
"number": 1,
1363+
"title": "An issue",
1364+
"body": "body",
1365+
"state": "OPEN",
1366+
"databaseId": 1,
1367+
"createdAt": "2023-01-01T00:00:00Z",
1368+
"updatedAt": "2023-01-01T00:00:00Z",
1369+
"author": map[string]any{"login": "user1"},
1370+
"labels": map[string]any{"nodes": []map[string]any{}},
1371+
"comments": map[string]any{"totalCount": 0},
1372+
},
1373+
}
1374+
1375+
pageInfo := map[string]any{
1376+
"hasNextPage": false,
1377+
"hasPreviousPage": false,
1378+
"startCursor": "",
1379+
"endCursor": "",
1380+
}
1381+
1382+
makeResponse := func(isPrivate bool) githubv4mock.GQLResponse {
1383+
return githubv4mock.DataResponse(map[string]any{
1384+
"repository": map[string]any{
1385+
"issues": map[string]any{
1386+
"nodes": mockIssues,
1387+
"pageInfo": pageInfo,
1388+
"totalCount": 1,
1389+
},
1390+
"isPrivate": isPrivate,
1391+
},
1392+
})
1393+
}
1394+
1395+
query := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}"
1396+
1397+
vars := map[string]any{
1398+
"owner": "octocat",
1399+
"repo": "hello",
1400+
"states": []any{"OPEN", "CLOSED"},
1401+
"orderBy": "CREATED_AT",
1402+
"direction": "DESC",
1403+
"first": float64(30),
1404+
"after": (*string)(nil),
1405+
}
1406+
1407+
reqParams := map[string]any{"owner": "octocat", "repo": "hello"}
1408+
1409+
t.Run("insiders mode disabled omits ifc label from result meta", func(t *testing.T) {
1410+
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(false))
1411+
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
1412+
deps := BaseDeps{
1413+
GQLClient: gqlClient,
1414+
Flags: FeatureFlags{InsidersMode: false},
1415+
}
1416+
handler := serverTool.Handler(deps)
1417+
1418+
request := createMCPRequest(reqParams)
1419+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1420+
require.NoError(t, err)
1421+
require.False(t, result.IsError)
1422+
1423+
assert.Nil(t, result.Meta, "result meta should be nil when insiders mode is disabled")
1424+
})
1425+
1426+
t.Run("insiders mode enabled on public repo emits public untrusted label", func(t *testing.T) {
1427+
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(false))
1428+
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
1429+
deps := BaseDeps{
1430+
GQLClient: gqlClient,
1431+
Flags: FeatureFlags{InsidersMode: true},
1432+
}
1433+
handler := serverTool.Handler(deps)
1434+
1435+
request := createMCPRequest(reqParams)
1436+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1437+
require.NoError(t, err)
1438+
require.False(t, result.IsError)
1439+
1440+
require.NotNil(t, result.Meta)
1441+
ifcLabel, ok := result.Meta["ifc"]
1442+
require.True(t, ok, "result meta should contain ifc key")
1443+
1444+
ifcJSON, err := json.Marshal(ifcLabel)
1445+
require.NoError(t, err)
1446+
var ifcMap map[string]any
1447+
require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
1448+
1449+
assert.Equal(t, "untrusted", ifcMap["integrity"])
1450+
confList, ok := ifcMap["confidentiality"].([]any)
1451+
require.True(t, ok, "confidentiality should be a list")
1452+
require.Len(t, confList, 1)
1453+
assert.Equal(t, "public", confList[0])
1454+
})
1455+
1456+
t.Run("insiders mode enabled on private repo emits private untrusted label with collaborators", func(t *testing.T) {
1457+
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
1458+
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
1459+
restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1460+
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
1461+
{Login: github.Ptr("octocat")},
1462+
{Login: github.Ptr("alice")},
1463+
{Login: github.Ptr("bob")},
1464+
}),
1465+
}))
1466+
deps := BaseDeps{
1467+
Client: restClient,
1468+
GQLClient: gqlClient,
1469+
Flags: FeatureFlags{InsidersMode: true},
1470+
}
1471+
handler := serverTool.Handler(deps)
1472+
1473+
request := createMCPRequest(reqParams)
1474+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1475+
require.NoError(t, err)
1476+
require.False(t, result.IsError)
1477+
1478+
require.NotNil(t, result.Meta)
1479+
ifcLabel, ok := result.Meta["ifc"]
1480+
require.True(t, ok, "result meta should contain ifc key")
1481+
1482+
ifcJSON, err := json.Marshal(ifcLabel)
1483+
require.NoError(t, err)
1484+
var ifcMap map[string]any
1485+
require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
1486+
1487+
assert.Equal(t, "untrusted", ifcMap["integrity"])
1488+
confList, ok := ifcMap["confidentiality"].([]any)
1489+
require.True(t, ok, "confidentiality should be a list")
1490+
assert.Equal(t, []any{"octocat", "alice", "bob"}, confList)
1491+
})
1492+
1493+
t.Run("insiders mode enabled on private repo falls back to owner when collaborators lookup fails", func(t *testing.T) {
1494+
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
1495+
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
1496+
restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1497+
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusInternalServerError, "boom"),
1498+
}))
1499+
deps := BaseDeps{
1500+
Client: restClient,
1501+
GQLClient: gqlClient,
1502+
Flags: FeatureFlags{InsidersMode: true},
1503+
}
1504+
handler := serverTool.Handler(deps)
1505+
1506+
request := createMCPRequest(reqParams)
1507+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1508+
require.NoError(t, err)
1509+
require.False(t, result.IsError)
1510+
1511+
require.NotNil(t, result.Meta)
1512+
ifcJSON, err := json.Marshal(result.Meta["ifc"])
1513+
require.NoError(t, err)
1514+
var ifcMap map[string]any
1515+
require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
1516+
1517+
assert.Equal(t, []any{"octocat"}, ifcMap["confidentiality"])
1518+
})
1519+
}
1520+
13521521
func Test_UpdateIssue(t *testing.T) {
13531522
// Verify tool definition
13541523
serverTool := IssueWrite(translations.NullTranslationHelper)

‎pkg/github/repositories.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,34 @@ func CreateRepository(t translations.TranslationHelperFunc) inventory.ServerTool
654654
)
655655
}
656656

657+
// FetchRepoCollaborators returns the login names of all collaborators on a
658+
// repository. It is provided as a shared helper for IFC label computation so
659+
// tools can populate the reader set for private repositories. The full list
660+
// is fetched eagerly via pagination; callers are expected to invoke this only
661+
// when needed (e.g. private repos under InsidersMode).
662+
func FetchRepoCollaborators(ctx context.Context, client *github.Client, owner, repo string) ([]string, error) {
663+
opts := &github.ListCollaboratorsOptions{
664+
ListOptions: github.ListOptions{PerPage: 100},
665+
}
666+
var logins []string
667+
for {
668+
page, resp, err := client.Repositories.ListCollaborators(ctx, owner, repo, opts)
669+
if err != nil {
670+
return nil, err
671+
}
672+
for _, c := range page {
673+
if login := c.GetLogin(); login != "" {
674+
logins = append(logins, login)
675+
}
676+
}
677+
if resp == nil || resp.NextPage == 0 {
678+
break
679+
}
680+
opts.Page = resp.NextPage
681+
}
682+
return logins, nil
683+
}
684+
657685
// GetFileContents creates a tool to get the contents of a file or directory from a GitHub repository.
658686
func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool {
659687
return NewTool(

0 commit comments

Comments
 (0)