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/3] 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/3] 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/3] 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).