Skip to content

Commit be5da1c

Browse files
Tethys0Tethys0
committed
Make project method errors actionable
1 parent 64a49f3 commit be5da1c

8 files changed

Lines changed: 224 additions & 12 deletions

File tree

‎pkg/github/__toolsnaps__/sub_issue_write.snap‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@
2121
},
2222
"method": {
2323
"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.\nWrites issue hierarchy. To move a sub-issue to a new parent, use `add` with `replace_parent=true`; there is no writable parent field.\n",
24+
"enum": [
25+
"add",
26+
"remove",
27+
"reprioritize"
28+
],
2429
"type": "string"
2530
},
2631
"owner": {

‎pkg/github/actions.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an
397397
result, payload, err := listWorkflowArtifacts(ctx, client, owner, repo, resourceIDInt, pagination)
398398
return attachIFC(result), payload, err
399399
default:
400-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
400+
return unknownMethodError(method, actionsMethodListWorkflows, actionsMethodListWorkflowRuns, actionsMethodListWorkflowJobs, actionsMethodListWorkflowArtifacts), nil, nil
401401
}
402402
},
403403
)
@@ -519,7 +519,7 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an
519519
result, payload, err := getWorkflowRunLogsURL(ctx, client, owner, repo, resourceIDInt)
520520
return attachIFC(result), payload, err
521521
default:
522-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
522+
return unknownMethodError(method, actionsMethodGetWorkflow, actionsMethodGetWorkflowRun, actionsMethodGetWorkflowJob, actionsMethodDownloadWorkflowArtifact, actionsMethodGetWorkflowRunUsage, actionsMethodGetWorkflowRunLogsURL), nil, nil
523523
}
524524
},
525525
)
@@ -636,7 +636,7 @@ func ActionsRunTrigger(t translations.TranslationHelperFunc) inventory.ServerToo
636636
case actionsMethodDeleteWorkflowRunLogs:
637637
return deleteWorkflowRunLogs(ctx, client, owner, repo, int64(runID))
638638
default:
639-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
639+
return unknownMethodError(method, actionsMethodRunWorkflow, actionsMethodRerunWorkflowRun, actionsMethodRerunFailedJobs, actionsMethodCancelWorkflowRun, actionsMethodDeleteWorkflowRunLogs), nil, nil
640640
}
641641
},
642642
)

‎pkg/github/dispatch_errors_test.go‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"maps"
6+
"net/http"
7+
"strings"
8+
"testing"
9+
10+
"github.com/github/github-mcp-server/pkg/inventory"
11+
"github.com/github/github-mcp-server/pkg/translations"
12+
"github.com/google/jsonschema-go/jsonschema"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestAffectedDispatchersReportSupportedMethods(t *testing.T) {
18+
t.Parallel()
19+
20+
tests := []struct {
21+
name string
22+
tool func() inventory.ServerTool
23+
requestArg map[string]any
24+
}{
25+
{
26+
name: "pull request read",
27+
tool: func() inventory.ServerTool {
28+
return PullRequestRead(translations.NullTranslationHelper)
29+
},
30+
requestArg: map[string]any{
31+
"owner": "owner",
32+
"repo": "repo",
33+
"pullNumber": float64(1),
34+
},
35+
},
36+
{
37+
name: "issue read",
38+
tool: func() inventory.ServerTool {
39+
return IssueRead(translations.NullTranslationHelper)
40+
},
41+
requestArg: map[string]any{
42+
"owner": "owner",
43+
"repo": "repo",
44+
"issue_number": float64(1),
45+
},
46+
},
47+
{
48+
name: "sub issue write",
49+
tool: func() inventory.ServerTool {
50+
return SubIssueWrite(translations.NullTranslationHelper)
51+
},
52+
requestArg: map[string]any{
53+
"owner": "owner",
54+
"repo": "repo",
55+
"issue_number": float64(1),
56+
"sub_issue_id": float64(2),
57+
},
58+
},
59+
{
60+
name: "actions list",
61+
tool: func() inventory.ServerTool {
62+
return ActionsList(translations.NullTranslationHelper)
63+
},
64+
requestArg: map[string]any{
65+
"owner": "owner",
66+
"repo": "repo",
67+
"resource_id": "1",
68+
},
69+
},
70+
{
71+
name: "actions get",
72+
tool: func() inventory.ServerTool {
73+
return ActionsGet(translations.NullTranslationHelper)
74+
},
75+
requestArg: map[string]any{
76+
"owner": "owner",
77+
"repo": "repo",
78+
"resource_id": "1",
79+
},
80+
},
81+
{
82+
name: "actions run",
83+
tool: func() inventory.ServerTool {
84+
return ActionsRunTrigger(translations.NullTranslationHelper)
85+
},
86+
requestArg: map[string]any{
87+
"owner": "owner",
88+
"repo": "repo",
89+
"run_id": float64(1),
90+
},
91+
},
92+
{
93+
name: "projects list",
94+
tool: func() inventory.ServerTool {
95+
return ProjectsList(translations.NullTranslationHelper)
96+
},
97+
requestArg: map[string]any{
98+
"owner": "owner",
99+
"owner_type": "org",
100+
},
101+
},
102+
{
103+
name: "projects get",
104+
tool: func() inventory.ServerTool {
105+
return ProjectsGet(translations.NullTranslationHelper)
106+
},
107+
requestArg: map[string]any{
108+
"owner": "owner",
109+
"owner_type": "org",
110+
"project_number": float64(1),
111+
},
112+
},
113+
{
114+
name: "projects write",
115+
tool: func() inventory.ServerTool {
116+
return ProjectsWrite(translations.NullTranslationHelper)
117+
},
118+
requestArg: map[string]any{
119+
"owner": "owner",
120+
"owner_type": "org",
121+
"project_number": float64(1),
122+
},
123+
},
124+
{
125+
name: "ui get",
126+
tool: func() inventory.ServerTool {
127+
return UIGet(translations.NullTranslationHelper)
128+
},
129+
requestArg: map[string]any{
130+
"owner": "owner",
131+
},
132+
},
133+
}
134+
135+
for _, tc := range tests {
136+
t.Run(tc.name, func(t *testing.T) {
137+
tool := tc.tool()
138+
schema := tool.Tool.InputSchema.(*jsonschema.Schema)
139+
methodSchema := schema.Properties["method"]
140+
require.NotNil(t, methodSchema)
141+
require.NotEmpty(t, methodSchema.Enum)
142+
143+
methods := make([]string, len(methodSchema.Enum))
144+
for i, method := range methodSchema.Enum {
145+
methods[i] = method.(string)
146+
}
147+
148+
args := make(map[string]any, len(tc.requestArg)+1)
149+
maps.Copy(args, tc.requestArg)
150+
args["method"] = "unknown_method"
151+
152+
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}))
153+
deps := BaseDeps{Client: client, GQLClient: defaultGQLClient}
154+
request := createMCPRequest(args)
155+
result, err := tool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request)
156+
157+
require.NoError(t, err)
158+
require.True(t, result.IsError)
159+
assert.Equal(t,
160+
"unknown method: unknown_method. Supported methods are: "+strings.Join(methods, ", "),
161+
getErrorResult(t, result).Text,
162+
)
163+
})
164+
}
165+
}
166+
167+
func TestPullRequestReviewWriteMissingMethodIsRequired(t *testing.T) {
168+
t.Parallel()
169+
170+
tool := PullRequestReviewWrite(translations.NullTranslationHelper)
171+
deps := BaseDeps{GQLClient: defaultGQLClient}
172+
request := createMCPRequest(map[string]any{
173+
"owner": "owner",
174+
"repo": "repo",
175+
"pullNumber": float64(1),
176+
})
177+
178+
result, err := tool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request)
179+
180+
require.NoError(t, err)
181+
require.True(t, result.IsError)
182+
assert.Equal(t, "missing required parameter: method", getErrorResult(t, result).Text)
183+
}

‎pkg/github/issues.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -888,7 +888,7 @@ func IssueRead(t translations.TranslationHelperFunc) inventory.ServerTool {
888888
result, err := GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber)
889889
return attachIFC(result), nil, err
890890
default:
891-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
891+
return unknownMethodError(method, "get", "get_comments", "get_sub_issues", "get_parent", "get_labels"), nil, nil
892892
}
893893
})
894894
}
@@ -1587,6 +1587,7 @@ func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
15871587
"- 'remove' - remove a sub-issue from a parent issue in a GitHub repository.\n" +
15881588
"- '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" +
15891589
"Writes issue hierarchy. To move a sub-issue to a new parent, use `add` with `replace_parent=true`; there is no writable parent field.\n",
1590+
Enum: []any{"add", "remove", "reprioritize"},
15901591
},
15911592
"owner": {
15921593
Type: "string",
@@ -1674,7 +1675,7 @@ func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
16741675
result, err := ReprioritizeSubIssue(ctx, client, owner, repo, issueNumber, subIssueID, afterID, beforeID)
16751676
return result, nil, err
16761677
default:
1677-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
1678+
return unknownMethodError(method, "add", "remove", "reprioritize"), nil, nil
16781679
}
16791680
})
16801681
st.FeatureFlagDisable = []string{FeatureFlagIssuesGranular}

‎pkg/github/method_errors.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package github
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
7+
"github.com/github/github-mcp-server/pkg/utils"
8+
"github.com/modelcontextprotocol/go-sdk/mcp"
9+
)
10+
11+
func unknownMethodError(method string, supportedMethods ...string) *mcp.CallToolResult {
12+
return utils.NewToolResultError(fmt.Sprintf(
13+
"unknown method: %s. Supported methods are: %s",
14+
method,
15+
strings.Join(supportedMethods, ", "),
16+
))
17+
}

‎pkg/github/projects.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -437,10 +437,10 @@ Use this tool to list projects for a user or organization, or list project field
437437
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate))
438438
return result, payload, err
439439
default:
440-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
440+
return unknownMethodError(method, projectsMethodListProjects, projectsMethodListProjectFields, projectsMethodListProjectItems, projectsMethodListProjectStatusUpdates, projectsMethodListProjectViews), nil, nil
441441
}
442442
default:
443-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
443+
return unknownMethodError(method, projectsMethodListProjects, projectsMethodListProjectFields, projectsMethodListProjectItems, projectsMethodListProjectStatusUpdates, projectsMethodListProjectViews), nil, nil
444444
}
445445
},
446446
)
@@ -642,7 +642,7 @@ Use this tool to get details about individual projects, project fields, project
642642
}
643643
return result, payload, err
644644
default:
645-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
645+
return unknownMethodError(method, projectsMethodGetProject, projectsMethodGetProjectField, projectsMethodGetProjectItem, projectsMethodGetProjectStatusUpdate, projectsMethodGetProjectView), nil, nil
646646
}
647647
},
648648
)
@@ -1037,7 +1037,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
10371037
case projectsMethodDeleteProjectView:
10381038
return deleteProjectView(ctx, gqlClient, args, owner, ownerType, projectNumber)
10391039
default:
1040-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
1040+
return unknownMethodError(method, projectsMethodAddProjectItem, projectsMethodUpdateProjectItem, projectsMethodUpdateProjectItems, projectsMethodDeleteProjectItem, projectsMethodCreateProjectStatusUpdate, projectsMethodCreateProjectView, projectsMethodUpdateProjectView, projectsMethodDeleteProjectView, projectsMethodCreateProject, projectsMethodCreateIterationField), nil, nil
10411041
}
10421042
},
10431043
)

‎pkg/github/pullrequests.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ Possible options:
154154
result, err := GetPullRequestCheckRuns(ctx, client, owner, repo, pullNumber, pagination)
155155
return attachIFC(result), nil, err
156156
default:
157-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
157+
return unknownMethodError(method, "get", "get_diff", "get_status", "get_files", "get_commits", "get_review_comments", "get_reviews", "get_comments", "get_check_runs"), nil, nil
158158
}
159159
})
160160
}
@@ -1836,10 +1836,16 @@ Available methods:
18361836
},
18371837
[]scopes.Scope{scopes.Repo},
18381838
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
1839+
method, err := RequiredParam[string](args, "method")
1840+
if err != nil {
1841+
return utils.NewToolResultError(err.Error()), nil, nil
1842+
}
1843+
18391844
var params PullRequestReviewWriteParams
18401845
if err := mapstructure.WeakDecode(args, &params); err != nil {
18411846
return utils.NewToolResultError(err.Error()), nil, nil
18421847
}
1848+
params.Method = method
18431849

18441850
// Given our owner, repo and PR number, lookup the GQL ID of the PR.
18451851
client, err := deps.GetGQLClient(ctx)
@@ -1864,7 +1870,7 @@ Available methods:
18641870
result, err := ResolveReviewThread(ctx, client, params.ThreadID, false)
18651871
return result, nil, err
18661872
default:
1867-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", params.Method)), nil, nil
1873+
return unknownMethodError(params.Method, "create", "submit_pending", "delete_pending", "resolve_thread", "unresolve_thread"), nil, nil
18681874
}
18691875
})
18701876
st.FeatureFlagDisable = []string{FeatureFlagPullRequestsGranular}

‎pkg/github/ui_tools.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func UIGet(t translations.TranslationHelperFunc) inventory.ServerTool {
9696
case "reviewers":
9797
return uiGetReviewers(ctx, deps, args, owner)
9898
default:
99-
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
99+
return unknownMethodError(method, "labels", "assignees", "milestones", "issue_types", "branches", "issue_fields", "reviewers"), nil, nil
100100
}
101101
})
102102
st.FeatureFlagEnable = MCPAppsFeatureFlag

0 commit comments

Comments
 (0)