Skip to content

Commit ccf54a5

Browse files
committed
Enforce and advertise the method enum for method-dispatch tools
The consolidated method-dispatch tools returned a bare "unknown method: <x>" with no hint of valid values, and each tool's valid method set lived only in its schema enum (or, for sub_issue_write, only in prose). labels.go already listed the supported methods on an unknown value; this brings the rest in line and makes the advertised methods the single source of truth. - Each tool's method set is a single []string that feeds both the input-schema enum (methodEnum) and the unknown-method error (unknownMethodError), so the advertised methods and the accepted methods cannot drift apart. - An unknown method now lists the supported set: "unknown method: x. Supported methods are: ...". - pull_request_review_write decoded args with WeakDecode (no RequiredParam), so an omitted method surfaced as "unknown method: " (empty). It now reports "missing required parameter: method", consistent with the other tools. - sub_issue_write advertised its method options only in the description; it now declares the enum like every other method-dispatch tool. Closes #2712
1 parent 4e8eb81 commit ccf54a5

11 files changed

Lines changed: 129 additions & 58 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_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 {
@@ -736,7 +743,7 @@ Options are:
736743
3. get_sub_issues - Get sub-issues of the issue.
737744
4. get_labels - Get labels assigned to the issue.
738745
`,
739-
Enum: []any{"get", "get_comments", "get_sub_issues", "get_labels"},
746+
Enum: methodEnum(issueReadMethods),
740747
},
741748
"owner": {
742749
Type: "string",
@@ -820,7 +827,7 @@ Options are:
820827
result, err := GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber)
821828
return attachIFC(result), nil, err
822829
default:
823-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
830+
return unknownMethodError(method, issueReadMethods), nil, nil
824831
}
825832
})
826833
}
@@ -1234,6 +1241,7 @@ func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
12341241
Properties: map[string]*jsonschema.Schema{
12351242
"method": {
12361243
Type: "string",
1244+
Enum: methodEnum(subIssueWriteMethods),
12371245
Description: `The action to perform on a single sub-issue
12381246
Options are:
12391247
- 'add' - add a sub-issue to a parent issue in a GitHub repository.
@@ -1327,7 +1335,7 @@ Options are:
13271335
result, err := ReprioritizeSubIssue(ctx, client, owner, repo, issueNumber, subIssueID, afterID, beforeID)
13281336
return result, nil, err
13291337
default:
1330-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
1338+
return unknownMethodError(method, subIssueWriteMethods), nil, nil
13311339
}
13321340
})
13331341
st.FeatureFlagDisable = []string{FeatureFlagIssuesGranular}

pkg/github/params.go

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

pkg/github/params_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,3 +642,19 @@ func TestOptionalPaginationParams(t *testing.T) {
642642
})
643643
}
644644
}
645+
646+
func TestMethodEnum(t *testing.T) {
647+
// methodEnum must preserve methods and order so the advertised schema enum
648+
// stays byte-identical to the source slice.
649+
assert.Equal(t, []any{"create", "submit_pending"}, methodEnum([]string{"create", "submit_pending"}))
650+
assert.Empty(t, methodEnum(nil))
651+
}
652+
653+
func TestUnknownMethodError(t *testing.T) {
654+
res := unknownMethodError("bogus", []string{"create", "submit_pending", "delete_pending"})
655+
assert.NotNil(t, res)
656+
assert.True(t, res.IsError)
657+
txt := getErrorResult(t, res).Text
658+
assert.Contains(t, txt, "unknown method: bogus")
659+
assert.Contains(t, txt, "Supported methods are: create, submit_pending, delete_pending")
660+
}

pkg/github/projects.go

Lines changed: 15 additions & 23 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 {
@@ -159,12 +168,7 @@ Use this tool to list projects for a user or organization, or list project field
159168
"method": {
160169
Type: "string",
161170
Description: "The action to perform",
162-
Enum: []any{
163-
projectsMethodListProjects,
164-
projectsMethodListProjectFields,
165-
projectsMethodListProjectItems,
166-
projectsMethodListProjectStatusUpdates,
167-
},
171+
Enum: methodEnum(projectsListMethods),
168172
},
169173
"owner_type": {
170174
Type: "string",
@@ -269,7 +273,7 @@ Use this tool to list projects for a user or organization, or list project field
269273
result, payload, err := listProjectStatusUpdates(ctx, gqlClient, args, owner, ownerType)
270274
return attachIFC(result), payload, err
271275
default:
272-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
276+
return unknownMethodError(method, projectsListMethods), nil, nil
273277
}
274278
}
275279
},
@@ -296,12 +300,7 @@ Use this tool to get details about individual projects, project fields, and proj
296300
"method": {
297301
Type: "string",
298302
Description: "The method to execute",
299-
Enum: []any{
300-
projectsMethodGetProject,
301-
projectsMethodGetProjectField,
302-
projectsMethodGetProjectItem,
303-
projectsMethodGetProjectStatusUpdate,
304-
},
303+
Enum: methodEnum(projectsGetMethods),
305304
},
306305
"owner_type": {
307306
Type: "string",
@@ -419,7 +418,7 @@ Use this tool to get details about individual projects, project fields, and proj
419418
result, payload, err := getProjectItem(ctx, client, owner, ownerType, projectNumber, itemID, fields)
420419
return attachIFC(result), payload, err
421420
default:
422-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
421+
return unknownMethodError(method, projectsGetMethods), nil, nil
423422
}
424423
},
425424
)
@@ -444,14 +443,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
444443
"method": {
445444
Type: "string",
446445
Description: "The method to execute",
447-
Enum: []any{
448-
projectsMethodAddProjectItem,
449-
projectsMethodUpdateProjectItem,
450-
projectsMethodDeleteProjectItem,
451-
projectsMethodCreateProjectStatusUpdate,
452-
projectsMethodCreateProject,
453-
projectsMethodCreateIterationField,
454-
},
446+
Enum: methodEnum(projectsWriteMethods),
455447
},
456448
"owner_type": {
457449
Type: "string",
@@ -669,7 +661,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
669661
case projectsMethodCreateIterationField:
670662
return createIterationField(ctx, gqlClient, owner, ownerType, projectNumber, args)
671663
default:
672-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
664+
return unknownMethodError(method, projectsWriteMethods), nil, nil
673665
}
674666
},
675667
)

pkg/github/projects_test.go

Lines changed: 3 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

@@ -435,6 +435,7 @@ func Test_ProjectsGet_GetProject(t *testing.T) {
435435
require.True(t, result.IsError)
436436
textContent := getTextResult(t, result)
437437
assert.Contains(t, textContent.Text, "unknown method: unknown_method")
438+
assert.Contains(t, textContent.Text, "Supported methods are:")
438439
})
439440
}
440441

@@ -850,6 +851,7 @@ func Test_ProjectsWrite_AddProjectItem(t *testing.T) {
850851
require.True(t, result.IsError)
851852
textContent := getTextResult(t, result)
852853
assert.Contains(t, textContent.Text, "unknown method: unknown_method")
854+
assert.Contains(t, textContent.Text, "Supported methods are:")
853855
})
854856
}
855857

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)