From d5105a4acf470f95bd517e3bd11d1ea4f6cf26f8 Mon Sep 17 00:00:00 2001 From: cointem <14988374+yin-jiashuai@user.noreply.gitee.com> Date: Sat, 29 Nov 2025 20:47:51 +0800 Subject: [PATCH 1/4] update: getOrgMembers --- pkg/github/__toolsnaps__/get_org_members.snap | 32 +++++ pkg/github/context_tools.go | 100 ++++++++++++++++ pkg/github/context_tools_test.go | 113 ++++++++++++++++++ pkg/github/tools.go | 7 +- 4 files changed, 250 insertions(+), 2 deletions(-) create mode 100644 pkg/github/__toolsnaps__/get_org_members.snap diff --git a/pkg/github/__toolsnaps__/get_org_members.snap b/pkg/github/__toolsnaps__/get_org_members.snap new file mode 100644 index 000000000..2b0bb4f1a --- /dev/null +++ b/pkg/github/__toolsnaps__/get_org_members.snap @@ -0,0 +1,32 @@ +{ + "annotations": { + "title": "Get organization members", + "readOnlyHint": true + }, + "description": "Get member users of a specific organization. Returns a list of user objects with fields: login, id, avatar_url, type. Limited to organizations accessible with current credentials", + "inputSchema": { + "properties": { + "org": { + "description": "Organization login (owner) to get members for.", + "type": "string" + } + ,"role": { + "description": "Filter by role: all, admin, member", + "type": "string" + }, + "per_page": { + "description": "Results per page (max 100)", + "type": "number" + }, + "page": { + "description": "Page number for pagination", + "type": "number" + } + }, + "required": [ + "org" + ], + "type": "object" + }, + "name": "get_org_members" +} \ No newline at end of file diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go index 06642aa15..c4825e8fa 100644 --- a/pkg/github/context_tools.go +++ b/pkg/github/context_tools.go @@ -2,10 +2,14 @@ package github import ( "context" + "fmt" + "strings" "time" ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" + "github.com/go-viper/mapstructure/v2" + "github.com/google/go-github/v79/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" "github.com/shurcooL/githubv4" @@ -249,3 +253,99 @@ func GetTeamMembers(getGQLClient GetGQLClientFn, t translations.TranslationHelpe return MarshalledTextResult(members), nil } } + +func getOrgMembers(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { + return mcp.NewTool("get_org_members", + mcp.WithDescription(t("TOOL_GET_ORG_MEMBERS_DESCRIPTION", "Get member users of a specific organization. Returns a list of user objects with fields: login, id, avatar_url, type. Limited to organizations accessible with current credentials")), + mcp.WithString("org", + mcp.Description(t("TOOL_GET_ORG_MEMBERS_ORG_DESCRIPTION", "Organization login (owner) to get members for.")), + mcp.Required(), + ), + mcp.WithString("role", + mcp.Description("Filter by role: all, admin, member"), + ), + mcp.WithNumber("per_page", + mcp.Description("Results per page (max 100)"), + ), + mcp.WithNumber("page", + mcp.Description("Page number for pagination"), + ), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_GET_ORG_MEMBERS_TITLE", "Get organization members"), + ReadOnlyHint: ToBoolPtr(true), + }), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // Decode params into struct to support optional numbers + var params struct { + Org string `mapstructure:"org"` + Role string `mapstructure:"role"` + PerPage int32 `mapstructure:"per_page"` + Page int32 `mapstructure:"page"` + } + if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + org := params.Org + role := params.Role + perPage := params.PerPage + page := params.Page + if org == "" { + return mcp.NewToolResultError("org is required"), nil + } + + // Defaults + if perPage <= 0 { + perPage = 30 + } + if perPage > 100 { + perPage = 100 + } + if page <= 0 { + page = 1 + } + client, err := getClient(ctx) + if err != nil { + return mcp.NewToolResultErrorFromErr("failed to get GitHub client", err), nil + } + + // Map role string to REST role filter expected by GitHub API ("all","admin","member"). + roleFilter := "" + if role != "" && strings.ToLower(role) != "all" { + roleFilter = strings.ToLower(role) + } + + // Use Organizations.ListMembers with pagination (page/per_page) + opts := &github.ListMembersOptions{ + Role: roleFilter, + ListOptions: github.ListOptions{ + PerPage: int(perPage), + Page: int(page), + }, + } + + users, resp, err := client.Organizations.ListMembers(ctx, org, opts) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to get organization members", resp, err), nil + } + + type outUser struct { + Login string `json:"login"` + ID string `json:"id"` + AvatarURL string `json:"avatar_url"` + Type string `json:"type"` + } + + var members []outUser + for _, u := range users { + members = append(members, outUser{ + Login: u.GetLogin(), + ID: fmt.Sprintf("%v", u.GetID()), + AvatarURL: u.GetAvatarURL(), + Type: u.GetType(), + }) + } + + return MarshalledTextResult(members), nil + } +} diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go index d3d5d0797..649f3da2b 100644 --- a/pkg/github/context_tools_test.go +++ b/pkg/github/context_tools_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net/http" "testing" "time" @@ -497,3 +498,115 @@ func Test_GetTeamMembers(t *testing.T) { }) } } + +func Test_GetOrgMembers(t *testing.T) { + t.Parallel() + + tool, _ := getOrgMembers(nil, translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "get_org_members", tool.Name) + assert.True(t, *tool.Annotations.ReadOnlyHint, "get_org_members tool should be read-only") + + // Mocked REST users as returned by GitHub REST API + mockUsers := []map[string]any{ + {"login": "user1", "id": 11, "avatar_url": "https://example.com/avatars/1", "type": "User"}, + {"login": "user2", "id": 22, "avatar_url": "https://example.com/avatars/2", "type": "User"}, + } + + tests := []struct { + name string + mockedClient *http.Client + stubGetClient GetClientFn + requestArgs map[string]any + expectToolErr bool + expectErrMsg string + expectCount int + }{ + { + name: "successful get org members", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(mock.MustMarshal(mockUsers)) + }), + ), + ), + requestArgs: map[string]any{"org": "testorg", "role": "all", "per_page": 30, "page": 1}, + expectCount: 2, + }, + { + name: "org with no members", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(mock.MustMarshal([]map[string]any{})) + }), + ), + ), + requestArgs: map[string]any{"org": "testorg", "role": "all", "per_page": 30, "page": 1}, + expectCount: 0, + }, + { + name: "getting client fails", + mockedClient: nil, + stubGetClient: stubGetClientFnErr("expected test error"), + requestArgs: map[string]any{"org": "testorg"}, + expectToolErr: true, + expectErrMsg: "failed to get GitHub client: expected test error", + }, + { + name: "api error", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, + mockResponse(t, http.StatusInternalServerError, map[string]string{"message": "boom"}), + ), + ), + requestArgs: map[string]any{"org": "testorg"}, + expectToolErr: true, + expectErrMsg: "Failed to get organization members", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var stubFn GetClientFn + if tc.stubGetClient != nil { + stubFn = tc.stubGetClient + } else if tc.mockedClient != nil { + stubFn = stubGetClientFromHTTPFn(tc.mockedClient) + } else { + stubFn = nil + } + + _, handler := getOrgMembers(stubFn, translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(context.Background(), request) + require.NoError(t, err) + textContent := getTextResult(t, result) + + if tc.expectToolErr { + assert.True(t, result.IsError) + assert.Contains(t, textContent.Text, tc.expectErrMsg) + return + } + + var members []struct { + Login string `json:"login"` + ID string `json:"id"` + AvatarURL string `json:"avatar_url"` + Type string `json:"type"` + } + err = json.Unmarshal([]byte(textContent.Text), &members) + require.NoError(t, err) + + assert.Len(t, members, tc.expectCount) + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 74f3d52f2..834a45856 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -14,8 +14,10 @@ import ( "github.com/shurcooL/githubv4" ) -type GetClientFn func(context.Context) (*github.Client, error) -type GetGQLClientFn func(context.Context) (*githubv4.Client, error) +type ( + GetClientFn func(context.Context) (*github.Client, error) + GetGQLClientFn func(context.Context) (*githubv4.Client, error) +) // ToolsetMetadata holds metadata for a toolset including its ID and description type ToolsetMetadata struct { @@ -312,6 +314,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(GetMe(getClient, t)), toolsets.NewServerTool(GetTeams(getClient, getGQLClient, t)), toolsets.NewServerTool(GetTeamMembers(getGQLClient, t)), + toolsets.NewServerTool(getOrgMembers(getClient, t)), ) gists := toolsets.NewToolset(ToolsetMetadataGists.ID, ToolsetMetadataGists.Description). From f33cf98e73e3ff8f96a6b8e827f85c790710dac6 Mon Sep 17 00:00:00 2001 From: cointem <14988374+yin-jiashuai@user.noreply.gitee.com> Date: Sun, 30 Nov 2025 00:25:24 +0800 Subject: [PATCH 2/4] update: List outside collaborators update: README.md --- README.md | 11 ++ pkg/github/__toolsnaps__/get_org_members.snap | 14 +- .../list_outside_collaborators.snap | 28 +++ pkg/github/context_tools.go | 89 ++++++++++ pkg/github/context_tools_test.go | 161 ++++++++++++++---- pkg/github/tools.go | 1 + 6 files changed, 266 insertions(+), 38 deletions(-) create mode 100644 pkg/github/__toolsnaps__/list_outside_collaborators.snap diff --git a/README.md b/README.md index c9a1fd70b..8ef3fc5b5 100644 --- a/README.md +++ b/README.md @@ -547,6 +547,12 @@ The following sets of tools are available: - **get_me** - Get my user profile - No parameters required +- **get_org_members** - Get organization members + - `org`: Organization login (owner) to get members for. (string, required) + - `page`: Page number for pagination (number, optional) + - `per_page`: Results per page (max 100) (number, optional) + - `role`: Filter by role: all, admin, member (string, optional) + - **get_team_members** - Get team members - `org`: Organization login (owner) that contains the team. (string, required) - `team_slug`: Team slug (string, required) @@ -554,6 +560,11 @@ The following sets of tools are available: - **get_teams** - Get teams - `user`: Username to get teams for. If not provided, uses the authenticated user. (string, optional) +- **list_outside_collaborators** - List outside collaborators + - `org`: The organization name (string, required) + - `page`: Page number for pagination (number, optional) + - `per_page`: Results per page (max 100) (number, optional) +
diff --git a/pkg/github/__toolsnaps__/get_org_members.snap b/pkg/github/__toolsnaps__/get_org_members.snap index 2b0bb4f1a..762caee30 100644 --- a/pkg/github/__toolsnaps__/get_org_members.snap +++ b/pkg/github/__toolsnaps__/get_org_members.snap @@ -9,18 +9,18 @@ "org": { "description": "Organization login (owner) to get members for.", "type": "string" - } - ,"role": { - "description": "Filter by role: all, admin, member", - "type": "string" + }, + "page": { + "description": "Page number for pagination", + "type": "number" }, "per_page": { "description": "Results per page (max 100)", "type": "number" }, - "page": { - "description": "Page number for pagination", - "type": "number" + "role": { + "description": "Filter by role: all, admin, member", + "type": "string" } }, "required": [ diff --git a/pkg/github/__toolsnaps__/list_outside_collaborators.snap b/pkg/github/__toolsnaps__/list_outside_collaborators.snap new file mode 100644 index 000000000..850924fc3 --- /dev/null +++ b/pkg/github/__toolsnaps__/list_outside_collaborators.snap @@ -0,0 +1,28 @@ +{ + "annotations": { + "title": "List outside collaborators", + "readOnlyHint": true + }, + "description": "List all outside collaborators of an organization (users with access to organization repositories but not members).", + "inputSchema": { + "properties": { + "org": { + "description": "The organization name", + "type": "string" + }, + "page": { + "description": "Page number for pagination", + "type": "number" + }, + "per_page": { + "description": "Results per page (max 100)", + "type": "number" + } + }, + "required": [ + "org" + ], + "type": "object" + }, + "name": "list_outside_collaborators" +} \ No newline at end of file diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go index c4825e8fa..54c99091d 100644 --- a/pkg/github/context_tools.go +++ b/pkg/github/context_tools.go @@ -334,6 +334,7 @@ func getOrgMembers(getClient GetClientFn, t translations.TranslationHelperFunc) ID string `json:"id"` AvatarURL string `json:"avatar_url"` Type string `json:"type"` + SiteAdmin bool `json:"site_admin"` } var members []outUser @@ -343,9 +344,97 @@ func getOrgMembers(getClient GetClientFn, t translations.TranslationHelperFunc) ID: fmt.Sprintf("%v", u.GetID()), AvatarURL: u.GetAvatarURL(), Type: u.GetType(), + SiteAdmin: u.GetSiteAdmin(), }) } return MarshalledTextResult(members), nil } } + +func listOutsideCollaborators(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { + return mcp.NewTool("list_outside_collaborators", + mcp.WithDescription(t("TOOL_LIST_OUTSIDE_COLLABORATORS_DESCRIPTION", "List all outside collaborators of an organization (users with access to organization repositories but not members).")), + mcp.WithString("org", + mcp.Description(t("TOOL_LIST_OUTSIDE_COLLABORATORS_ORG_DESCRIPTION", "The organization name")), + mcp.Required(), + ), + mcp.WithNumber("per_page", + mcp.Description("Results per page (max 100)"), + ), + mcp.WithNumber("page", + mcp.Description("Page number for pagination"), + ), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_OUTSIDE_COLLABORATORS_TITLE", "List outside collaborators"), + ReadOnlyHint: ToBoolPtr(true), + }), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // Decode params into struct to support optional numbers + var params struct { + Org string `mapstructure:"org"` + PerPage int32 `mapstructure:"per_page"` + Page int32 `mapstructure:"page"` + } + if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + org := params.Org + perPage := params.PerPage + page := params.Page + if org == "" { + return mcp.NewToolResultError("org is required"), nil + } + + // Defaults + if perPage <= 0 { + perPage = 30 + } + if perPage > 100 { + perPage = 100 + } + if page <= 0 { + page = 1 + } + + client, err := getClient(ctx) + if err != nil { + return mcp.NewToolResultErrorFromErr("failed to get GitHub client", err), nil + } + + // Use Organizations.ListOutsideCollaborators with pagination + opts := &github.ListOutsideCollaboratorsOptions{ + ListOptions: github.ListOptions{ + PerPage: int(perPage), + Page: int(page), + }, + } + + users, resp, err := client.Organizations.ListOutsideCollaborators(ctx, org, opts) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to list outside collaborators", resp, err), nil + } + + type outUser struct { + Login string `json:"login"` + ID string `json:"id"` + AvatarURL string `json:"avatar_url"` + Type string `json:"type"` + SiteAdmin bool `json:"site_admin"` + } + + var collaborators []outUser + for _, u := range users { + collaborators = append(collaborators, outUser{ + Login: u.GetLogin(), + ID: fmt.Sprintf("%v", u.GetID()), + AvatarURL: u.GetAvatarURL(), + Type: u.GetType(), + SiteAdmin: u.GetSiteAdmin(), + }) + } + + return MarshalledTextResult(collaborators), nil + } +} diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go index 649f3da2b..430b08f77 100644 --- a/pkg/github/context_tools_test.go +++ b/pkg/github/context_tools_test.go @@ -510,22 +510,21 @@ func Test_GetOrgMembers(t *testing.T) { // Mocked REST users as returned by GitHub REST API mockUsers := []map[string]any{ - {"login": "user1", "id": 11, "avatar_url": "https://example.com/avatars/1", "type": "User"}, - {"login": "user2", "id": 22, "avatar_url": "https://example.com/avatars/2", "type": "User"}, + {"login": "user1", "id": 11, "avatar_url": "https://example.com/avatars/1", "type": "User", "site_admin": false}, + {"login": "user2", "id": 22, "avatar_url": "https://example.com/avatars/2", "type": "User", "site_admin": false}, } tests := []struct { - name string - mockedClient *http.Client - stubGetClient GetClientFn - requestArgs map[string]any - expectToolErr bool - expectErrMsg string - expectCount int + name string + stubbedGetClientFn GetClientFn + requestArgs map[string]any + expectToolErr bool + expectErrMsg string + expectCount int }{ { name: "successful get org members", - mockedClient: mock.NewMockedHTTPClient( + stubbedGetClientFn: stubGetClientFromHTTPFn(mock.NewMockedHTTPClient( mock.WithRequestMatchHandler( mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -533,13 +532,13 @@ func Test_GetOrgMembers(t *testing.T) { _, _ = w.Write(mock.MustMarshal(mockUsers)) }), ), - ), + )), requestArgs: map[string]any{"org": "testorg", "role": "all", "per_page": 30, "page": 1}, expectCount: 2, }, { name: "org with no members", - mockedClient: mock.NewMockedHTTPClient( + stubbedGetClientFn: stubGetClientFromHTTPFn(mock.NewMockedHTTPClient( mock.WithRequestMatchHandler( mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -547,24 +546,25 @@ func Test_GetOrgMembers(t *testing.T) { _, _ = w.Write(mock.MustMarshal([]map[string]any{})) }), ), - ), + )), requestArgs: map[string]any{"org": "testorg", "role": "all", "per_page": 30, "page": 1}, expectCount: 0, }, { - name: "getting client fails", - mockedClient: nil, - stubGetClient: stubGetClientFnErr("expected test error"), - requestArgs: map[string]any{"org": "testorg"}, - expectToolErr: true, - expectErrMsg: "failed to get GitHub client: expected test error", + name: "getting client fails", + stubbedGetClientFn: stubGetClientFnErr("expected test error"), + requestArgs: map[string]any{"org": "testorg"}, + expectToolErr: true, + expectErrMsg: "failed to get GitHub client: expected test error", }, { name: "api error", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, - mockResponse(t, http.StatusInternalServerError, map[string]string{"message": "boom"}), + stubbedGetClientFn: stubGetClientFromHTTPFn( + mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, + mockResponse(t, http.StatusInternalServerError, map[string]string{"message": "boom"}), + ), ), ), requestArgs: map[string]any{"org": "testorg"}, @@ -575,14 +575,7 @@ func Test_GetOrgMembers(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - var stubFn GetClientFn - if tc.stubGetClient != nil { - stubFn = tc.stubGetClient - } else if tc.mockedClient != nil { - stubFn = stubGetClientFromHTTPFn(tc.mockedClient) - } else { - stubFn = nil - } + stubFn := tc.stubbedGetClientFn _, handler := getOrgMembers(stubFn, translations.NullTranslationHelper) @@ -602,6 +595,7 @@ func Test_GetOrgMembers(t *testing.T) { ID string `json:"id"` AvatarURL string `json:"avatar_url"` Type string `json:"type"` + SiteAdmin bool `json:"site_admin"` } err = json.Unmarshal([]byte(textContent.Text), &members) require.NoError(t, err) @@ -610,3 +604,108 @@ func Test_GetOrgMembers(t *testing.T) { }) } } + +func Test_ListOutsideCollaborators(t *testing.T) { + t.Parallel() + + tool, _ := listOutsideCollaborators(nil, translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "list_outside_collaborators", tool.Name) + assert.True(t, *tool.Annotations.ReadOnlyHint, "list_outside_collaborators tool should be read-only") + + mockUsers := []map[string]any{ + {"login": "ext1", "id": 101, "avatar_url": "https://example.com/a/1", "type": "User", "site_admin": false}, + {"login": "ext2", "id": 202, "avatar_url": "https://example.com/a/2", "type": "User", "site_admin": true}, + } + + tests := []struct { + name string + stubbedGetClientFn GetClientFn + requestArgs map[string]any + expectToolErr bool + expectErrMsg string + expectCount int + }{ + { + name: "successful list outside collaborators", + stubbedGetClientFn: stubGetClientFromHTTPFn(mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{Pattern: "/orgs/{org}/outside_collaborators", Method: http.MethodGet}, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(mock.MustMarshal(mockUsers)) + }), + ), + )), + requestArgs: map[string]any{"org": "testorg", "per_page": 30, "page": 1}, + expectCount: 2, + }, + { + name: "no collaborators", + stubbedGetClientFn: stubGetClientFromHTTPFn(mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{Pattern: "/orgs/{org}/outside_collaborators", Method: http.MethodGet}, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(mock.MustMarshal([]map[string]any{})) + }), + ), + )), + requestArgs: map[string]any{"org": "testorg", "per_page": 30, "page": 1}, + expectCount: 0, + }, + { + name: "getting client fails", + stubbedGetClientFn: stubGetClientFnErr("expected test error"), + requestArgs: map[string]any{"org": "testorg"}, + expectToolErr: true, + expectErrMsg: "failed to get GitHub client: expected test error", + }, + { + name: "api error", + stubbedGetClientFn: stubGetClientFromHTTPFn( + mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{Pattern: "/orgs/{org}/outside_collaborators", Method: http.MethodGet}, + mockResponse(t, http.StatusInternalServerError, map[string]string{"message": "boom"}), + ), + ), + ), + requestArgs: map[string]any{"org": "testorg"}, + expectToolErr: true, + expectErrMsg: "Failed to list outside collaborators", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + stubFn := tc.stubbedGetClientFn + + _, handler := listOutsideCollaborators(stubFn, translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(context.Background(), request) + require.NoError(t, err) + textContent := getTextResult(t, result) + + if tc.expectToolErr { + assert.True(t, result.IsError) + assert.Contains(t, textContent.Text, tc.expectErrMsg) + return + } + + var collabs []struct { + Login string `json:"login"` + ID string `json:"id"` + AvatarURL string `json:"avatar_url"` + Type string `json:"type"` + SiteAdmin bool `json:"site_admin"` + } + err = json.Unmarshal([]byte(textContent.Text), &collabs) + require.NoError(t, err) + + assert.Len(t, collabs, tc.expectCount) + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 834a45856..499a34bef 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -315,6 +315,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(GetTeams(getClient, getGQLClient, t)), toolsets.NewServerTool(GetTeamMembers(getGQLClient, t)), toolsets.NewServerTool(getOrgMembers(getClient, t)), + toolsets.NewServerTool(listOutsideCollaborators(getClient, t)), ) gists := toolsets.NewToolset(ToolsetMetadataGists.ID, ToolsetMetadataGists.Description). From 6ed22053d6c6dd40d5944769ae10a79e84a3fc2c Mon Sep 17 00:00:00 2001 From: cointem <14988374+yin-jiashuai@user.noreply.gitee.com> Date: Sun, 30 Nov 2025 00:54:16 +0800 Subject: [PATCH 3/4] fix:extract OutUser struct to file-level to remove duplication fix: change method name capitalization to adjust export visibility --- docs/remote-server.md | 2 +- pkg/github/context_tools.go | 36 +++++++++++++------------------- pkg/github/context_tools_test.go | 8 +++---- pkg/github/tools.go | 4 ++-- 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/docs/remote-server.md b/docs/remote-server.md index 1030911ef..e06d41a75 100644 --- a/docs/remote-server.md +++ b/docs/remote-server.md @@ -19,7 +19,7 @@ Below is a table of available toolsets for the remote GitHub MCP Server. Each to | Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) | |----------------|--------------------------------------------------|-------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| Default | ["Default" toolset](../README.md#default-toolset) | https://api.githubcopilot.com/mcp/ | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2F%22%7D) | [read-only](https://api.githubcopilot.com/mcp/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Freadonly%22%7D) | +| all | All available GitHub MCP tools | https://api.githubcopilot.com/mcp/ | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2F%22%7D) | [read-only](https://api.githubcopilot.com/mcp/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=github&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Freadonly%22%7D) | | Actions | GitHub Actions workflows and CI/CD operations | https://api.githubcopilot.com/mcp/x/actions | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-actions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Factions%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/actions/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-actions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Factions%2Freadonly%22%7D) | | Code Security | Code security related tools, such as GitHub Code Scanning | https://api.githubcopilot.com/mcp/x/code_security | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-code_security&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcode_security%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/code_security/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-code_security&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fcode_security%2Freadonly%22%7D) | | Dependabot | Dependabot tools | https://api.githubcopilot.com/mcp/x/dependabot | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-dependabot&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fdependabot%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/dependabot/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-dependabot&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fdependabot%2Freadonly%22%7D) | diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go index 54c99091d..d9a2de8f3 100644 --- a/pkg/github/context_tools.go +++ b/pkg/github/context_tools.go @@ -107,6 +107,14 @@ type OrganizationTeams struct { Teams []TeamInfo `json:"teams"` } +type OutUser struct { + Login string `json:"login"` + ID string `json:"id"` + AvatarURL string `json:"avatar_url"` + Type string `json:"type"` + SiteAdmin bool `json:"site_admin"` +} + func GetTeams(getClient GetClientFn, getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("get_teams", mcp.WithDescription(t("TOOL_GET_TEAMS_DESCRIPTION", "Get details of the teams the user is a member of. Limited to organizations accessible with current credentials")), @@ -254,7 +262,7 @@ func GetTeamMembers(getGQLClient GetGQLClientFn, t translations.TranslationHelpe } } -func getOrgMembers(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { +func GetOrgMembers(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("get_org_members", mcp.WithDescription(t("TOOL_GET_ORG_MEMBERS_DESCRIPTION", "Get member users of a specific organization. Returns a list of user objects with fields: login, id, avatar_url, type. Limited to organizations accessible with current credentials")), mcp.WithString("org", @@ -329,17 +337,9 @@ func getOrgMembers(getClient GetClientFn, t translations.TranslationHelperFunc) return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to get organization members", resp, err), nil } - type outUser struct { - Login string `json:"login"` - ID string `json:"id"` - AvatarURL string `json:"avatar_url"` - Type string `json:"type"` - SiteAdmin bool `json:"site_admin"` - } - - var members []outUser + var members []OutUser for _, u := range users { - members = append(members, outUser{ + members = append(members, OutUser{ Login: u.GetLogin(), ID: fmt.Sprintf("%v", u.GetID()), AvatarURL: u.GetAvatarURL(), @@ -352,7 +352,7 @@ func getOrgMembers(getClient GetClientFn, t translations.TranslationHelperFunc) } } -func listOutsideCollaborators(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { +func ListOutsideCollaborators(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("list_outside_collaborators", mcp.WithDescription(t("TOOL_LIST_OUTSIDE_COLLABORATORS_DESCRIPTION", "List all outside collaborators of an organization (users with access to organization repositories but not members).")), mcp.WithString("org", @@ -416,17 +416,9 @@ func listOutsideCollaborators(getClient GetClientFn, t translations.TranslationH return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to list outside collaborators", resp, err), nil } - type outUser struct { - Login string `json:"login"` - ID string `json:"id"` - AvatarURL string `json:"avatar_url"` - Type string `json:"type"` - SiteAdmin bool `json:"site_admin"` - } - - var collaborators []outUser + var collaborators []OutUser for _, u := range users { - collaborators = append(collaborators, outUser{ + collaborators = append(collaborators, OutUser{ Login: u.GetLogin(), ID: fmt.Sprintf("%v", u.GetID()), AvatarURL: u.GetAvatarURL(), diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go index 430b08f77..de83784a9 100644 --- a/pkg/github/context_tools_test.go +++ b/pkg/github/context_tools_test.go @@ -502,7 +502,7 @@ func Test_GetTeamMembers(t *testing.T) { func Test_GetOrgMembers(t *testing.T) { t.Parallel() - tool, _ := getOrgMembers(nil, translations.NullTranslationHelper) + tool, _ := GetOrgMembers(nil, translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "get_org_members", tool.Name) @@ -577,7 +577,7 @@ func Test_GetOrgMembers(t *testing.T) { t.Run(tc.name, func(t *testing.T) { stubFn := tc.stubbedGetClientFn - _, handler := getOrgMembers(stubFn, translations.NullTranslationHelper) + _, handler := GetOrgMembers(stubFn, translations.NullTranslationHelper) request := createMCPRequest(tc.requestArgs) result, err := handler(context.Background(), request) @@ -608,7 +608,7 @@ func Test_GetOrgMembers(t *testing.T) { func Test_ListOutsideCollaborators(t *testing.T) { t.Parallel() - tool, _ := listOutsideCollaborators(nil, translations.NullTranslationHelper) + tool, _ := ListOutsideCollaborators(nil, translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "list_outside_collaborators", tool.Name) @@ -682,7 +682,7 @@ func Test_ListOutsideCollaborators(t *testing.T) { t.Run(tc.name, func(t *testing.T) { stubFn := tc.stubbedGetClientFn - _, handler := listOutsideCollaborators(stubFn, translations.NullTranslationHelper) + _, handler := ListOutsideCollaborators(stubFn, translations.NullTranslationHelper) request := createMCPRequest(tc.requestArgs) result, err := handler(context.Background(), request) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index c8807d1ba..03736b062 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -314,8 +314,8 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(GetMe(getClient, t)), toolsets.NewServerTool(GetTeams(getClient, getGQLClient, t)), toolsets.NewServerTool(GetTeamMembers(getGQLClient, t)), - toolsets.NewServerTool(getOrgMembers(getClient, t)), - toolsets.NewServerTool(listOutsideCollaborators(getClient, t)), + toolsets.NewServerTool(GetOrgMembers(getClient, t)), + toolsets.NewServerTool(ListOutsideCollaborators(getClient, t)), ) gists := toolsets.NewToolset(ToolsetMetadataGists.ID, ToolsetMetadataGists.Description). From d7be20394dd5dbb1bce11ec1fa34de0efd6ea5d3 Mon Sep 17 00:00:00 2001 From: cointem Date: Mon, 9 Feb 2026 03:01:51 +0800 Subject: [PATCH 4/4] Refactor GitHub tools to use GraphQL for organization members and outside collaborators - Updated GetOrgMembers to utilize GraphQL for fetching organization members with roles. - Refactored ListOutsideCollaborators to use the GitHub API for listing outside collaborators. - Improved error handling and response structures in both tools. - Added tests for new GraphQL implementations and ensured compatibility with existing functionality. --- README.md | 8 +- go.mod | 11 +- go.sum | 8 + pkg/github/__toolsnaps__/get_org_members.snap | 12 +- .../list_outside_collaborators.snap | 12 +- pkg/github/context_tools.go | 409 +++++------------- pkg/github/context_tools_test.go | 251 ++++++----- pkg/github/tools.go | 2 + 8 files changed, 268 insertions(+), 445 deletions(-) diff --git a/README.md b/README.md index 9925ed9cd..ed4bc854e 100644 --- a/README.md +++ b/README.md @@ -672,9 +672,9 @@ The following sets of tools are available: - No parameters required - **get_org_members** - Get organization members + - **Required OAuth Scopes**: `read:org` + - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `write:org` - `org`: Organization login (owner) to get members for. (string, required) - - `page`: Page number for pagination (number, optional) - - `per_page`: Results per page (max 100) (number, optional) - `role`: Filter by role: all, admin, member (string, optional) - **get_team_members** - Get team members @@ -689,9 +689,9 @@ The following sets of tools are available: - `user`: Username to get teams for. If not provided, uses the authenticated user. (string, optional) - **list_outside_collaborators** - List outside collaborators + - **Required OAuth Scopes**: `read:org` + - **Accepted OAuth Scopes**: `admin:org`, `read:org`, `write:org` - `org`: The organization name (string, required) - - `page`: Page number for pagination (number, optional) - - `per_page`: Results per page (max 100) (number, optional)
diff --git a/go.mod b/go.mod index c6c6e2967..5a50e646a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,8 @@ module github.com/github/github-mcp-server -go 1.24.0 +go 1.24.4 + +toolchain go1.24.11 require ( github.com/google/go-github/v79 v79.0.0 @@ -13,6 +15,12 @@ require ( github.com/stretchr/testify v1.11.1 ) +require ( + github.com/google/go-github/v73 v73.0.0 // indirect + github.com/gorilla/mux v1.8.1 // indirect + golang.org/x/time v0.11.0 // indirect +) + require ( github.com/aymerick/douceur v0.2.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect @@ -27,6 +35,7 @@ require ( github.com/josharian/intern v1.0.0 // indirect github.com/lithammer/fuzzysearch v1.1.8 github.com/mailru/easyjson v0.7.7 // indirect + github.com/migueleliasweb/go-github-mock v1.5.0 github.com/modelcontextprotocol/go-sdk v1.2.0 github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect diff --git a/go.sum b/go.sum index d525cb0a1..c7a6abd14 100644 --- a/go.sum +++ b/go.sum @@ -24,6 +24,8 @@ github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVI github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/go-github/v73 v73.0.0 h1:aR+Utnh+Y4mMkS+2qLQwcQ/cF9mOTpdwnzlaw//rG24= +github.com/google/go-github/v73 v73.0.0/go.mod h1:fa6w8+/V+edSU0muqdhCVY7Beh1M8F1IlQPZIANKIYw= github.com/google/go-github/v79 v79.0.0 h1:MdodQojuFPBhmtwHiBcIGLw/e/wei2PvFX9ndxK0X4Y= github.com/google/go-github/v79 v79.0.0/go.mod h1:OAFbNhq7fQwohojb06iIIQAB9CBGYLq999myfUFnrS4= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= @@ -32,6 +34,8 @@ github.com/google/jsonschema-go v0.4.2 h1:tmrUohrwoLZZS/P3x7ex0WAVknEkBZM46iALbc github.com/google/jsonschema-go v0.4.2/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= github.com/gorilla/css v1.0.1 h1:ntNaBIghp6JmvWnxbZKANoLyuXTPZ4cAMlo6RyhlbO8= github.com/gorilla/css v1.0.1/go.mod h1:BvnYkspnSzMmwRK+b8/xgNPLiIuNZr6vbZBTPQ2A3b0= +github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= +github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/josephburnett/jd v1.9.2 h1:ECJRRFXCCqbtidkAHckHGSZm/JIaAxS1gygHLF8MI5Y= @@ -55,6 +59,8 @@ github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0 github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/microcosm-cc/bluemonday v1.0.27 h1:MpEUotklkwCSLeH+Qdx1VJgNqLlpY2KXwXFM08ygZfk= github.com/microcosm-cc/bluemonday v1.0.27/go.mod h1:jFi9vgW+H7c3V0lb6nR74Ib/DIB5OBs92Dimizgw2cA= +github.com/migueleliasweb/go-github-mock v1.5.0 h1:dIr6vgVz8QY9sDiDopWxk6pDw4d7K/xIcCk/NQe4ajM= +github.com/migueleliasweb/go-github-mock v1.5.0/go.mod h1:/DUmhXkxrgVlDOVBqGoUXkV4w0ms5n1jDQHotYm135o= github.com/modelcontextprotocol/go-sdk v1.2.0 h1:Y23co09300CEk8iZ/tMxIX1dVmKZkzoSBZOpJwUnc/s= github.com/modelcontextprotocol/go-sdk v1.2.0/go.mod h1:6fM3LCm3yV7pAs8isnKLn07oKtB0MP9LHd3DfAcKw10= github.com/muesli/cache2go v0.0.0-20221011235721-518229cd8021 h1:31Y+Yu373ymebRdJN1cWLLooHH8xAr0MhKTEJGV/87g= @@ -138,6 +144,8 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng= golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU= +golang.org/x/time v0.11.0 h1:/bpjEDfN9tkoN/ryeYHnv5hcMlc8ncjMcM4XBk5NWV0= +golang.org/x/time v0.11.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/pkg/github/__toolsnaps__/get_org_members.snap b/pkg/github/__toolsnaps__/get_org_members.snap index 762caee30..6c0b71525 100644 --- a/pkg/github/__toolsnaps__/get_org_members.snap +++ b/pkg/github/__toolsnaps__/get_org_members.snap @@ -1,7 +1,7 @@ { "annotations": { - "title": "Get organization members", - "readOnlyHint": true + "readOnlyHint": true, + "title": "Get organization members" }, "description": "Get member users of a specific organization. Returns a list of user objects with fields: login, id, avatar_url, type. Limited to organizations accessible with current credentials", "inputSchema": { @@ -10,14 +10,6 @@ "description": "Organization login (owner) to get members for.", "type": "string" }, - "page": { - "description": "Page number for pagination", - "type": "number" - }, - "per_page": { - "description": "Results per page (max 100)", - "type": "number" - }, "role": { "description": "Filter by role: all, admin, member", "type": "string" diff --git a/pkg/github/__toolsnaps__/list_outside_collaborators.snap b/pkg/github/__toolsnaps__/list_outside_collaborators.snap index 850924fc3..60bbbdfa1 100644 --- a/pkg/github/__toolsnaps__/list_outside_collaborators.snap +++ b/pkg/github/__toolsnaps__/list_outside_collaborators.snap @@ -1,7 +1,7 @@ { "annotations": { - "title": "List outside collaborators", - "readOnlyHint": true + "readOnlyHint": true, + "title": "List outside collaborators" }, "description": "List all outside collaborators of an organization (users with access to organization repositories but not members).", "inputSchema": { @@ -9,14 +9,6 @@ "org": { "description": "The organization name", "type": "string" - }, - "page": { - "description": "Page number for pagination", - "type": "number" - }, - "per_page": { - "description": "Results per page (max 100)", - "type": "number" } }, "required": [ diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go index 65eacf853..35379c850 100644 --- a/pkg/github/context_tools.go +++ b/pkg/github/context_tools.go @@ -3,7 +3,6 @@ package github import ( "context" "encoding/json" - "fmt" "strings" "time" @@ -11,8 +10,6 @@ import ( "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" - "github.com/go-viper/mapstructure/v2" - "github.com/google/go-github/v79/github" "github.com/github/github-mcp-server/pkg/utils" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -115,8 +112,6 @@ type OrganizationTeams struct { Teams []TeamInfo `json:"teams"` } - - type OutUser struct { Login string `json:"login"` ID string `json:"id"` @@ -131,7 +126,6 @@ func GetTeams(t translations.TranslationHelperFunc) inventory.ServerTool { mcp.Tool{ Name: "get_teams", Description: t("TOOL_GET_TEAMS_DESCRIPTION", "Get details of the teams the user is a member of. Limited to organizations accessible with current credentials"), - Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_GET_TEAMS_TITLE", "Get teams"), ReadOnlyHint: true, @@ -295,341 +289,136 @@ func GetTeamMembers(t translations.TranslationHelperFunc) inventory.ServerTool { ) } -func GetOrgMembers(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("get_org_members", - mcp.WithDescription(t("TOOL_GET_ORG_MEMBERS_DESCRIPTION", "Get member users of a specific organization. Returns a list of user objects with fields: login, id, avatar_url, type. Limited to organizations accessible with current credentials")), - mcp.WithString("org", - mcp.Description(t("TOOL_GET_ORG_MEMBERS_ORG_DESCRIPTION", "Organization login (owner) to get members for.")), - mcp.Required(), - ), - mcp.WithString("role", - mcp.Description("Filter by role: all, admin, member"), - ), - mcp.WithNumber("per_page", - mcp.Description("Results per page (max 100)"), - ), - mcp.WithNumber("page", - mcp.Description("Page number for pagination"), - ), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ +func GetOrgMembers(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataContext, + mcp.Tool{ + Name: "get_org_members", + Description: t("TOOL_GET_ORG_MEMBERS_DESCRIPTION", "Get member users of a specific organization. Returns a list of user objects with fields: login, id, avatar_url, type. Limited to organizations accessible with current credentials"), + Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_GET_ORG_MEMBERS_TITLE", "Get organization members"), - ReadOnlyHint: ToBoolPtr(true), - }), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - // Decode params into struct to support optional numbers - var params struct { - Org string `mapstructure:"org"` - Role string `mapstructure:"role"` - PerPage int32 `mapstructure:"per_page"` - Page int32 `mapstructure:"page"` - } - if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - org := params.Org - role := params.Role - perPage := params.PerPage - page := params.Page - if org == "" { - return mcp.NewToolResultError("org is required"), nil - } - - // Defaults - if perPage <= 0 { - perPage = 30 - } - if perPage > 100 { - perPage = 100 - } - if page <= 0 { - page = 1 - } - client, err := getClient(ctx) - if err != nil { - return mcp.NewToolResultErrorFromErr("failed to get GitHub client", err), nil - } - - // Map role string to REST role filter expected by GitHub API ("all","admin","member"). - roleFilter := "" - if role != "" && strings.ToLower(role) != "all" { - roleFilter = strings.ToLower(role) - } - - // Use Organizations.ListMembers with pagination (page/per_page) - opts := &github.ListMembersOptions{ - Role: roleFilter, - ListOptions: github.ListOptions{ - PerPage: int(perPage), - Page: int(page), + ReadOnlyHint: true, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "org": { + Type: "string", + Description: t("TOOL_GET_ORG_MEMBERS_ORG_DESCRIPTION", "Organization login (owner) to get members for."), + }, + "role": { + Type: "string", + Description: "Filter by role: all, admin, member", + }, }, - } - - users, resp, err := client.Organizations.ListMembers(ctx, org, opts) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to get organization members", resp, err), nil - } - - var members []OutUser - for _, u := range users { - members = append(members, OutUser{ - Login: u.GetLogin(), - ID: fmt.Sprintf("%v", u.GetID()), - AvatarURL: u.GetAvatarURL(), - Type: u.GetType(), - SiteAdmin: u.GetSiteAdmin(), - }) - } - - return MarshalledTextResult(members), nil, nil + Required: []string{"org"}, + }, }, - ) -} - -func ListOutsideCollaborators(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("list_outside_collaborators", - mcp.WithDescription(t("TOOL_LIST_OUTSIDE_COLLABORATORS_DESCRIPTION", "List all outside collaborators of an organization (users with access to organization repositories but not members).")), - mcp.WithString("org", - mcp.Description(t("TOOL_LIST_OUTSIDE_COLLABORATORS_ORG_DESCRIPTION", "The organization name")), - mcp.Required(), - ), - mcp.WithNumber("per_page", - mcp.Description("Results per page (max 100)"), - ), - mcp.WithNumber("page", - mcp.Description("Page number for pagination"), - ), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_LIST_OUTSIDE_COLLABORATORS_TITLE", "List outside collaborators"), - ReadOnlyHint: ToBoolPtr(true), - }), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - // Decode params into struct to support optional numbers - var params struct { - Org string `mapstructure:"org"` - PerPage int32 `mapstructure:"per_page"` - Page int32 `mapstructure:"page"` - } - if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - org := params.Org - perPage := params.PerPage - page := params.Page - if org == "" { - return mcp.NewToolResultError("org is required"), nil - } - - // Defaults - if perPage <= 0 { - perPage = 30 - } - if perPage > 100 { - perPage = 100 - } - if page <= 0 { - page = 1 - } - - client, err := getClient(ctx) + []scopes.Scope{scopes.ReadOrg}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + org, err := RequiredParam[string](args, "org") if err != nil { - return mcp.NewToolResultErrorFromErr("failed to get GitHub client", err), nil - } - - // Use Organizations.ListOutsideCollaborators with pagination - opts := &github.ListOutsideCollaboratorsOptions{ - ListOptions: github.ListOptions{ - PerPage: int(perPage), - Page: int(page), - }, + return utils.NewToolResultError(err.Error()), nil, nil } - users, resp, err := client.Organizations.ListOutsideCollaborators(ctx, org, opts) + role, err := OptionalParam[string](args, "role") if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to list outside collaborators", resp, err), nil - } - - var collaborators []OutUser - for _, u := range users { - collaborators = append(collaborators, OutUser{ - Login: u.GetLogin(), - ID: fmt.Sprintf("%v", u.GetID()), - AvatarURL: u.GetAvatarURL(), - Type: u.GetType(), - SiteAdmin: u.GetSiteAdmin(), - }) - } - - return MarshalledTextResult(collaborators), nil - } -} - -func GetOrgMembers(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("get_org_members", - mcp.WithDescription(t("TOOL_GET_ORG_MEMBERS_DESCRIPTION", "Get member users of a specific organization. Returns a list of user objects with fields: login, id, avatar_url, type. Limited to organizations accessible with current credentials")), - mcp.WithString("org", - mcp.Description(t("TOOL_GET_ORG_MEMBERS_ORG_DESCRIPTION", "Organization login (owner) to get members for.")), - mcp.Required(), - ), - mcp.WithString("role", - mcp.Description("Filter by role: all, admin, member"), - ), - mcp.WithNumber("per_page", - mcp.Description("Results per page (max 100)"), - ), - mcp.WithNumber("page", - mcp.Description("Page number for pagination"), - ), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_GET_ORG_MEMBERS_TITLE", "Get organization members"), - ReadOnlyHint: ToBoolPtr(true), - }), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - // Decode params into struct to support optional numbers - var params struct { - Org string `mapstructure:"org"` - Role string `mapstructure:"role"` - PerPage int32 `mapstructure:"per_page"` - Page int32 `mapstructure:"page"` - } - if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - org := params.Org - role := params.Role - perPage := params.PerPage - page := params.Page - if org == "" { - return mcp.NewToolResultError("org is required"), nil + return utils.NewToolResultError(err.Error()), nil, nil } - // Defaults - if perPage <= 0 { - perPage = 30 - } - if perPage > 100 { - perPage = 100 - } - if page <= 0 { - page = 1 - } - client, err := getClient(ctx) + gqlClient, err := deps.GetGQLClient(ctx) if err != nil { - return mcp.NewToolResultErrorFromErr("failed to get GitHub client", err), nil + return utils.NewToolResultErrorFromErr("failed to get GitHub GQL client", err), nil, nil } - // Map role string to REST role filter expected by GitHub API ("all","admin","member"). - roleFilter := "" - if role != "" && strings.ToLower(role) != "all" { - roleFilter = strings.ToLower(role) + roleFilter := strings.ToLower(strings.TrimSpace(role)) + if roleFilter == "all" { + roleFilter = "" } - // Use Organizations.ListMembers with pagination (page/per_page) - opts := &github.ListMembersOptions{ - Role: roleFilter, - ListOptions: github.ListOptions{ - PerPage: int(perPage), - Page: int(page), - }, + var q struct { + Organization struct { + MembersWithRole struct { + Edges []struct { + Role githubv4.String + Node struct { + Login githubv4.String + } + } + } `graphql:"membersWithRole(first: 100)"` + } `graphql:"organization(login: $org)"` + } + vars := map[string]any{ + "org": githubv4.String(org), } - users, resp, err := client.Organizations.ListMembers(ctx, org, opts) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to get organization members", resp, err), nil + if err := gqlClient.Query(ctx, &q, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to get organization members", err), nil, nil } - var members []OutUser - for _, u := range users { - members = append(members, OutUser{ - Login: u.GetLogin(), - ID: fmt.Sprintf("%v", u.GetID()), - AvatarURL: u.GetAvatarURL(), - Type: u.GetType(), - SiteAdmin: u.GetSiteAdmin(), - }) + members := make([]struct { + Login string `json:"login"` + Role string `json:"role"` + }, 0, len(q.Organization.MembersWithRole.Edges)) + for _, member := range q.Organization.MembersWithRole.Edges { + if roleFilter != "" && strings.ToLower(string(member.Role)) != roleFilter { + continue + } + members = append(members, struct { + Login string `json:"login"` + Role string `json:"role"` + }{Login: string(member.Node.Login), Role: string(member.Role)}) } - return MarshalledTextResult(members), nil - } + return MarshalledTextResult(members), nil, nil + }, + ) } -func ListOutsideCollaborators(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("list_outside_collaborators", - mcp.WithDescription(t("TOOL_LIST_OUTSIDE_COLLABORATORS_DESCRIPTION", "List all outside collaborators of an organization (users with access to organization repositories but not members).")), - mcp.WithString("org", - mcp.Description(t("TOOL_LIST_OUTSIDE_COLLABORATORS_ORG_DESCRIPTION", "The organization name")), - mcp.Required(), - ), - mcp.WithNumber("per_page", - mcp.Description("Results per page (max 100)"), - ), - mcp.WithNumber("page", - mcp.Description("Page number for pagination"), - ), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ +func ListOutsideCollaborators(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataContext, + mcp.Tool{ + Name: "list_outside_collaborators", + Description: t("TOOL_LIST_OUTSIDE_COLLABORATORS_DESCRIPTION", "List all outside collaborators of an organization (users with access to organization repositories but not members)."), + Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_LIST_OUTSIDE_COLLABORATORS_TITLE", "List outside collaborators"), - ReadOnlyHint: ToBoolPtr(true), - }), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - // Decode params into struct to support optional numbers - var params struct { - Org string `mapstructure:"org"` - PerPage int32 `mapstructure:"per_page"` - Page int32 `mapstructure:"page"` - } - if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - org := params.Org - perPage := params.PerPage - page := params.Page - if org == "" { - return mcp.NewToolResultError("org is required"), nil - } - - // Defaults - if perPage <= 0 { - perPage = 30 - } - if perPage > 100 { - perPage = 100 - } - if page <= 0 { - page = 1 - } - - client, err := getClient(ctx) + ReadOnlyHint: true, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "org": { + Type: "string", + Description: t("TOOL_LIST_OUTSIDE_COLLABORATORS_ORG_DESCRIPTION", "The organization name"), + }, + }, + Required: []string{"org"}, + }, + }, + []scopes.Scope{scopes.ReadOrg}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + org, err := RequiredParam[string](args, "org") if err != nil { - return mcp.NewToolResultErrorFromErr("failed to get GitHub client", err), nil + return utils.NewToolResultError(err.Error()), nil, nil } - // Use Organizations.ListOutsideCollaborators with pagination - opts := &github.ListOutsideCollaboratorsOptions{ - ListOptions: github.ListOptions{ - PerPage: int(perPage), - Page: int(page), - }, + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - users, resp, err := client.Organizations.ListOutsideCollaborators(ctx, org, opts) + users, resp, err := client.Organizations.ListOutsideCollaborators(ctx, org, nil) if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to list outside collaborators", resp, err), nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to list outside collaborators", resp, err), nil, nil } - var collaborators []OutUser - for _, u := range users { - collaborators = append(collaborators, OutUser{ - Login: u.GetLogin(), - ID: fmt.Sprintf("%v", u.GetID()), - AvatarURL: u.GetAvatarURL(), - Type: u.GetType(), - SiteAdmin: u.GetSiteAdmin(), - }) + collaborators := make([]string, 0, len(users)) + for _, user := range users { + collaborators = append(collaborators, user.GetLogin()) } - return MarshalledTextResult(collaborators), nil - } + return MarshalledTextResult(collaborators), nil, nil + }, + ) } diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go index ee2e4ffd8..6773f31b2 100644 --- a/pkg/github/context_tools_test.go +++ b/pkg/github/context_tools_test.go @@ -11,6 +11,7 @@ import ( "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v79/github" + "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -517,71 +518,100 @@ func Test_GetTeamMembers(t *testing.T) { func Test_GetOrgMembers(t *testing.T) { t.Parallel() - tool, _ := GetOrgMembers(nil, translations.NullTranslationHelper) + serverTool := GetOrgMembers(translations.NullTranslationHelper) + tool := serverTool.Tool require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "get_org_members", tool.Name) - assert.True(t, *tool.Annotations.ReadOnlyHint, "get_org_members tool should be read-only") + assert.True(t, tool.Annotations.ReadOnlyHint, "get_org_members tool should be read-only") + + var membersQuery struct { + Organization struct { + MembersWithRole struct { + Edges []struct { + Role githubv4.String + Node struct { + Login githubv4.String + } + } + } `graphql:"membersWithRole(first: 100)"` + } `graphql:"organization(login: $org)"` + } + vars := map[string]any{ + "org": githubv4.String("testorg"), + } - // Mocked REST users as returned by GitHub REST API - mockUsers := []map[string]any{ - {"login": "user1", "id": 11, "avatar_url": "https://example.com/avatars/1", "type": "User", "site_admin": false}, - {"login": "user2", "id": 22, "avatar_url": "https://example.com/avatars/2", "type": "User", "site_admin": false}, + mockMembersResponse := githubv4mock.DataResponse(map[string]any{ + "organization": map[string]any{ + "membersWithRole": map[string]any{ + "edges": []map[string]any{ + { + "role": "ADMIN", + "node": map[string]any{ + "login": "user1", + }, + }, + { + "role": "MEMBER", + "node": map[string]any{ + "login": "user2", + }, + }, + }, + }, + }, + }) + + mockNoMembersResponse := githubv4mock.DataResponse(map[string]any{ + "organization": map[string]any{ + "membersWithRole": map[string]any{ + "edges": []map[string]any{}, + }, + }, + }) + + gqlClientWithMembers := func(response githubv4mock.GQLResponse) *githubv4.Client { + matcher := githubv4mock.NewQueryMatcher(membersQuery, vars, response) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + return githubv4.NewClient(httpClient) } tests := []struct { - name string - stubbedGetClientFn GetClientFn - requestArgs map[string]any - expectToolErr bool - expectErrMsg string - expectCount int + name string + makeDeps func() ToolDependencies + requestArgs map[string]any + expectToolErr bool + expectErrMsg string + expectCount int }{ { name: "successful get org members", - stubbedGetClientFn: stubGetClientFromHTTPFn(mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = w.Write(mock.MustMarshal(mockUsers)) - }), - ), - )), - requestArgs: map[string]any{"org": "testorg", "role": "all", "per_page": 30, "page": 1}, + makeDeps: func() ToolDependencies { + return BaseDeps{GQLClient: gqlClientWithMembers(mockMembersResponse)} + }, + requestArgs: map[string]any{"org": "testorg", "role": "all"}, expectCount: 2, }, { name: "org with no members", - stubbedGetClientFn: stubGetClientFromHTTPFn(mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = w.Write(mock.MustMarshal([]map[string]any{})) - }), - ), - )), - requestArgs: map[string]any{"org": "testorg", "role": "all", "per_page": 30, "page": 1}, + makeDeps: func() ToolDependencies { + return BaseDeps{GQLClient: gqlClientWithMembers(mockNoMembersResponse)} + }, + requestArgs: map[string]any{"org": "testorg", "role": "all"}, expectCount: 0, }, { - name: "getting client fails", - stubbedGetClientFn: stubGetClientFnErr("expected test error"), - requestArgs: map[string]any{"org": "testorg"}, - expectToolErr: true, - expectErrMsg: "failed to get GitHub client: expected test error", + name: "getting client fails", + makeDeps: func() ToolDependencies { return stubDeps{gqlClientFn: stubGQLClientFnErr("expected test error")} }, + requestArgs: map[string]any{"org": "testorg"}, + expectToolErr: true, + expectErrMsg: "failed to get GitHub GQL client: expected test error", }, { name: "api error", - stubbedGetClientFn: stubGetClientFromHTTPFn( - mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.EndpointPattern{Pattern: "/orgs/{org}/members", Method: http.MethodGet}, - mockResponse(t, http.StatusInternalServerError, map[string]string{"message": "boom"}), - ), - ), - ), + makeDeps: func() ToolDependencies { + return BaseDeps{GQLClient: gqlClientWithMembers(githubv4mock.ErrorResponse("boom"))} + }, requestArgs: map[string]any{"org": "testorg"}, expectToolErr: true, expectErrMsg: "Failed to get organization members", @@ -590,27 +620,25 @@ func Test_GetOrgMembers(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - stubFn := tc.stubbedGetClientFn - - _, handler := GetOrgMembers(stubFn, translations.NullTranslationHelper) + deps := tc.makeDeps() + handler := serverTool.Handler(deps) request := createMCPRequest(tc.requestArgs) - result, err := handler(context.Background(), request) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) - textContent := getTextResult(t, result) if tc.expectToolErr { - assert.True(t, result.IsError) - assert.Contains(t, textContent.Text, tc.expectErrMsg) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectErrMsg) return } + textContent := getTextResult(t, result) + var members []struct { - Login string `json:"login"` - ID string `json:"id"` - AvatarURL string `json:"avatar_url"` - Type string `json:"type"` - SiteAdmin bool `json:"site_admin"` + Login string `json:"login"` + Role string `json:"role"` } err = json.Unmarshal([]byte(textContent.Text), &members) require.NoError(t, err) @@ -623,70 +651,78 @@ func Test_GetOrgMembers(t *testing.T) { func Test_ListOutsideCollaborators(t *testing.T) { t.Parallel() - tool, _ := ListOutsideCollaborators(nil, translations.NullTranslationHelper) + serverTool := ListOutsideCollaborators(translations.NullTranslationHelper) + tool := serverTool.Tool require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "list_outside_collaborators", tool.Name) - assert.True(t, *tool.Annotations.ReadOnlyHint, "list_outside_collaborators tool should be read-only") + assert.True(t, tool.Annotations.ReadOnlyHint, "list_outside_collaborators tool should be read-only") mockUsers := []map[string]any{ - {"login": "ext1", "id": 101, "avatar_url": "https://example.com/a/1", "type": "User", "site_admin": false}, - {"login": "ext2", "id": 202, "avatar_url": "https://example.com/a/2", "type": "User", "site_admin": true}, + {"login": "ext1"}, + {"login": "ext2"}, } tests := []struct { - name string - stubbedGetClientFn GetClientFn - requestArgs map[string]any - expectToolErr bool - expectErrMsg string - expectCount int + name string + makeDeps func() ToolDependencies + requestArgs map[string]any + expectToolErr bool + expectErrMsg string + expectCount int }{ { name: "successful list outside collaborators", - stubbedGetClientFn: stubGetClientFromHTTPFn(mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.EndpointPattern{Pattern: "/orgs/{org}/outside_collaborators", Method: http.MethodGet}, - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = w.Write(mock.MustMarshal(mockUsers)) - }), - ), - )), - requestArgs: map[string]any{"org": "testorg", "per_page": 30, "page": 1}, + makeDeps: func() ToolDependencies { + httpClient := mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{Pattern: "/orgs/{org}/outside_collaborators", Method: http.MethodGet}, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(mock.MustMarshal(mockUsers)) + }), + ), + ) + return BaseDeps{Client: github.NewClient(httpClient)} + }, + requestArgs: map[string]any{"org": "testorg"}, expectCount: 2, }, { name: "no collaborators", - stubbedGetClientFn: stubGetClientFromHTTPFn(mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.EndpointPattern{Pattern: "/orgs/{org}/outside_collaborators", Method: http.MethodGet}, - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = w.Write(mock.MustMarshal([]map[string]any{})) - }), - ), - )), - requestArgs: map[string]any{"org": "testorg", "per_page": 30, "page": 1}, + makeDeps: func() ToolDependencies { + httpClient := mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{Pattern: "/orgs/{org}/outside_collaborators", Method: http.MethodGet}, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write(mock.MustMarshal([]map[string]any{})) + }), + ), + ) + return BaseDeps{Client: github.NewClient(httpClient)} + }, + requestArgs: map[string]any{"org": "testorg"}, expectCount: 0, }, { - name: "getting client fails", - stubbedGetClientFn: stubGetClientFnErr("expected test error"), - requestArgs: map[string]any{"org": "testorg"}, - expectToolErr: true, - expectErrMsg: "failed to get GitHub client: expected test error", + name: "getting client fails", + makeDeps: func() ToolDependencies { return stubDeps{clientFn: stubClientFnErr("expected test error")} }, + requestArgs: map[string]any{"org": "testorg"}, + expectToolErr: true, + expectErrMsg: "failed to get GitHub client: expected test error", }, { name: "api error", - stubbedGetClientFn: stubGetClientFromHTTPFn( - mock.NewMockedHTTPClient( + makeDeps: func() ToolDependencies { + httpClient := mock.NewMockedHTTPClient( mock.WithRequestMatchHandler( mock.EndpointPattern{Pattern: "/orgs/{org}/outside_collaborators", Method: http.MethodGet}, mockResponse(t, http.StatusInternalServerError, map[string]string{"message": "boom"}), ), - ), - ), + ) + return BaseDeps{Client: github.NewClient(httpClient)} + }, requestArgs: map[string]any{"org": "testorg"}, expectToolErr: true, expectErrMsg: "Failed to list outside collaborators", @@ -695,28 +731,23 @@ func Test_ListOutsideCollaborators(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - stubFn := tc.stubbedGetClientFn - - _, handler := ListOutsideCollaborators(stubFn, translations.NullTranslationHelper) + deps := tc.makeDeps() + handler := serverTool.Handler(deps) request := createMCPRequest(tc.requestArgs) - result, err := handler(context.Background(), request) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) - textContent := getTextResult(t, result) if tc.expectToolErr { - assert.True(t, result.IsError) - assert.Contains(t, textContent.Text, tc.expectErrMsg) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectErrMsg) return } - var collabs []struct { - Login string `json:"login"` - ID string `json:"id"` - AvatarURL string `json:"avatar_url"` - Type string `json:"type"` - SiteAdmin bool `json:"site_admin"` - } + textContent := getTextResult(t, result) + + var collabs []string err = json.Unmarshal([]byte(textContent.Text), &collabs) require.NoError(t, err) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 28af63ea0..172a21890 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -161,6 +161,8 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GetMe(t), GetTeams(t), GetTeamMembers(t), + GetOrgMembers(t), + ListOutsideCollaborators(t), // Repository tools SearchRepositories(t),