Skip to content

Commit 3812578

Browse files
CopilotSamMorrowDrums
Copilot
and
SamMorrowDrums
committed
Simplify scope implementation per feedback
- Replace NewToolWithScopes with updated NewTool that includes scope parameters - All tools now use the same constructor with explicit scope requirements - Documentation only shows accepted scopes when they differ from required - Removed NewToolWithScopes and NewToolFromHandlerWithScopes helper functions Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
1 parent f543aa4 commit 3812578

19 files changed

+131
-197
lines changed

‎README.md‎

Lines changed: 0 additions & 74 deletions
Large diffs are not rendered by default.

‎cmd/github-mcp-server/generate_docs.go‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,11 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
227227
fmt.Fprintf(buf, "- **%s** - %s\n", tool.Tool.Name, tool.Tool.Annotations.Title)
228228

229229
// OAuth scopes if present
230-
if len(tool.RequiredScopes) > 0 || len(tool.AcceptedScopes) > 0 {
231-
if len(tool.RequiredScopes) > 0 {
232-
fmt.Fprintf(buf, " - **Required OAuth Scopes**: `%s`\n", strings.Join(tool.RequiredScopes, "`, `"))
233-
}
234-
if len(tool.AcceptedScopes) > 0 {
230+
if len(tool.RequiredScopes) > 0 {
231+
fmt.Fprintf(buf, " - **Required OAuth Scopes**: `%s`\n", strings.Join(tool.RequiredScopes, "`, `"))
232+
233+
// Only show accepted scopes if they differ from required scopes
234+
if len(tool.AcceptedScopes) > 0 && !scopesEqual(tool.RequiredScopes, tool.AcceptedScopes) {
235235
fmt.Fprintf(buf, " - **Accepted OAuth Scopes**: `%s`\n", strings.Join(tool.AcceptedScopes, "`, `"))
236236
}
237237
}
@@ -290,6 +290,28 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
290290
}
291291
}
292292

293+
// scopesEqual checks if two scope slices contain the same elements (order-independent)
294+
func scopesEqual(a, b []string) bool {
295+
if len(a) != len(b) {
296+
return false
297+
}
298+
299+
// Create a map for quick lookup
300+
aMap := make(map[string]bool, len(a))
301+
for _, scope := range a {
302+
aMap[scope] = true
303+
}
304+
305+
// Check if all elements in b are in a
306+
for _, scope := range b {
307+
if !aMap[scope] {
308+
return false
309+
}
310+
}
311+
312+
return true
313+
}
314+
293315
func contains(slice []string, item string) bool {
294316
for _, s := range slice {
295317
if s == item {

‎pkg/github/actions.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ const (
5151

5252
// ListWorkflows creates a tool to list workflows in a repository
5353
func ListWorkflows(t translations.TranslationHelperFunc) inventory.ServerTool {
54-
tool := NewToolWithScopes(
54+
tool := NewTool(
5555
ToolsetMetadataActions,
5656
mcp.Tool{
5757
Name: "list_workflows",
@@ -124,7 +124,7 @@ func ListWorkflows(t translations.TranslationHelperFunc) inventory.ServerTool {
124124

125125
// ListWorkflowRuns creates a tool to list workflow runs for a specific workflow
126126
func ListWorkflowRuns(t translations.TranslationHelperFunc) inventory.ServerTool {
127-
tool := NewToolWithScopes(
127+
tool := NewTool(
128128
ToolsetMetadataActions,
129129
mcp.Tool{
130130
Name: "list_workflow_runs",
@@ -280,7 +280,7 @@ func ListWorkflowRuns(t translations.TranslationHelperFunc) inventory.ServerTool
280280

281281
// RunWorkflow creates a tool to run an Actions workflow
282282
func RunWorkflow(t translations.TranslationHelperFunc) inventory.ServerTool {
283-
tool := NewToolWithScopes(
283+
tool := NewTool(
284284
ToolsetMetadataActions,
285285
mcp.Tool{
286286
Name: "run_workflow",
@@ -394,7 +394,7 @@ func RunWorkflow(t translations.TranslationHelperFunc) inventory.ServerTool {
394394

395395
// GetWorkflowRun creates a tool to get details of a specific workflow run
396396
func GetWorkflowRun(t translations.TranslationHelperFunc) inventory.ServerTool {
397-
tool := NewToolWithScopes(
397+
tool := NewTool(
398398
ToolsetMetadataActions,
399399
mcp.Tool{
400400
Name: "get_workflow_run",
@@ -464,7 +464,7 @@ func GetWorkflowRun(t translations.TranslationHelperFunc) inventory.ServerTool {
464464

465465
// GetWorkflowRunLogs creates a tool to download logs for a specific workflow run
466466
func GetWorkflowRunLogs(t translations.TranslationHelperFunc) inventory.ServerTool {
467-
tool := NewToolWithScopes(
467+
tool := NewTool(
468468
ToolsetMetadataActions,
469469
mcp.Tool{
470470
Name: "get_workflow_run_logs",
@@ -544,7 +544,7 @@ func GetWorkflowRunLogs(t translations.TranslationHelperFunc) inventory.ServerTo
544544

545545
// ListWorkflowJobs creates a tool to list jobs for a specific workflow run
546546
func ListWorkflowJobs(t translations.TranslationHelperFunc) inventory.ServerTool {
547-
tool := NewToolWithScopes(
547+
tool := NewTool(
548548
ToolsetMetadataActions,
549549
mcp.Tool{
550550
Name: "list_workflow_jobs",
@@ -646,7 +646,7 @@ func ListWorkflowJobs(t translations.TranslationHelperFunc) inventory.ServerTool
646646

647647
// GetJobLogs creates a tool to download logs for a specific workflow job or efficiently get all failed job logs for a workflow run
648648
func GetJobLogs(t translations.TranslationHelperFunc) inventory.ServerTool {
649-
tool := NewToolWithScopes(
649+
tool := NewTool(
650650
ToolsetMetadataActions,
651651
mcp.Tool{
652652
Name: "get_job_logs",
@@ -913,7 +913,7 @@ func downloadLogContent(ctx context.Context, logURL string, tailLines int, maxLi
913913

914914
// RerunWorkflowRun creates a tool to re-run an entire workflow run
915915
func RerunWorkflowRun(t translations.TranslationHelperFunc) inventory.ServerTool {
916-
tool := NewToolWithScopes(
916+
tool := NewTool(
917917
ToolsetMetadataActions,
918918
mcp.Tool{
919919
Name: "rerun_workflow_run",
@@ -990,7 +990,7 @@ func RerunWorkflowRun(t translations.TranslationHelperFunc) inventory.ServerTool
990990

991991
// RerunFailedJobs creates a tool to re-run only the failed jobs in a workflow run
992992
func RerunFailedJobs(t translations.TranslationHelperFunc) inventory.ServerTool {
993-
tool := NewToolWithScopes(
993+
tool := NewTool(
994994
ToolsetMetadataActions,
995995
mcp.Tool{
996996
Name: "rerun_failed_jobs",
@@ -1067,7 +1067,7 @@ func RerunFailedJobs(t translations.TranslationHelperFunc) inventory.ServerTool
10671067

10681068
// CancelWorkflowRun creates a tool to cancel a workflow run
10691069
func CancelWorkflowRun(t translations.TranslationHelperFunc) inventory.ServerTool {
1070-
tool := NewToolWithScopes(
1070+
tool := NewTool(
10711071
ToolsetMetadataActions,
10721072
mcp.Tool{
10731073
Name: "cancel_workflow_run",
@@ -1146,7 +1146,7 @@ func CancelWorkflowRun(t translations.TranslationHelperFunc) inventory.ServerToo
11461146

11471147
// ListWorkflowRunArtifacts creates a tool to list artifacts for a workflow run
11481148
func ListWorkflowRunArtifacts(t translations.TranslationHelperFunc) inventory.ServerTool {
1149-
tool := NewToolWithScopes(
1149+
tool := NewTool(
11501150
ToolsetMetadataActions,
11511151
mcp.Tool{
11521152
Name: "list_workflow_run_artifacts",
@@ -1228,7 +1228,7 @@ func ListWorkflowRunArtifacts(t translations.TranslationHelperFunc) inventory.Se
12281228

12291229
// DownloadWorkflowRunArtifact creates a tool to download a workflow run artifact
12301230
func DownloadWorkflowRunArtifact(t translations.TranslationHelperFunc) inventory.ServerTool {
1231-
tool := NewToolWithScopes(
1231+
tool := NewTool(
12321232
ToolsetMetadataActions,
12331233
mcp.Tool{
12341234
Name: "download_workflow_run_artifact",
@@ -1307,7 +1307,7 @@ func DownloadWorkflowRunArtifact(t translations.TranslationHelperFunc) inventory
13071307

13081308
// DeleteWorkflowRunLogs creates a tool to delete logs for a workflow run
13091309
func DeleteWorkflowRunLogs(t translations.TranslationHelperFunc) inventory.ServerTool {
1310-
tool := NewToolWithScopes(
1310+
tool := NewTool(
13111311
ToolsetMetadataActions,
13121312
mcp.Tool{
13131313
Name: "delete_workflow_run_logs",
@@ -1385,7 +1385,7 @@ func DeleteWorkflowRunLogs(t translations.TranslationHelperFunc) inventory.Serve
13851385

13861386
// GetWorkflowRunUsage creates a tool to get usage metrics for a workflow run
13871387
func GetWorkflowRunUsage(t translations.TranslationHelperFunc) inventory.ServerTool {
1388-
tool := NewToolWithScopes(
1388+
tool := NewTool(
13891389
ToolsetMetadataActions,
13901390
mcp.Tool{
13911391
Name: "get_workflow_run_usage",
@@ -1455,7 +1455,7 @@ func GetWorkflowRunUsage(t translations.TranslationHelperFunc) inventory.ServerT
14551455

14561456
// ActionsList returns the tool and handler for listing GitHub Actions resources.
14571457
func ActionsList(t translations.TranslationHelperFunc) inventory.ServerTool {
1458-
tool := NewToolWithScopes(
1458+
tool := NewTool(
14591459
ToolsetMetadataActions,
14601460
mcp.Tool{
14611461
Name: "actions_list",
@@ -1652,7 +1652,7 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an
16521652

16531653
// ActionsGet returns the tool and handler for getting GitHub Actions resources.
16541654
func ActionsGet(t translations.TranslationHelperFunc) inventory.ServerTool {
1655-
tool := NewToolWithScopes(
1655+
tool := NewTool(
16561656
ToolsetMetadataActions,
16571657
mcp.Tool{
16581658
Name: "actions_get",
@@ -1762,7 +1762,7 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an
17621762

17631763
// ActionsRunTrigger returns the tool and handler for triggering GitHub Actions workflows.
17641764
func ActionsRunTrigger(t translations.TranslationHelperFunc) inventory.ServerTool {
1765-
tool := NewToolWithScopes(
1765+
tool := NewTool(
17661766
ToolsetMetadataActions,
17671767
mcp.Tool{
17681768
Name: "actions_run_trigger",
@@ -1882,7 +1882,7 @@ func ActionsRunTrigger(t translations.TranslationHelperFunc) inventory.ServerToo
18821882

18831883
// ActionsGetJobLogs returns the tool and handler for getting workflow job logs.
18841884
func ActionsGetJobLogs(t translations.TranslationHelperFunc) inventory.ServerTool {
1885-
tool := NewToolWithScopes(
1885+
tool := NewTool(
18861886
ToolsetMetadataActions,
18871887
mcp.Tool{
18881888
Name: "get_job_logs",

‎pkg/github/code_scanning.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717
)
1818

1919
func GetCodeScanningAlert(t translations.TranslationHelperFunc) inventory.ServerTool {
20-
return NewToolWithScopes(
20+
return NewTool(
2121
ToolsetMetadataCodeSecurity,
2222
mcp.Tool{
2323
Name: "get_code_scanning_alert",
@@ -95,7 +95,7 @@ func GetCodeScanningAlert(t translations.TranslationHelperFunc) inventory.Server
9595
}
9696

9797
func ListCodeScanningAlerts(t translations.TranslationHelperFunc) inventory.ServerTool {
98-
return NewToolWithScopes(
98+
return NewTool(
9999
ToolsetMetadataCodeSecurity,
100100
mcp.Tool{
101101
Name: "list_code_scanning_alerts",

‎pkg/github/context_tools.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ type UserDetails struct {
3939

4040
// GetMe creates a tool to get details of the authenticated user.
4141
func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool {
42-
return NewToolWithScopes(
42+
return NewTool(
4343
ToolsetMetadataContext,
4444
mcp.Tool{
4545
Name: "get_me",
@@ -113,7 +113,7 @@ type OrganizationTeams struct {
113113
}
114114

115115
func GetTeams(t translations.TranslationHelperFunc) inventory.ServerTool {
116-
return NewToolWithScopes(
116+
return NewTool(
117117
ToolsetMetadataContext,
118118
mcp.Tool{
119119
Name: "get_teams",
@@ -212,7 +212,7 @@ func GetTeams(t translations.TranslationHelperFunc) inventory.ServerTool {
212212
}
213213

214214
func GetTeamMembers(t translations.TranslationHelperFunc) inventory.ServerTool {
215-
return NewToolWithScopes(
215+
return NewTool(
216216
ToolsetMetadataContext,
217217
mcp.Tool{
218218
Name: "get_team_members",

‎pkg/github/dependabot.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import (
1818
)
1919

2020
func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTool {
21-
return NewToolWithScopes(
21+
return NewTool(
2222
ToolsetMetadataDependabot,
2323
mcp.Tool{
2424
Name: "get_dependabot_alert",
@@ -96,7 +96,7 @@ func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTo
9696
}
9797

9898
func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.ServerTool {
99-
return NewToolWithScopes(
99+
return NewTool(
100100
ToolsetMetadataDependabot,
101101
mcp.Tool{
102102
Name: "list_dependabot_alerts",

‎pkg/github/dependencies.go‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -148,16 +148,9 @@ func (d BaseDeps) GetContentWindowSize() int { return d.ContentWindowSize }
148148
//
149149
// The handler function receives deps extracted from context via MustDepsFromContext.
150150
// Ensure ContextWithDeps is called to inject deps before any tool handlers are invoked.
151-
func NewTool[In, Out any](toolset inventory.ToolsetMetadata, tool mcp.Tool, handler func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args In) (*mcp.CallToolResult, Out, error)) inventory.ServerTool {
152-
return inventory.NewServerToolWithContextHandler(tool, toolset, func(ctx context.Context, req *mcp.CallToolRequest, args In) (*mcp.CallToolResult, Out, error) {
153-
deps := MustDepsFromContext(ctx)
154-
return handler(ctx, deps, req, args)
155-
})
156-
}
157-
158-
// NewToolWithScopes creates a ServerTool with OAuth scope requirements.
159-
// This is like NewTool but also accepts required and accepted scopes.
160-
func NewToolWithScopes[In, Out any](
151+
//
152+
// All tools must explicitly specify their OAuth scope requirements, even if empty (nil).
153+
func NewTool[In, Out any](
161154
toolset inventory.ToolsetMetadata,
162155
tool mcp.Tool,
163156
requiredScopes []string,
@@ -178,16 +171,9 @@ func NewToolWithScopes[In, Out any](
178171
//
179172
// The handler function receives deps extracted from context via MustDepsFromContext.
180173
// Ensure ContextWithDeps is called to inject deps before any tool handlers are invoked.
181-
func NewToolFromHandler(toolset inventory.ToolsetMetadata, tool mcp.Tool, handler func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest) (*mcp.CallToolResult, error)) inventory.ServerTool {
182-
return inventory.NewServerToolWithRawContextHandler(tool, toolset, func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
183-
deps := MustDepsFromContext(ctx)
184-
return handler(ctx, deps, req)
185-
})
186-
}
187-
188-
// NewToolFromHandlerWithScopes creates a ServerTool with OAuth scope requirements.
189-
// This is like NewToolFromHandler but also accepts required and accepted scopes.
190-
func NewToolFromHandlerWithScopes(
174+
//
175+
// All tools must explicitly specify their OAuth scope requirements, even if empty (nil).
176+
func NewToolFromHandler(
191177
toolset inventory.ToolsetMetadata,
192178
tool mcp.Tool,
193179
requiredScopes []string,

‎pkg/github/discussions.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func getQueryType(useOrdering bool, categoryID *githubv4.ID) any {
124124
}
125125

126126
func ListDiscussions(t translations.TranslationHelperFunc) inventory.ServerTool {
127-
return NewToolWithScopes(
127+
return NewTool(
128128
ToolsetMetadataDiscussions,
129129
mcp.Tool{
130130
Name: "list_discussions",
@@ -278,7 +278,7 @@ func ListDiscussions(t translations.TranslationHelperFunc) inventory.ServerTool
278278
}
279279

280280
func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
281-
return NewToolWithScopes(
281+
return NewTool(
282282
ToolsetMetadataDiscussions,
283283
mcp.Tool{
284284
Name: "get_discussion",
@@ -383,7 +383,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
383383
}
384384

385385
func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.ServerTool {
386-
return NewToolWithScopes(
386+
return NewTool(
387387
ToolsetMetadataDiscussions,
388388
mcp.Tool{
389389
Name: "get_discussion_comments",
@@ -511,7 +511,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
511511
}
512512

513513
func ListDiscussionCategories(t translations.TranslationHelperFunc) inventory.ServerTool {
514-
return NewToolWithScopes(
514+
return NewTool(
515515
ToolsetMetadataDiscussions,
516516
mcp.Tool{
517517
Name: "list_discussion_categories",

‎pkg/github/gists.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919

2020
// ListGists creates a tool to list gists for a user
2121
func ListGists(t translations.TranslationHelperFunc) inventory.ServerTool {
22-
return NewToolWithScopes(
22+
return NewTool(
2323
ToolsetMetadataGists,
2424
mcp.Tool{
2525
Name: "list_gists",
@@ -107,7 +107,7 @@ func ListGists(t translations.TranslationHelperFunc) inventory.ServerTool {
107107

108108
// GetGist creates a tool to get the content of a gist
109109
func GetGist(t translations.TranslationHelperFunc) inventory.ServerTool {
110-
return NewToolWithScopes(
110+
return NewTool(
111111
ToolsetMetadataGists,
112112
mcp.Tool{
113113
Name: "get_gist",
@@ -166,7 +166,7 @@ func GetGist(t translations.TranslationHelperFunc) inventory.ServerTool {
166166

167167
// CreateGist creates a tool to create a new gist
168168
func CreateGist(t translations.TranslationHelperFunc) inventory.ServerTool {
169-
return NewToolWithScopes(
169+
return NewTool(
170170
ToolsetMetadataGists,
171171
mcp.Tool{
172172
Name: "create_gist",
@@ -270,7 +270,7 @@ func CreateGist(t translations.TranslationHelperFunc) inventory.ServerTool {
270270

271271
// UpdateGist creates a tool to edit an existing gist
272272
func UpdateGist(t translations.TranslationHelperFunc) inventory.ServerTool {
273-
return NewToolWithScopes(
273+
return NewTool(
274274
ToolsetMetadataGists,
275275
mcp.Tool{
276276
Name: "update_gist",

‎pkg/github/git.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ type TreeResponse struct {
4040

4141
// GetRepositoryTree creates a tool to get the tree structure of a GitHub repository.
4242
func GetRepositoryTree(t translations.TranslationHelperFunc) inventory.ServerTool {
43-
return NewToolWithScopes(
43+
return NewTool(
4444
ToolsetMetadataGit,
4545
mcp.Tool{
4646
Name: "get_repository_tree",

0 commit comments

Comments
 (0)