|
| 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 | +if info, 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. |
0 commit comments