Skip to content

Commit 7fbfa8c

Browse files
committed
fix(tools): list supported methods in unknown-method dispatch errors
- Add methodEnum/unknownMethodError helpers in params.go - Use shared method slices for schema enums and runtime validation - Update actions, issues, pull_requests, projects, and ui_get tools - Require method param in pull_request_review_write before WeakDecode - Add regression tests for helper and pull_request_review_write errors Fixes #2712
1 parent 9430064 commit 7fbfa8c

11 files changed

Lines changed: 132 additions & 59 deletions

File tree

pkg/github/__toolsnaps__/sub_issue_write.snap

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@
1919
},
2020
"method": {
2121
"description": "The action to perform on a single sub-issue\nOptions are:\n- 'add' - add a sub-issue to a parent issue in a GitHub repository.\n- 'remove' - remove a sub-issue from a parent issue in a GitHub repository.\n- 'reprioritize' - change the order of sub-issues within a parent issue in a GitHub repository. Use either 'after_id' or 'before_id' to specify the new position.\n\t\t\t\t",
22+
"enum": [
23+
"add",
24+
"remove",
25+
"reprioritize"
26+
],
2227
"type": "string"
2328
},
2429
"owner": {

pkg/github/actions.go

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ const (
4646
actionsMethodDeleteWorkflowRunLogs = "delete_workflow_run_logs"
4747
)
4848

49+
// Method sets for the consolidated actions tools. Each slice is the single
50+
// source for its tool: it feeds both the schema enum (methodEnum) and the
51+
// unknown-method error (unknownMethodError). Order matches the advertised enum.
52+
var (
53+
actionsWorkflowListMethods = []string{actionsMethodListWorkflows, actionsMethodListWorkflowRuns, actionsMethodListWorkflowJobs, actionsMethodListWorkflowArtifacts}
54+
actionsWorkflowGetMethods = []string{actionsMethodGetWorkflow, actionsMethodGetWorkflowRun, actionsMethodGetWorkflowJob, actionsMethodDownloadWorkflowArtifact, actionsMethodGetWorkflowRunUsage, actionsMethodGetWorkflowRunLogsURL}
55+
actionsWorkflowRunMethods = []string{actionsMethodRunWorkflow, actionsMethodRerunWorkflowRun, actionsMethodRerunFailedJobs, actionsMethodCancelWorkflowRun, actionsMethodDeleteWorkflowRunLogs}
56+
)
57+
4958
// handleFailedJobLogs gets logs for all failed jobs in a workflow run
5059
func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo string, runID int64, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, any, error) {
5160
// First, get all jobs for the workflow run
@@ -217,12 +226,7 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an
217226
"method": {
218227
Type: "string",
219228
Description: "The action to perform",
220-
Enum: []any{
221-
actionsMethodListWorkflows,
222-
actionsMethodListWorkflowRuns,
223-
actionsMethodListWorkflowJobs,
224-
actionsMethodListWorkflowArtifacts,
225-
},
229+
Enum: methodEnum(actionsWorkflowListMethods),
226230
},
227231
"owner": {
228232
Type: "string",
@@ -397,7 +401,7 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an
397401
result, payload, err := listWorkflowArtifacts(ctx, client, owner, repo, resourceIDInt, pagination)
398402
return attachIFC(result), payload, err
399403
default:
400-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
404+
return unknownMethodError(method, actionsWorkflowListMethods), nil, nil
401405
}
402406
},
403407
)
@@ -423,14 +427,7 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an
423427
"method": {
424428
Type: "string",
425429
Description: "The method to execute",
426-
Enum: []any{
427-
actionsMethodGetWorkflow,
428-
actionsMethodGetWorkflowRun,
429-
actionsMethodGetWorkflowJob,
430-
actionsMethodDownloadWorkflowArtifact,
431-
actionsMethodGetWorkflowRunUsage,
432-
actionsMethodGetWorkflowRunLogsURL,
433-
},
430+
Enum: methodEnum(actionsWorkflowGetMethods),
434431
},
435432
"owner": {
436433
Type: "string",
@@ -519,7 +516,7 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an
519516
result, payload, err := getWorkflowRunLogsURL(ctx, client, owner, repo, resourceIDInt)
520517
return attachIFC(result), payload, err
521518
default:
522-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
519+
return unknownMethodError(method, actionsWorkflowGetMethods), nil, nil
523520
}
524521
},
525522
)
@@ -544,13 +541,7 @@ func ActionsRunTrigger(t translations.TranslationHelperFunc) inventory.ServerToo
544541
"method": {
545542
Type: "string",
546543
Description: "The method to execute",
547-
Enum: []any{
548-
actionsMethodRunWorkflow,
549-
actionsMethodRerunWorkflowRun,
550-
actionsMethodRerunFailedJobs,
551-
actionsMethodCancelWorkflowRun,
552-
actionsMethodDeleteWorkflowRunLogs,
553-
},
544+
Enum: methodEnum(actionsWorkflowRunMethods),
554545
},
555546
"owner": {
556547
Type: "string",
@@ -636,7 +627,7 @@ func ActionsRunTrigger(t translations.TranslationHelperFunc) inventory.ServerToo
636627
case actionsMethodDeleteWorkflowRunLogs:
637628
return deleteWorkflowRunLogs(ctx, client, owner, repo, int64(runID))
638629
default:
639-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
630+
return unknownMethodError(method, actionsWorkflowRunMethods), nil, nil
640631
}
641632
},
642633
)

pkg/github/issues.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ import (
2424
"github.com/shurcooL/githubv4"
2525
)
2626

27+
// Method sets for the consolidated issue tools (single source for the schema
28+
// enum via methodEnum and the unknown-method error via unknownMethodError).
29+
var (
30+
issueReadMethods = []string{"get", "get_comments", "get_sub_issues", "get_parent", "get_labels"}
31+
subIssueWriteMethods = []string{"add", "remove", "reprioritize"}
32+
)
33+
2734
// CloseIssueInput represents the input for closing an issue via the GraphQL API.
2835
// Used to extend the functionality of the githubv4 library to support closing issues as duplicates.
2936
type CloseIssueInput struct {
@@ -620,7 +627,7 @@ Options are:
620627
4. get_parent - Get the parent issue, if this issue is a sub-issue of another.
621628
5. get_labels - Get labels assigned to the issue.
622629
`,
623-
Enum: []any{"get", "get_comments", "get_sub_issues", "get_parent", "get_labels"},
630+
Enum: methodEnum(issueReadMethods),
624631
},
625632
"owner": {
626633
Type: "string",
@@ -707,7 +714,7 @@ Options are:
707714
result, err := GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber)
708715
return attachIFC(result), nil, err
709716
default:
710-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
717+
return unknownMethodError(method, issueReadMethods), nil, nil
711718
}
712719
})
713720
}
@@ -1206,6 +1213,7 @@ func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
12061213
Properties: map[string]*jsonschema.Schema{
12071214
"method": {
12081215
Type: "string",
1216+
Enum: methodEnum(subIssueWriteMethods),
12091217
Description: `The action to perform on a single sub-issue
12101218
Options are:
12111219
- 'add' - add a sub-issue to a parent issue in a GitHub repository.
@@ -1299,7 +1307,7 @@ Options are:
12991307
result, err := ReprioritizeSubIssue(ctx, client, owner, repo, issueNumber, subIssueID, afterID, beforeID)
13001308
return result, nil, err
13011309
default:
1302-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
1310+
return unknownMethodError(method, subIssueWriteMethods), nil, nil
13031311
}
13041312
})
13051313
st.FeatureFlagDisable = []string{FeatureFlagIssuesGranular}

pkg/github/params.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@ import (
55
"fmt"
66
"math"
77
"strconv"
8+
"strings"
89

10+
"github.com/github/github-mcp-server/pkg/utils"
911
"github.com/google/go-github/v87/github"
1012
"github.com/google/jsonschema-go/jsonschema"
13+
"github.com/modelcontextprotocol/go-sdk/mcp"
1114
)
1215

1316
// OptionalParamOK is a helper function that can be used to fetch a requested parameter from the request.
@@ -488,3 +491,24 @@ func (p PaginationParams) ToGraphQLParams() (*GraphQLPaginationParams, error) {
488491
}
489492
return cursor.ToGraphQLParams()
490493
}
494+
495+
496+
// methodEnum renders a tool's supported method list as the []any value the
497+
// input-schema Enum field expects. Declaring the enum from the same []string
498+
// the handler validates against keeps the advertised methods and the runtime
499+
// check from drifting apart.
500+
func methodEnum(methods []string) []any {
501+
out := make([]any, len(methods))
502+
for i, m := range methods {
503+
out[i] = m
504+
}
505+
return out
506+
}
507+
508+
// unknownMethodError is the tool-result error returned when a method-dispatch
509+
// tool is called with a value outside its advertised method enum. Listing the
510+
// supported methods lets the caller (often an LLM) self-correct. supported must
511+
// be the same list advertised in the tool's input-schema enum.
512+
func unknownMethodError(method string, supported []string) *mcp.CallToolResult {
513+
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s. Supported methods are: %s", method, strings.Join(supported, ", ")))
514+
}

pkg/github/params_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,3 +642,18 @@ func TestOptionalPaginationParams(t *testing.T) {
642642
})
643643
}
644644
}
645+
646+
647+
func TestMethodEnum(t *testing.T) {
648+
assert.Equal(t, []any{"create", "submit_pending"}, methodEnum([]string{"create", "submit_pending"}))
649+
assert.Empty(t, methodEnum(nil))
650+
}
651+
652+
func TestUnknownMethodError(t *testing.T) {
653+
res := unknownMethodError("bogus", []string{"create", "submit_pending", "delete_pending"})
654+
assert.NotNil(t, res)
655+
assert.True(t, res.IsError)
656+
txt := getErrorResult(t, res).Text
657+
assert.Contains(t, txt, "unknown method: bogus")
658+
assert.Contains(t, txt, "Supported methods are: create, submit_pending, delete_pending")
659+
}

pkg/github/projects.go

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,15 @@ const (
5151
projectsMethodCreateIterationField = "create_iteration_field"
5252
)
5353

54+
// Method sets for the consolidated project tools. Single source per tool: feeds
55+
// both the schema enum (methodEnum) and the unknown-method error
56+
// (unknownMethodError). Order matches the advertised enum.
57+
var (
58+
projectsListMethods = []string{projectsMethodListProjects, projectsMethodListProjectFields, projectsMethodListProjectItems, projectsMethodListProjectStatusUpdates}
59+
projectsGetMethods = []string{projectsMethodGetProject, projectsMethodGetProjectField, projectsMethodGetProjectItem, projectsMethodGetProjectStatusUpdate}
60+
projectsWriteMethods = []string{projectsMethodAddProjectItem, projectsMethodUpdateProjectItem, projectsMethodDeleteProjectItem, projectsMethodCreateProjectStatusUpdate, projectsMethodCreateProject, projectsMethodCreateIterationField}
61+
)
62+
5463
// GraphQL types for ProjectV2 status updates
5564

5665
type statusUpdateNode struct {
@@ -169,12 +178,7 @@ Use this tool to list projects for a user or organization, or list project field
169178
"method": {
170179
Type: "string",
171180
Description: "The action to perform",
172-
Enum: []any{
173-
projectsMethodListProjects,
174-
projectsMethodListProjectFields,
175-
projectsMethodListProjectItems,
176-
projectsMethodListProjectStatusUpdates,
177-
},
181+
Enum: methodEnum(projectsListMethods),
178182
},
179183
"owner_type": {
180184
Type: "string",
@@ -284,10 +288,10 @@ Use this tool to list projects for a user or organization, or list project field
284288
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate))
285289
return result, payload, err
286290
default:
287-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
291+
return unknownMethodError(method, projectsListMethods), nil, nil
288292
}
289293
default:
290-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
294+
return unknownMethodError(method, projectsListMethods), nil, nil
291295
}
292296
},
293297
)
@@ -313,12 +317,7 @@ Use this tool to get details about individual projects, project fields, and proj
313317
"method": {
314318
Type: "string",
315319
Description: "The method to execute",
316-
Enum: []any{
317-
projectsMethodGetProject,
318-
projectsMethodGetProjectField,
319-
projectsMethodGetProjectItem,
320-
projectsMethodGetProjectStatusUpdate,
321-
},
320+
Enum: methodEnum(projectsGetMethods),
322321
},
323322
"owner_type": {
324323
Type: "string",
@@ -442,7 +441,7 @@ Use this tool to get details about individual projects, project fields, and proj
442441
}
443442
return result, payload, err
444443
default:
445-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
444+
return unknownMethodError(method, projectsGetMethods), nil, nil
446445
}
447446
},
448447
)
@@ -467,14 +466,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
467466
"method": {
468467
Type: "string",
469468
Description: "The method to execute",
470-
Enum: []any{
471-
projectsMethodAddProjectItem,
472-
projectsMethodUpdateProjectItem,
473-
projectsMethodDeleteProjectItem,
474-
projectsMethodCreateProjectStatusUpdate,
475-
projectsMethodCreateProject,
476-
projectsMethodCreateIterationField,
477-
},
469+
Enum: methodEnum(projectsWriteMethods),
478470
},
479471
"owner_type": {
480472
Type: "string",
@@ -692,7 +684,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
692684
case projectsMethodCreateIterationField:
693685
return createIterationField(ctx, gqlClient, owner, ownerType, projectNumber, args)
694686
default:
695-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
687+
return unknownMethodError(method, projectsWriteMethods), nil, nil
696688
}
697689
},
698690
)

pkg/github/projects_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func Test_ProjectsList_ListProjects(t *testing.T) {
9393
"owner_type": "org",
9494
},
9595
expectError: true,
96-
expectedErrMsg: "unknown method: unknown_method",
96+
expectedErrMsg: "unknown method: unknown_method. Supported methods are:",
9797
},
9898
}
9999

@@ -625,9 +625,11 @@ func Test_ProjectsGet_GetProject(t *testing.T) {
625625
require.True(t, result.IsError)
626626
textContent := getTextResult(t, result)
627627
assert.Contains(t, textContent.Text, "unknown method: unknown_method")
628+
assert.Contains(t, textContent.Text, "Supported methods are:")
628629
})
629630
}
630631

632+
631633
func Test_ProjectsGet_IFC_InsidersMode(t *testing.T) {
632634
toolDef := ProjectsGet(translations.NullTranslationHelper)
633635

@@ -1113,9 +1115,11 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) {
11131115
require.True(t, result.IsError)
11141116
textContent := getTextResult(t, result)
11151117
assert.Contains(t, textContent.Text, "unknown method: unknown_method")
1118+
assert.Contains(t, textContent.Text, "Supported methods are:")
11161119
})
11171120
}
11181121

1122+
11191123
func Test_ProjectsWrite_UpdateProjectItem(t *testing.T) {
11201124
toolDef := ProjectsWrite(translations.NullTranslationHelper)
11211125

pkg/github/pullrequests.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ import (
2323
"github.com/github/github-mcp-server/pkg/utils"
2424
)
2525

26+
// Method sets for the consolidated pull request tools (single source for the
27+
// schema enum via methodEnum and the unknown-method error via unknownMethodError).
28+
var (
29+
pullRequestReadMethods = []string{"get", "get_diff", "get_status", "get_files", "get_commits", "get_review_comments", "get_reviews", "get_comments", "get_check_runs"}
30+
pullRequestReviewWriteMethods = []string{"create", "submit_pending", "delete_pending", "resolve_thread", "unresolve_thread"}
31+
)
32+
2633
// PullRequestRead creates a tool to get details of a specific pull request.
2734
func PullRequestRead(t translations.TranslationHelperFunc) inventory.ServerTool {
2835
schema := &jsonschema.Schema{
@@ -42,7 +49,7 @@ Possible options:
4249
8. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned.
4350
9. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR.
4451
`,
45-
Enum: []any{"get", "get_diff", "get_status", "get_files", "get_commits", "get_review_comments", "get_reviews", "get_comments", "get_check_runs"},
52+
Enum: methodEnum(pullRequestReadMethods),
4653
},
4754
"owner": {
4855
Type: "string",
@@ -155,7 +162,7 @@ Possible options:
155162
result, err := GetPullRequestCheckRuns(ctx, client, owner, repo, pullNumber, pagination)
156163
return attachIFC(result), nil, err
157164
default:
158-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
165+
return unknownMethodError(method, pullRequestReadMethods), nil, nil
159166
}
160167
})
161168
}
@@ -1728,7 +1735,7 @@ func PullRequestReviewWrite(t translations.TranslationHelperFunc) inventory.Serv
17281735
"method": {
17291736
Type: "string",
17301737
Description: `The write operation to perform on pull request review.`,
1731-
Enum: []any{"create", "submit_pending", "delete_pending", "resolve_thread", "unresolve_thread"},
1738+
Enum: methodEnum(pullRequestReviewWriteMethods),
17321739
},
17331740
"owner": {
17341741
Type: "string",
@@ -1789,6 +1796,15 @@ Available methods:
17891796
return utils.NewToolResultError(err.Error()), nil, nil
17901797
}
17911798

1799+
// Unlike the other method-dispatch tools, this handler decodes args
1800+
// with WeakDecode rather than RequiredParam, so an omitted method
1801+
// would otherwise reach the switch default as an empty value. Enforce
1802+
// it explicitly so a missing method is reported the same way as every
1803+
// other missing required parameter.
1804+
if _, err := RequiredParam[string](args, "method"); err != nil {
1805+
return utils.NewToolResultError(err.Error()), nil, nil
1806+
}
1807+
17921808
// Given our owner, repo and PR number, lookup the GQL ID of the PR.
17931809
client, err := deps.GetGQLClient(ctx)
17941810
if err != nil {
@@ -1812,7 +1828,7 @@ Available methods:
18121828
result, err := ResolveReviewThread(ctx, client, params.ThreadID, false)
18131829
return result, nil, err
18141830
default:
1815-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", params.Method)), nil, nil
1831+
return unknownMethodError(params.Method, pullRequestReviewWriteMethods), nil, nil
18161832
}
18171833
})
18181834
st.FeatureFlagDisable = []string{FeatureFlagPullRequestsGranular}

0 commit comments

Comments
 (0)