|
| 1 | +Use this file as a place to stash updated plans, knowledge, results or references so that even if you stop work you can continue without losing all context. Record decisions here too. |
| 2 | + |
| 3 | +--- |
| 4 | + |
| 5 | +##Progress & Decisions |
| 6 | + |
| 7 | +###Phase 1 - COMPLETED ✅ |
| 8 | + |
| 9 | +**Completed:** |
| 10 | +-[x] Read OAuth scopes documentation:https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/scopes-for-oauth-apps |
| 11 | +-[x] Created`pkg/scopes/scopes.go` with OAuth scope constants and hierarchy utilities |
| 12 | +-[x] Added scopes to ALL tool definitions using`mcp.Tool.Meta` field |
| 13 | +-[x] Updated`cmd/github-mcp-server/generate_docs.go` to include scopes in README |
| 14 | +-[x] Generated updated README.md with scope information |
| 15 | +-[x] Updated all toolsnaps to reflect Meta field with scopes |
| 16 | + |
| 17 | +**Key Decision - Where to Store Scopes:** |
| 18 | +- ❌ REJECTED: Adding`RequiredScopes` to`ServerTool` struct in toolsets package |
| 19 | +- Reason: Adds complexity to toolsets package, requires changes to how tools are registered |
| 20 | +- Creates tight coupling between toolsets and scopes packages |
| 21 | +- ✅ ACCEPTED: Use`mcp.Tool.Meta` field to store scopes directly on tool definitions |
| 22 | +- The MCP SDK's`Tool` struct has a`Meta map[string]any` field for custom metadata |
| 23 | +- Scopes should be defined where tools are defined (repositories.go, issues.go, etc.) |
| 24 | +- Cleaner separation of concerns - each tool knows its own requirements |
| 25 | +- Easier to maintain - scope info is next to the API calls that require them |
| 26 | + |
| 27 | +**OAuth Scope Mapping (from docs):** |
| 28 | +| API Area| Read Scope| Write Scope| |
| 29 | +|----------|------------|-------------| |
| 30 | +| Repos (public)| (no scope)|`public_repo`| |
| 31 | +| Repos (private)|`repo`|`repo`| |
| 32 | +| Issues|`repo`|`repo`| |
| 33 | +| Pull Requests|`repo`|`repo`| |
| 34 | +| Actions|`repo`|`repo`| |
| 35 | +| Notifications|`notifications`|`notifications`| |
| 36 | +| Gists (public)| (no scope)|`gist`| |
| 37 | +| Users (public)| (no scope)|`user`| |
| 38 | +| Organizations|`read:org`|`admin:org`| |
| 39 | +| Code Scanning|`security_events`|`security_events`| |
| 40 | +| Secret Scanning|`repo`|`repo`| |
| 41 | +| Projects|`read:project`|`project`| |
| 42 | +| Discussions|`repo`|`repo`| |
| 43 | + |
| 44 | +**Files Updated with Scopes (90+ tools):** |
| 45 | +-[x] pkg/github/repositories.go - repos tools (repo, public_repo scopes) |
| 46 | +-[x] pkg/github/git.go - git tools (repo scope) |
| 47 | +-[x] pkg/github/issues.go - issues tools (repo scope) |
| 48 | +-[x] pkg/github/pullrequests.go - PR tools (repo scope) |
| 49 | +-[x] pkg/github/actions.go - actions tools (repo scope) |
| 50 | +-[x] pkg/github/notifications.go - notifications tools (notifications scope) |
| 51 | +-[x] pkg/github/discussions.go - discussions tools (repo scope) |
| 52 | +-[x] pkg/github/gists.go - gists tools (gist scope for write) |
| 53 | +-[x] pkg/github/search.go - search tools (repo scope) |
| 54 | +-[x] pkg/github/code_scanning.go - code security (security_events scope) |
| 55 | +-[x] pkg/github/secret_scanning.go - secret protection (repo scope) |
| 56 | +-[x] pkg/github/dependabot.go - dependabot (repo scope) |
| 57 | +-[x] pkg/github/projects.go - projects (project/read:project scope) |
| 58 | +-[x] pkg/github/context_tools.go - context tools (no scope, read:org for teams) |
| 59 | +-[x] pkg/github/labels.go - labels (repo scope) |
| 60 | +-[x] pkg/github/security_advisories.go - security advisories (repo scope) |
| 61 | + |
| 62 | +**Package Structure:** |
| 63 | +-`pkg/scopes/scopes.go` - Scope constants, hierarchy map, helper functions |
| 64 | +-`Scope` type and constants (Repo, PublicRepo, Notifications, etc.) |
| 65 | +-`ScopeHierarchy` map showing which scopes include others |
| 66 | +-`WithScopes()` - creates Meta map for tool definitions |
| 67 | +-`GetScopesFromMeta()` - extracts scopes from tool Meta |
| 68 | +-`ScopeIncludes()`,`HasRequiredScopes()` - scope checking utilities |
| 69 | + |
| 70 | +**Documentation:** |
| 71 | +- README.md now shows`(scopes: \`repo\`)` after each tool description |
| 72 | +- Toolsnaps include`_meta.requiredOAuthScopes` array |
| 73 | + |
| 74 | +--- |
| 75 | + |
| 76 | +###Phase 2 - COMPLETED ✅ |
| 77 | + |
| 78 | +**Completed:** |
| 79 | +-[x] Read fine-grained permissions documentation:https://docs.github.com/en/rest/authentication/permissions-required-for-fine-grained-personal-access-tokens |
| 80 | +-[x] Extended`pkg/scopes/scopes.go` with fine-grained permission types and helpers: |
| 81 | +-`Permission` type with constants for all fine-grained permissions (actions, contents, issues, etc.) |
| 82 | +-`PermissionLevel` type (read, write, admin) |
| 83 | +-`FineGrainedPermission` struct combining permission and level |
| 84 | +-`WithScopesAndPermissions()` helper for tool metadata |
| 85 | +-`AddPermissions()` helper to add permissions to existing meta |
| 86 | +-`GetPermissionsFromMeta()` to extract permissions |
| 87 | +-`ReadPerm()`,`WritePerm()`,`AdminPerm()` convenience functions |
| 88 | +-[x] Added comprehensive tests for fine-grained permissions in`pkg/scopes/scopes_test.go` |
| 89 | +-[x] Created`docs/tool-permissions.md` with comprehensive documentation: |
| 90 | +- OAuth scope hierarchy table |
| 91 | +- Fine-grained permission levels explanation |
| 92 | +- Complete tool-by-category permission mapping tables |
| 93 | +- Minimum required scopes by use case |
| 94 | +- Notes about limitations (notifications, metadata, etc.) |
| 95 | +-[x] Updated README.md with links to tool-permissions.md: |
| 96 | +- Added link in Prerequisites section |
| 97 | +- Added callout note before Tools section |
| 98 | + |
| 99 | +**Key Decision - Documentation vs Code Changes:** |
| 100 | +- ✅ ACCEPTED: Create comprehensive documentation in`docs/tool-permissions.md` |
| 101 | +- Documents all~90 tools with both OAuth scopes AND fine-grained permissions |
| 102 | +- Easier to maintain as a single reference document |
| 103 | +- Doesn't require modifying every tool file |
| 104 | +- Users can look up permissions by tool or by category |
| 105 | +- ⏸️ DEFERRED: Adding fine-grained permissions to every tool's Meta field |
| 106 | +- Would require changes to~90 tool definitions |
| 107 | +- Phase 1 OAuth scopes are sufficient for tool metadata |
| 108 | +- Documentation approach provides same info with less risk |
| 109 | + |
| 110 | +--- |
| 111 | + |
| 112 | +###Phase 3 - COMPLETED ✅ |
| 113 | + |
| 114 | +**Completed:** |
| 115 | +-[x] Created`cmd/github-mcp-server/list_scopes.go` - new command to list required OAuth scopes |
| 116 | +-[x] Created`script/list-scopes` - convenience wrapper script |
| 117 | +-[x] Command respects all the same flags as stdio command (--toolsets, --read-only, etc.) |
| 118 | +-[x] Three output formats: text (default), json, summary |
| 119 | +-[x] JSON output includes: tools, unique_scopes, scopes_by_tool, tools_by_scope |
| 120 | +-[x] Lint passes, tests pass |
| 121 | + |
| 122 | +**Implementation Details:** |
| 123 | +- Added`list-scopes` subcommand to github-mcp-server binary |
| 124 | +- Uses same toolset configuration logic as stdio server |
| 125 | +- Creates toolset group with mock clients (no API calls needed) |
| 126 | +- Extracts scopes from tool Meta field using existing scopes package |
| 127 | +- Calculates accepted scopes (parent scopes that satisfy requirements) |
| 128 | + |
| 129 | +**Usage Examples:** |
| 130 | +```bash |
| 131 | +# List scopes for default toolsets |
| 132 | +github-mcp-server list-scopes |
| 133 | + |
| 134 | +# List scopes for specific toolsets |
| 135 | +github-mcp-server list-scopes --toolsets=repos,issues,pull_requests |
| 136 | + |
| 137 | +# List scopes for all toolsets |
| 138 | +github-mcp-server list-scopes --toolsets=all |
| 139 | + |
| 140 | +# Output as JSON (for programmatic use) |
| 141 | +github-mcp-server list-scopes --output=json |
| 142 | + |
| 143 | +# Just show unique scopes needed |
| 144 | +github-mcp-server list-scopes --output=summary |
| 145 | + |
| 146 | +# Read-only mode (excludes write tools) |
| 147 | +github-mcp-server list-scopes --read-only --output=summary |
| 148 | +``` |
| 149 | + |
| 150 | +--- |
| 151 | + |
| 152 | +###Phase 4 - COMPLETED ✅ |
| 153 | + |
| 154 | +**Completed:** |
| 155 | +-[x] Created`pkg/scopes/tool_scope_map.go` with exported types for library use |
| 156 | +-[x] Created`pkg/scopes/tool_scope_map_test.go` with comprehensive tests |
| 157 | +-[x] Lint passes, tests pass |
| 158 | + |
| 159 | +**Exported Types:** |
| 160 | +-`ToolScopeMap` - map[string]*ToolScopeInfo for fast tool name -> scopes lookup |
| 161 | +-`ToolScopeInfo` - contains RequiredScopes and AcceptedScopes as ScopeSet |
| 162 | +-`ScopeSet` - map[string]bool for O(1) scope lookup |
| 163 | +-`ToolMeta` - minimal struct for building scope maps |
| 164 | + |
| 165 | +**Key Methods:** |
| 166 | +-`NewToolScopeInfo(required []Scope)` - creates info from required scopes, auto-calculates accepted |
| 167 | +-`BuildToolScopeMapFromMeta(tools []ToolMeta)` - builds map from tool definitions |
| 168 | +-`GetToolScopeInfo(meta map[string]any)` - creates info from tool Meta field |
| 169 | +-`ToolScopeInfo.HasAcceptedScope(userScopes ...string)` - checks if token has access |
| 170 | +-`ToolScopeInfo.MissingScopes(userScopes ...string)` - returns missing required scopes |
| 171 | +-`ToolScopeMap.AllRequiredScopes()` - returns all unique required scopes |
| 172 | +-`ToolScopeMap.ToolsRequiringScope(scope)` - returns tools that require a scope |
| 173 | +-`ToolScopeMap.ToolsAcceptingScope(scope)` - returns tools that accept a scope |
| 174 | + |
| 175 | +**Usage Example:** |
| 176 | +```go |
| 177 | +import"github.com/github/github-mcp-server/pkg/scopes" |
| 178 | + |
| 179 | +// Build scope map from tool definitions |
| 180 | +tools:= []scopes.ToolMeta{ |
| 181 | + {Name:"get_repo", Meta: someToolMeta}, |
| 182 | + {Name:"create_issue", Meta: anotherToolMeta}, |
| 183 | +} |
| 184 | +scopeMap:= scopes.BuildToolScopeMapFromMeta(tools) |
| 185 | + |
| 186 | +// Check if user's token can use a tool |
| 187 | +ifinfo,ok:= scopeMap["create_issue"]; ok { |
| 188 | +userScopes:= []string{"repo","user"} |
| 189 | +if info.HasAcceptedScope(userScopes...) { |
| 190 | +// User can use this tool |
| 191 | + }else { |
| 192 | +missing:= info.MissingScopes(userScopes...) |
| 193 | + fmt.Printf("Missing scopes:%v\n", missing) |
| 194 | + } |
| 195 | +} |
| 196 | + |
| 197 | +// Get all required scopes |
| 198 | +allRequired:= scopeMap.AllRequiredScopes() |
| 199 | +fmt.Printf("All required:%v\n", allRequired.ToSlice()) |
| 200 | +``` |
| 201 | + |
| 202 | +--- |
| 203 | + |
| 204 | +##Original Requirements |
| 205 | + |
| 206 | +The phases can be a stacked PR, so each one should have a new branch, and when a phase is complete we want a full pull request based on the previous, with the base branch of phase 1 being omgitsads/go-sdk |
| 207 | + |
| 208 | +IMPORTANT you MUST check all API calls and GraphQL calls etc. and verify what scopes are required. If unsure you must check before proceeding. |
| 209 | + |
| 210 | +These changes must be clean and clear. |