feat: Add get organization members and list all outside collaborators of an organization #1508
Conversation
update: README.md
| refs #1331 |
There was a problem hiding this comment.
Pull request overview
This PR adds two new tools for organization management: get_org_members to retrieve organization members with role filtering and pagination, and list_outside_collaborators to list users with repository access who aren't organization members. The implementation follows existing patterns with comprehensive test coverage and documentation updates.
- Adds organization member and outside collaborator listing capabilities
- Includes full test coverage with success and error scenarios
- Updates README and toolsnap schema files
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/github/context_tools.go | Implements the two new organization tools with pagination support and user data transformation |
| pkg/github/context_tools_test.go | Adds comprehensive unit tests for both tools covering success, empty results, client errors, and API errors |
| pkg/github/tools.go | Registers the new tools in the context toolset and refactors type declarations into grouped format |
| pkg/github/toolsnaps/get_org_members.snap | Documents the API schema for the get_org_members tool |
| pkg/github/toolsnaps/list_outside_collaborators.snap | Documents the API schema for the list_outside_collaborators tool |
| README.md | Adds documentation for both new tools in the Context section |
fix: change method name capitalization to adjust export visibility
| org := params.Org | ||
| perPage := params.PerPage | ||
| page := params.Page |
There was a problem hiding this comment.
These intermediate variable assignments (lines 383-385) are unnecessary and reduce code clarity. The params struct fields (params.Org, params.PerPage, params.Page) can be used directly throughout the function, which is the pattern followed by other tools in the codebase (e.g., GetDiscussion and GetDiscussionComments in discussions.go).
| 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"` | ||
| } |
There was a problem hiding this comment.
The OutUser struct should use int64 for the ID field instead of string to maintain consistency with the existing MinimalUser struct in minimal_types.go. The existing pattern uses ID int64 \json:"id,omitempty"`throughout the codebase. The conversion logic inGetOrgMembers(line 344) andListOutsideCollaborators(line 423) should be updated fromfmt.Sprintf("%v", u.GetID())to justu.GetID()`.
| | ||
| 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")), |
There was a problem hiding this comment.
The tool description claims to return "login, id, avatar_url, type" but the implementation also returns site_admin. The description should be updated to include site_admin in the list of returned fields for accuracy.
| 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.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, site_admin. Limited to organizations accessible with current credentials")), |
| | ||
| 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).")), |
There was a problem hiding this comment.
The tool description should explicitly list all returned fields for consistency with other tools in the codebase. Consider adding "Returns a list of user objects with fields: login, id, avatar_url, type, site_admin" to match the pattern used in get_org_members and provide clarity about the response structure.
| 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.WithDescription(t("TOOL_LIST_OUTSIDE_COLLABORATORS_DESCRIPTION", "List all outside collaborators of an organization (users with access to organization repositories but not members). Returns a list of user objects with fields: login, id, avatar_url, type, site_admin.")), |
| 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(), | ||
| }) | ||
| } |
There was a problem hiding this comment.
The conversion logic from *github.User to OutUser is duplicated between GetOrgMembers (lines 340-349) and ListOutsideCollaborators (lines 419-428). Consider extracting this into a helper function (e.g., convertToOutUser) following the pattern used in minimal_types.go (see convertToMinimalUser) to improve maintainability and reduce code duplication.
| org := params.Org | ||
| role := params.Role | ||
| perPage := params.PerPage | ||
| page := params.Page |
There was a problem hiding this comment.
These intermediate variable assignments (lines 297-300) are unnecessary and reduce code clarity. The params struct fields (params.Org, params.Role, etc.) can be used directly throughout the function, which is the pattern followed by other tools in the codebase (e.g., GetDiscussion and GetDiscussionComments in discussions.go).
1 participant
This pull request adds two new tools for organization management in the GitHub integration, along with their documentation, implementation, and test coverage. The new tools enable fetching organization members and listing outside collaborators, both with support for pagination and filtering. The changes are grouped into tool additions, documentation updates, and test enhancements.
New organization management tools:
get_org_memberstool, which retrieves members of a specified organization, with support for pagination and filtering by role (all,admin,member). [1] [2]list_outside_collaboratorstool, which lists users who have access to organization repositories but are not organization members, also supporting pagination. [1] [2]Documentation updates:
README.mdto include usage details for the newget_org_membersandlist_outside_collaboratorstools, including their parameters and descriptions.Test coverage:
These changes improve the ability to manage and audit organization membership and access via the GitHub integration.