feat: Add OAuth scopes to tool metadata (Phase 1) #1485
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add pkg/scopes package with OAuth scope constants and utilities - Add scopes to all ~90 tool definitions using mcp.Tool.Meta field - Update generate_docs.go to include scopes in README output - Add comprehensive test coverage for scopes package - Update all toolsnaps to include _meta.requiredOAuthScopes Scope assignments follow GitHub OAuth app documentation: - repo: Most repository operations (private repos) - public_repo: Star/unstar operations - notifications: Notification operations - gist: Gist write operations - security_events: Code scanning, dependabot, secret scanning - project/read:project: Project operations - read:org: Organization/team read operations - No scope: Public read operations (get_me, list_gists)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements Phase 1 of OAuth scope support by adding scope metadata to all MCP tools, enabling clients to determine required permissions before making tool calls.
Key Changes:
- New
pkg/scopespackage with OAuth scope constants, hierarchy definitions, and utility functions for scope validation - Added
Meta: scopes.WithScopes(...)to ~90 MCP tools across the codebase with appropriate scope assignments - Updated documentation generation to include scope information in README
- All toolsnaps updated with
_meta.requiredOAuthScopesfield
Reviewed changes
Copilot reviewed 113 out of 113 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/scopes/scopes.go | New package defining OAuth scopes, hierarchy, and utility functions (MetaKey, WithScopes, GetScopesFromMeta, ScopeIncludes, HasRequiredScopes, GetAcceptedScopes) |
| pkg/scopes/scopes_test.go | Comprehensive test coverage for all scope utilities including hierarchy checks and metadata handling |
| pkg/github/*.go (multiple files) | Added scope metadata to tools - repo for most operations, security_events for security features, notifications for notification tools, gist for gist writes, read:org/project for reads, NoScope for public access |
| pkg/github/toolsnaps/*.snap (many files) | Added _meta.requiredOAuthScopes arrays to all tool snapshots |
| cmd/github-mcp-server/generate_docs.go | Enhanced doc generation to append scope information (e.g., "(scopes: `repo`)") to tool descriptions |
| README.md | Updated with scope annotations for all tools in documentation |
Comment on lines
+153
to
+176
| // GetAcceptedScopes returns all scopes that satisfy the given required scope. | ||
| // This includes the scope itself plus any parent scopes that include it. | ||
| func GetAcceptedScopes(required Scope) []Scope { | ||
| accepted := []Scope{required} | ||
|
|
||
| // Find all scopes that include the required scope | ||
| for parent, children := range ScopeHierarchy { | ||
| for _, child := range children { | ||
| if child == required { | ||
| accepted = append(accepted, parent) | ||
| // Also check if this parent is included by another scope | ||
| for grandparent, grandchildren := range ScopeHierarchy { | ||
| for _, gc := range grandchildren { | ||
| if gc == parent { | ||
| accepted = append(accepted, grandparent) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return accepted | ||
| } |
There was a problem hiding this comment.
[nitpick] The GetAcceptedScopes function only handles up to 2 levels of hierarchy (parent and grandparent) using nested loops, while ScopeIncludes properly handles arbitrary depth using recursion. Although this works for the current GitHub OAuth scope hierarchy (which has at most 3 levels), consider refactoring to use recursion for consistency and future-proofing:
func GetAcceptedScopes(required Scope) []Scope {
accepted := []Scope{required}
seen := make(map[Scope]bool)
seen[required] = true
// Recursively find all parent scopes
var findParents func(Scope)
findParents = func(child Scope) {
for parent, children := range ScopeHierarchy {
for _, c := range children {
if c == child && !seen[parent] {
seen[parent] = true
accepted = append(accepted, parent)
findParents(parent) // Recursively find parents of this parent
}
}
}
}
findParents(required)
return accepted
}This approach would handle hierarchies of any depth and avoid potential issues if the scope hierarchy is extended in the future.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds OAuth scope metadata to all MCP tools, enabling clients to know which scopes are required for each tool before calling them.
Changes
New Package:
pkg/scopesscopes.go: OAuth scope constants based on GitHub's OAuth app scopes documentationScopetype with constants for all GitHub OAuth scopes (Repo,PublicRepo,Notifications,Gist,SecurityEvents,Project,ReadProject,ReadOrg, etc.)ScopeHierarchymap defining parent-child relationships (e.g.,repoincludespublic_repo,security_events, etc.)WithScopes()helper to createmcp.Tool.MetamapsGetScopesFromMeta()to extract scopes from tool metadataScopeIncludes(),HasRequiredScopes()for scope checkingscopes_test.go: Comprehensive test coverage for all utilitiesTool Updates (~90 tools)
Added
Meta: scopes.WithScopes(...)to all tool definitions:repopublic_repostar_repository,unstar_repositorynotificationsgistcreate_gist,update_gistsecurity_eventsprojectread:projectread:orgget_teams,get_team_members,list_issue_typesget_me,list_gists,get_gist(public reads)Documentation Updates
generate_docs.go: Updated to include scope information in README output(scopes: \repo`)` after each tool description_meta.requiredOAuthScopesarrayTesting
pkg/scopes/scopes_test.gowith tests for:TestScopeString- Scope string conversionTestScopeIncludes- Hierarchy checkingTestHasRequiredScopes- Multiple scope validationTestWithScopes- Meta map creationTestGetScopesFromMeta- Meta extractionTestGetAcceptedScopes- Parent scope lookupTestScopeStringsAndParseScopes- Round-trip conversionPart of OAuth Scopes Work (Phase 1 of 4)