From d8732b593e02d96091dea290cfb632490d134cff Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Wed, 8 Apr 2026 15:32:11 +0100 Subject: [PATCH 1/5] initial OSS granular PRs and issues toolsets --- pkg/github/granular_tools_test.go | 247 +++++++++ pkg/github/issues_granular.go | 723 +++++++++++++++++++++++++++ pkg/github/pullrequests_granular.go | 742 ++++++++++++++++++++++++++++ pkg/github/tools.go | 36 ++ 4 files changed, 1748 insertions(+) create mode 100644 pkg/github/granular_tools_test.go create mode 100644 pkg/github/issues_granular.go create mode 100644 pkg/github/pullrequests_granular.go diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go new file mode 100644 index 000000000..465d7164a --- /dev/null +++ b/pkg/github/granular_tools_test.go @@ -0,0 +1,247 @@ +package github + +import ( + "context" + "net/http" + "testing" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/translations" + gogithub "github.com/google/go-github/v82/github" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func granularToolsForToolset(id inventory.ToolsetID) []inventory.ServerTool { + var result []inventory.ServerTool + for _, tool := range AllTools(translations.NullTranslationHelper) { + if tool.Toolset.ID == id { + result = append(result, tool) + } + } + return result +} + +func TestIssuesGranularToolset(t *testing.T) { + t.Run("toolset contains expected tools", func(t *testing.T) { + tools := granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) + + toolNames := make([]string, 0, len(tools)) + for _, tool := range tools { + toolNames = append(toolNames, tool.Tool.Name) + } + + assert.Contains(t, toolNames, "create_issue") + assert.Contains(t, toolNames, "update_issue_title") + assert.Contains(t, toolNames, "update_issue_body") + assert.Contains(t, toolNames, "update_issue_assignees") + assert.Contains(t, toolNames, "update_issue_labels") + assert.Contains(t, toolNames, "update_issue_milestone") + assert.Contains(t, toolNames, "update_issue_type") + assert.Contains(t, toolNames, "update_issue_state") + assert.Contains(t, toolNames, "add_sub_issue") + assert.Contains(t, toolNames, "remove_sub_issue") + assert.Contains(t, toolNames, "reprioritize_sub_issue") + }) + + t.Run("all tools belong to issues_granular toolset", func(t *testing.T) { + tools := granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) + + for _, tool := range tools { + assert.Equal(t, ToolsetMetadataIssuesGranular.ID, tool.Toolset.ID, "tool %s should belong to issues_granular toolset", tool.Tool.Name) + } + }) + + t.Run("all tools have ReadOnlyHint false", func(t *testing.T) { + tools := granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) + + for _, tool := range tools { + assert.False(t, tool.Tool.Annotations.ReadOnlyHint, "tool %s should have ReadOnlyHint=false", tool.Tool.Name) + } + }) + + t.Run("toolset is non-default", func(t *testing.T) { + assert.False(t, ToolsetMetadataIssuesGranular.Default, "issues_granular toolset should not be default") + }) +} + +func TestPullRequestsGranularToolset(t *testing.T) { + t.Run("toolset contains expected tools", func(t *testing.T) { + tools := granularToolsForToolset(ToolsetMetadataPullRequestsGranular.ID) + + toolNames := make([]string, 0, len(tools)) + for _, tool := range tools { + toolNames = append(toolNames, tool.Tool.Name) + } + + assert.Contains(t, toolNames, "update_pull_request_title") + assert.Contains(t, toolNames, "update_pull_request_body") + assert.Contains(t, toolNames, "update_pull_request_state") + assert.Contains(t, toolNames, "update_pull_request_draft_state") + assert.Contains(t, toolNames, "request_pull_request_reviewers") + assert.Contains(t, toolNames, "create_pull_request_review") + assert.Contains(t, toolNames, "submit_pending_pull_request_review") + assert.Contains(t, toolNames, "delete_pending_pull_request_review") + assert.Contains(t, toolNames, "add_pull_request_review_comment") + }) + + t.Run("all tools belong to pull_requests_granular toolset", func(t *testing.T) { + tools := granularToolsForToolset(ToolsetMetadataPullRequestsGranular.ID) + + for _, tool := range tools { + assert.Equal(t, ToolsetMetadataPullRequestsGranular.ID, tool.Toolset.ID, "tool %s should belong to pull_requests_granular toolset", tool.Tool.Name) + } + }) + + t.Run("toolset is non-default", func(t *testing.T) { + assert.False(t, ToolsetMetadataPullRequestsGranular.Default, "pull_requests_granular toolset should not be default") + }) +} + +func TestGranularCreateIssue(t *testing.T) { + mockIssue := &gogithub.Issue{ + Number: gogithub.Ptr(1), + Title: gogithub.Ptr("Test Issue"), + Body: gogithub.Ptr("Test body"), + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectedErrMsg string + }{ + { + name: "successful creation", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposIssuesByOwnerByRepo: expectRequestBody(t, map[string]any{ + "title": "Test Issue", + "body": "Test body", + }).andThen(mockResponse(t, http.StatusCreated, mockIssue)), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "title": "Test Issue", + "body": "Test body", + }, + }, + { + name: "missing required parameter", + mockedClient: MockHTTPClientWithHandlers(nil), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + }, + expectedErrMsg: "missing required parameter: title", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := gogithub.NewClient(tc.mockedClient) + deps := BaseDeps{Client: client} + serverTool := GranularCreateIssue(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + if tc.expectedErrMsg != "" { + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + assert.False(t, result.IsError) + }) + } +} + +func TestGranularUpdateIssueTitle(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &gogithub.Issue{ + Number: gogithub.Ptr(42), + Title: gogithub.Ptr("New Title"), + }), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueTitle(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + "title": "New Title", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func TestGranularUpdateIssueLabels(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "labels": []any{"bug", "enhancement"}, + }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueLabels(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": []string{"bug", "enhancement"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func TestGranularUpdateIssueState(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "state": "closed", + "state_reason": "completed", + }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{ + Number: gogithub.Ptr(1), + State: gogithub.Ptr("closed"), + })), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueState(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "state_reason": "completed", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func TestGranularRequestPullRequestReviewers(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &gogithub.PullRequest{Number: gogithub.Ptr(1)}), + })) + deps := BaseDeps{Client: client} + serverTool := GranularRequestPullRequestReviewers(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(1), + "reviewers": []string{"user1", "user2"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go new file mode 100644 index 000000000..ae267d79d --- /dev/null +++ b/pkg/github/issues_granular.go @@ -0,0 +1,723 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "maps" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/google/go-github/v82/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/shurcooL/githubv4" +) + +// issueUpdateTool is a helper to create single-field issue update tools. +func issueUpdateTool( + t translations.TranslationHelperFunc, + name, description, title string, + extraProps map[string]*jsonschema.Schema, + extraRequired []string, + buildRequest func(args map[string]any) (*github.IssueRequest, error), +) inventory.ServerTool { + props := map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "issue_number": { + Type: "number", + Description: "The issue number to update", + Minimum: jsonschema.Ptr(1.0), + }, + } + maps.Copy(props, extraProps) + + required := append([]string{"owner", "repo", "issue_number"}, extraRequired...) + + return NewTool( + ToolsetMetadataIssuesGranular, + mcp.Tool{ + Name: name, + Description: t("TOOL_"+name+"_DESCRIPTION", description), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_"+name+"_USER_TITLE", title), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: props, + Required: required, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + issueReq, err := buildRequest(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + issue, _, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueReq) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to update issue", err), nil, nil + } + + r, err := json.Marshal(issue) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GranularCreateIssue creates a tool to create a new issue. +func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataIssuesGranular, + mcp.Tool{ + Name: "create_issue", + Description: t("TOOL_CREATE_ISSUE_DESCRIPTION", "Create a new issue in a GitHub repository with a title and optional body."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_CREATE_ISSUE_USER_TITLE", "Create Issue"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "title": { + Type: "string", + Description: "Issue title", + }, + "body": { + Type: "string", + Description: "Issue body content (optional)", + }, + }, + Required: []string{"owner", "repo", "title"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + title, err := RequiredParam[string](args, "title") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + body, _ := OptionalParam[string](args, "body") + + issueReq := &github.IssueRequest{ + Title: &title, + } + if body != "" { + issueReq.Body = &body + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + issue, _, err := client.Issues.Create(ctx, owner, repo, issueReq) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create issue", err), nil, nil + } + + r, err := json.Marshal(issue) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GranularUpdateIssueTitle creates a tool to update an issue's title. +func GranularUpdateIssueTitle(t translations.TranslationHelperFunc) inventory.ServerTool { + return issueUpdateTool(t, + "update_issue_title", + "Update the title of an existing issue.", + "Update Issue Title", + map[string]*jsonschema.Schema{ + "title": {Type: "string", Description: "The new title for the issue"}, + }, + []string{"title"}, + func(args map[string]any) (*github.IssueRequest, error) { + title, err := RequiredParam[string](args, "title") + if err != nil { + return nil, err + } + return &github.IssueRequest{Title: &title}, nil + }, + ) +} + +// GranularUpdateIssueBody creates a tool to update an issue's body. +func GranularUpdateIssueBody(t translations.TranslationHelperFunc) inventory.ServerTool { + return issueUpdateTool(t, + "update_issue_body", + "Update the body content of an existing issue.", + "Update Issue Body", + map[string]*jsonschema.Schema{ + "body": {Type: "string", Description: "The new body content for the issue"}, + }, + []string{"body"}, + func(args map[string]any) (*github.IssueRequest, error) { + body, err := RequiredParam[string](args, "body") + if err != nil { + return nil, err + } + return &github.IssueRequest{Body: &body}, nil + }, + ) +} + +// GranularUpdateIssueAssignees creates a tool to update an issue's assignees. +func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventory.ServerTool { + return issueUpdateTool(t, + "update_issue_assignees", + "Update the assignees of an existing issue. This replaces the current assignees with the provided list.", + "Update Issue Assignees", + map[string]*jsonschema.Schema{ + "assignees": { + Type: "array", + Description: "GitHub usernames to assign to this issue", + Items: &jsonschema.Schema{Type: "string"}, + }, + }, + []string{"assignees"}, + func(args map[string]any) (*github.IssueRequest, error) { + raw, _ := OptionalParam[[]any](args, "assignees") + if len(raw) == 0 { + return nil, fmt.Errorf("missing required parameter: assignees") + } + assignees := make([]string, 0, len(raw)) + for _, v := range raw { + if s, ok := v.(string); ok { + assignees = append(assignees, s) + } + } + return &github.IssueRequest{Assignees: &assignees}, nil + }, + ) +} + +// GranularUpdateIssueLabels creates a tool to update an issue's labels. +func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.ServerTool { + return issueUpdateTool(t, + "update_issue_labels", + "Update the labels of an existing issue. This replaces the current labels with the provided list.", + "Update Issue Labels", + map[string]*jsonschema.Schema{ + "labels": { + Type: "array", + Description: "Labels to apply to this issue", + Items: &jsonschema.Schema{Type: "string"}, + }, + }, + []string{"labels"}, + func(args map[string]any) (*github.IssueRequest, error) { + raw, _ := OptionalParam[[]any](args, "labels") + if len(raw) == 0 { + return nil, fmt.Errorf("missing required parameter: labels") + } + labels := make([]string, 0, len(raw)) + for _, v := range raw { + if s, ok := v.(string); ok { + labels = append(labels, s) + } + } + return &github.IssueRequest{Labels: &labels}, nil + }, + ) +} + +// GranularUpdateIssueMilestone creates a tool to update an issue's milestone. +func GranularUpdateIssueMilestone(t translations.TranslationHelperFunc) inventory.ServerTool { + return issueUpdateTool(t, + "update_issue_milestone", + "Update the milestone of an existing issue.", + "Update Issue Milestone", + map[string]*jsonschema.Schema{ + "milestone": { + Type: "number", + Description: "The milestone number to set on the issue", + Minimum: jsonschema.Ptr(0.0), + }, + }, + []string{"milestone"}, + func(args map[string]any) (*github.IssueRequest, error) { + milestone, err := RequiredParam[float64](args, "milestone") + if err != nil { + return nil, err + } + m := int(milestone) + return &github.IssueRequest{Milestone: &m}, nil + }, + ) +} + +// GranularUpdateIssueType creates a tool to update an issue's type. +func GranularUpdateIssueType(t translations.TranslationHelperFunc) inventory.ServerTool { + return issueUpdateTool(t, + "update_issue_type", + "Update the type of an existing issue (e.g. 'bug', 'feature').", + "Update Issue Type", + map[string]*jsonschema.Schema{ + "issue_type": { + Type: "string", + Description: "The issue type to set", + }, + }, + []string{"issue_type"}, + func(args map[string]any) (*github.IssueRequest, error) { + issueType, err := RequiredParam[string](args, "issue_type") + if err != nil { + return nil, err + } + return &github.IssueRequest{Type: &issueType}, nil + }, + ) +} + +// GranularUpdateIssueState creates a tool to update an issue's state. +func GranularUpdateIssueState(t translations.TranslationHelperFunc) inventory.ServerTool { + return issueUpdateTool(t, + "update_issue_state", + "Update the state of an existing issue (open or closed), with an optional state reason.", + "Update Issue State", + map[string]*jsonschema.Schema{ + "state": { + Type: "string", + Description: "The new state for the issue", + Enum: []any{"open", "closed"}, + }, + "state_reason": { + Type: "string", + Description: "The reason for the state change (only for closed state)", + Enum: []any{"completed", "not_planned", "duplicate"}, + }, + }, + []string{"state"}, + func(args map[string]any) (*github.IssueRequest, error) { + state, err := RequiredParam[string](args, "state") + if err != nil { + return nil, err + } + req := &github.IssueRequest{State: &state} + + stateReason, _ := OptionalParam[string](args, "state_reason") + if stateReason != "" { + req.StateReason = &stateReason + } + return req, nil + }, + ) +} + +// GranularAddSubIssue creates a tool to add a sub-issue. +func GranularAddSubIssue(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataIssuesGranular, + mcp.Tool{ + Name: "add_sub_issue", + Description: t("TOOL_ADD_SUB_ISSUE_DESCRIPTION", "Add a sub-issue to a parent issue."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ADD_SUB_ISSUE_USER_TITLE", "Add Sub-Issue"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "issue_number": { + Type: "number", + Description: "The parent issue number", + Minimum: jsonschema.Ptr(1.0), + }, + "sub_issue_id": { + Type: "number", + Description: "The global node ID of the issue to add as a sub-issue", + }, + "replace_parent": { + Type: "boolean", + Description: "If true, reparent the sub-issue if it already has a parent", + }, + }, + Required: []string{"owner", "repo", "issue_number", "sub_issue_id"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + subIssueID, err := RequiredParam[float64](args, "sub_issue_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + replaceParent, _ := OptionalParam[bool](args, "replace_parent") + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + parentNodeID, err := getGranularIssueNodeID(ctx, gqlClient, owner, repo, issueNumber) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get parent issue", err), nil, nil + } + + var mutation struct { + AddSubIssue struct { + Issue struct { + ID string + Title string + URL string + } + SubIssue struct { + ID string + Title string + URL string + } + } `graphql:"addSubIssue(input: $input)"` + } + + input := GranularAddSubIssueInput{ + IssueID: parentNodeID, + SubIssueID: fmt.Sprintf("%d", int(subIssueID)), + ReplaceParent: githubv4.Boolean(replaceParent), + } + + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultErrorFromErr("failed to add sub-issue", err), nil, nil + } + + r, err := json.Marshal(mutation.AddSubIssue) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GranularRemoveSubIssue creates a tool to remove a sub-issue. +func GranularRemoveSubIssue(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataIssuesGranular, + mcp.Tool{ + Name: "remove_sub_issue", + Description: t("TOOL_REMOVE_SUB_ISSUE_DESCRIPTION", "Remove a sub-issue from a parent issue."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_REMOVE_SUB_ISSUE_USER_TITLE", "Remove Sub-Issue"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(true), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "issue_number": { + Type: "number", + Description: "The parent issue number", + Minimum: jsonschema.Ptr(1.0), + }, + "sub_issue_id": { + Type: "number", + Description: "The global node ID of the sub-issue to remove", + }, + }, + Required: []string{"owner", "repo", "issue_number", "sub_issue_id"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + subIssueID, err := RequiredParam[float64](args, "sub_issue_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + parentNodeID, err := getGranularIssueNodeID(ctx, gqlClient, owner, repo, issueNumber) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get parent issue", err), nil, nil + } + + var mutation struct { + RemoveSubIssue struct { + Issue struct { + ID string + Title string + URL string + } + SubIssue struct { + ID string + Title string + URL string + } + } `graphql:"removeSubIssue(input: $input)"` + } + + input := GranularRemoveSubIssueInput{ + IssueID: parentNodeID, + SubIssueID: fmt.Sprintf("%d", int(subIssueID)), + } + + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultErrorFromErr("failed to remove sub-issue", err), nil, nil + } + + r, err := json.Marshal(mutation.RemoveSubIssue) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GranularReprioritizeSubIssue creates a tool to reorder a sub-issue. +func GranularReprioritizeSubIssue(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataIssuesGranular, + mcp.Tool{ + Name: "reprioritize_sub_issue", + Description: t("TOOL_REPRIORITIZE_SUB_ISSUE_DESCRIPTION", "Reprioritize (reorder) a sub-issue relative to other sub-issues."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_REPRIORITIZE_SUB_ISSUE_USER_TITLE", "Reprioritize Sub-Issue"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "issue_number": { + Type: "number", + Description: "The parent issue number", + Minimum: jsonschema.Ptr(1.0), + }, + "sub_issue_id": { + Type: "number", + Description: "The global node ID of the sub-issue to reorder", + }, + "after_id": { + Type: "number", + Description: "The global node ID of the sub-issue to place this after (optional)", + }, + "before_id": { + Type: "number", + Description: "The global node ID of the sub-issue to place this before (optional)", + }, + }, + Required: []string{"owner", "repo", "issue_number", "sub_issue_id"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + issueNumber, err := RequiredInt(args, "issue_number") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + subIssueID, err := RequiredParam[float64](args, "sub_issue_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + afterID, _ := OptionalParam[float64](args, "after_id") + beforeID, _ := OptionalParam[float64](args, "before_id") + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + parentNodeID, err := getGranularIssueNodeID(ctx, gqlClient, owner, repo, issueNumber) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get parent issue", err), nil, nil + } + + var mutation struct { + ReprioritizeSubIssue struct { + Issue struct { + ID string + Title string + URL string + } + } `graphql:"reprioritizeSubIssue(input: $input)"` + } + + input := GranularReprioritizeSubIssueInput{ + IssueID: parentNodeID, + SubIssueID: fmt.Sprintf("%d", int(subIssueID)), + } + if afterID != 0 { + id := githubv4.ID(fmt.Sprintf("%d", int(afterID))) + input.AfterID = &id + } + if beforeID != 0 { + id := githubv4.ID(fmt.Sprintf("%d", int(beforeID))) + input.BeforeID = &id + } + + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultErrorFromErr("failed to reprioritize sub-issue", err), nil, nil + } + + r, err := json.Marshal(mutation.ReprioritizeSubIssue) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GraphQL input types for sub-issue mutations. + +// GranularAddSubIssueInput is the input for the addSubIssue GraphQL mutation. +type GranularAddSubIssueInput struct { + IssueID string `json:"issueId"` + SubIssueID string `json:"subIssueId"` + ReplaceParent githubv4.Boolean `json:"replaceParent"` +} + +// GranularRemoveSubIssueInput is the input for the removeSubIssue GraphQL mutation. +type GranularRemoveSubIssueInput struct { + IssueID string `json:"issueId"` + SubIssueID string `json:"subIssueId"` +} + +// GranularReprioritizeSubIssueInput is the input for the reprioritizeSubIssue GraphQL mutation. +type GranularReprioritizeSubIssueInput struct { + IssueID string `json:"issueId"` + SubIssueID string `json:"subIssueId"` + AfterID *githubv4.ID `json:"afterId,omitempty"` + BeforeID *githubv4.ID `json:"beforeId,omitempty"` +} + +// getGranularIssueNodeID fetches the GraphQL node ID for an issue. +func getGranularIssueNodeID(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueNumber int) (string, error) { + var query struct { + Repository struct { + Issue struct { + ID string + } `graphql:"issue(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + vars := map[string]any{ + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + "number": githubv4.Int(issueNumber), // #nosec G115 - issue numbers are always small positive integers + } + + if err := gqlClient.Query(ctx, &query, vars); err != nil { + return "", fmt.Errorf("failed to query issue node ID: %w", err) + } + + return query.Repository.Issue.ID, nil +} diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go new file mode 100644 index 000000000..e761516d6 --- /dev/null +++ b/pkg/github/pullrequests_granular.go @@ -0,0 +1,742 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "maps" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/pkg/utils" + gogithub "github.com/google/go-github/v82/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/shurcooL/githubv4" +) + +// prUpdateTool is a helper to create single-field pull request update tools via REST. +func prUpdateTool( + t translations.TranslationHelperFunc, + name, description, title string, + extraProps map[string]*jsonschema.Schema, + extraRequired []string, + buildRequest func(args map[string]any) (*gogithub.PullRequest, error), +) inventory.ServerTool { + props := map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "pullNumber": { + Type: "number", + Description: "The pull request number", + Minimum: jsonschema.Ptr(1.0), + }, + } + maps.Copy(props, extraProps) + + required := append([]string{"owner", "repo", "pullNumber"}, extraRequired...) + + return NewTool( + ToolsetMetadataPullRequestsGranular, + mcp.Tool{ + Name: name, + Description: t("TOOL_"+name+"_DESCRIPTION", description), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_"+name+"_USER_TITLE", title), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: props, + Required: required, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + prReq, err := buildRequest(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + pr, _, err := client.PullRequests.Edit(ctx, owner, repo, pullNumber, prReq) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to update pull request", err), nil, nil + } + + r, err := json.Marshal(pr) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GranularUpdatePullRequestTitle creates a tool to update a PR's title. +func GranularUpdatePullRequestTitle(t translations.TranslationHelperFunc) inventory.ServerTool { + return prUpdateTool(t, + "update_pull_request_title", + "Update the title of an existing pull request.", + "Update Pull Request Title", + map[string]*jsonschema.Schema{ + "title": {Type: "string", Description: "The new title for the pull request"}, + }, + []string{"title"}, + func(args map[string]any) (*gogithub.PullRequest, error) { + title, err := RequiredParam[string](args, "title") + if err != nil { + return nil, err + } + return &gogithub.PullRequest{Title: &title}, nil + }, + ) +} + +// GranularUpdatePullRequestBody creates a tool to update a PR's body. +func GranularUpdatePullRequestBody(t translations.TranslationHelperFunc) inventory.ServerTool { + return prUpdateTool(t, + "update_pull_request_body", + "Update the body description of an existing pull request.", + "Update Pull Request Body", + map[string]*jsonschema.Schema{ + "body": {Type: "string", Description: "The new body content for the pull request"}, + }, + []string{"body"}, + func(args map[string]any) (*gogithub.PullRequest, error) { + body, err := RequiredParam[string](args, "body") + if err != nil { + return nil, err + } + return &gogithub.PullRequest{Body: &body}, nil + }, + ) +} + +// GranularUpdatePullRequestState creates a tool to update a PR's state. +func GranularUpdatePullRequestState(t translations.TranslationHelperFunc) inventory.ServerTool { + return prUpdateTool(t, + "update_pull_request_state", + "Update the state of an existing pull request (open or closed).", + "Update Pull Request State", + map[string]*jsonschema.Schema{ + "state": { + Type: "string", + Description: "The new state for the pull request", + Enum: []any{"open", "closed"}, + }, + }, + []string{"state"}, + func(args map[string]any) (*gogithub.PullRequest, error) { + state, err := RequiredParam[string](args, "state") + if err != nil { + return nil, err + } + return &gogithub.PullRequest{State: &state}, nil + }, + ) +} + +// GranularUpdatePullRequestDraftState creates a tool to toggle draft state. +func GranularUpdatePullRequestDraftState(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataPullRequestsGranular, + mcp.Tool{ + Name: "update_pull_request_draft_state", + Description: t("TOOL_UPDATE_PULL_REQUEST_DRAFT_STATE_DESCRIPTION", "Mark a pull request as draft or ready for review."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_UPDATE_PULL_REQUEST_DRAFT_STATE_USER_TITLE", "Update Pull Request Draft State"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": {Type: "string", Description: "Repository owner (username or organization)"}, + "repo": {Type: "string", Description: "Repository name"}, + "pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)}, + "draft": {Type: "boolean", Description: "Set to true to convert to draft, false to mark as ready for review"}, + }, + Required: []string{"owner", "repo", "pullNumber", "draft"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + draft, err := RequiredParam[bool](args, "draft") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + prNodeID, err := getGranularPullRequestNodeID(ctx, gqlClient, owner, repo, pullNumber) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get pull request", err), nil, nil + } + + if draft { + var mutation struct { + ConvertPullRequestToDraft struct { + PullRequest struct { + ID, Title, URL string + IsDraft bool + } + } `graphql:"convertPullRequestToDraft(input: $input)"` + } + input := map[string]any{"pullRequestId": githubv4.ID(prNodeID)} + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultErrorFromErr("failed to convert to draft", err), nil, nil + } + r, err := json.Marshal(mutation.ConvertPullRequestToDraft.PullRequest) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + } + + var mutation struct { + MarkPullRequestReadyForReview struct { + PullRequest struct { + ID, Title, URL string + IsDraft bool + } + } `graphql:"markPullRequestReadyForReview(input: $input)"` + } + input := map[string]any{"pullRequestId": githubv4.ID(prNodeID)} + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultErrorFromErr("failed to mark ready for review", err), nil, nil + } + r, err := json.Marshal(mutation.MarkPullRequestReadyForReview.PullRequest) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GranularRequestPullRequestReviewers creates a tool to request reviewers. +func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataPullRequestsGranular, + mcp.Tool{ + Name: "request_pull_request_reviewers", + Description: t("TOOL_REQUEST_PULL_REQUEST_REVIEWERS_DESCRIPTION", "Request reviewers for a pull request."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_REQUEST_PULL_REQUEST_REVIEWERS_USER_TITLE", "Request Pull Request Reviewers"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": {Type: "string", Description: "Repository owner (username or organization)"}, + "repo": {Type: "string", Description: "Repository name"}, + "pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)}, + "reviewers": { + Type: "array", + Description: "GitHub usernames to request reviews from", + Items: &jsonschema.Schema{Type: "string"}, + }, + }, + Required: []string{"owner", "repo", "pullNumber", "reviewers"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + rawReviewers, _ := OptionalParam[[]any](args, "reviewers") + if len(rawReviewers) == 0 { + return utils.NewToolResultError("missing required parameter: reviewers"), nil, nil + } + reviewers := make([]string, 0, len(rawReviewers)) + for _, v := range rawReviewers { + if s, ok := v.(string); ok { + reviewers = append(reviewers, s) + } + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + pr, _, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{Reviewers: reviewers}) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to request reviewers", err), nil, nil + } + + r, err := json.Marshal(pr) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GranularCreatePullRequestReview creates a tool to create a PR review. +func GranularCreatePullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataPullRequestsGranular, + mcp.Tool{ + Name: "create_pull_request_review", + Description: t("TOOL_CREATE_PULL_REQUEST_REVIEW_DESCRIPTION", "Create a review on a pull request. If event is provided, the review is submitted immediately; otherwise a pending review is created."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_CREATE_PULL_REQUEST_REVIEW_USER_TITLE", "Create Pull Request Review"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": {Type: "string", Description: "Repository owner (username or organization)"}, + "repo": {Type: "string", Description: "Repository name"}, + "pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)}, + "body": {Type: "string", Description: "The review body text (optional)"}, + "event": {Type: "string", Description: "The review action to perform. If omitted, creates a pending review.", Enum: []any{"APPROVE", "REQUEST_CHANGES", "COMMENT"}}, + "commitID": {Type: "string", Description: "The SHA of the commit to review (optional, defaults to latest)"}, + }, + Required: []string{"owner", "repo", "pullNumber"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + body, _ := OptionalParam[string](args, "body") + event, _ := OptionalParam[string](args, "event") + commitID, _ := OptionalParam[string](args, "commitID") + + reviewReq := &gogithub.PullRequestReviewRequest{} + if body != "" { + reviewReq.Body = &body + } + if event != "" { + reviewReq.Event = &event + } + if commitID != "" { + reviewReq.CommitID = &commitID + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + review, _, err := client.PullRequests.CreateReview(ctx, owner, repo, pullNumber, reviewReq) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create review", err), nil, nil + } + + r, err := json.Marshal(review) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GranularSubmitPendingPullRequestReview creates a tool to submit a pending review. +func GranularSubmitPendingPullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataPullRequestsGranular, + mcp.Tool{ + Name: "submit_pending_pull_request_review", + Description: t("TOOL_SUBMIT_PENDING_PULL_REQUEST_REVIEW_DESCRIPTION", "Submit a pending pull request review."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_SUBMIT_PENDING_PULL_REQUEST_REVIEW_USER_TITLE", "Submit Pending Pull Request Review"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": {Type: "string", Description: "Repository owner (username or organization)"}, + "repo": {Type: "string", Description: "Repository name"}, + "pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)}, + "event": {Type: "string", Description: "The review action to perform", Enum: []any{"APPROVE", "REQUEST_CHANGES", "COMMENT"}}, + "body": {Type: "string", Description: "The review body text (optional)"}, + }, + Required: []string{"owner", "repo", "pullNumber", "event"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + event, err := RequiredParam[string](args, "event") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + body, _ := OptionalParam[string](args, "body") + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + prNodeID, err := getGranularPullRequestNodeID(ctx, gqlClient, owner, repo, pullNumber) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get pull request", err), nil, nil + } + + // Find pending review + var reviewQuery struct { + Repository struct { + PullRequest struct { + Reviews struct { + Nodes []struct { + ID, State string + } + } `graphql:"reviews(first: 1, states: PENDING)"` + } `graphql:"pullRequest(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + vars := map[string]any{ + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + "number": githubv4.Int(pullNumber), // #nosec G115 - PR numbers are always small positive integers + } + if err := gqlClient.Query(ctx, &reviewQuery, vars); err != nil { + return utils.NewToolResultErrorFromErr("failed to find pending review", err), nil, nil + } + + if len(reviewQuery.Repository.PullRequest.Reviews.Nodes) == 0 { + return utils.NewToolResultError("no pending review found for the current user"), nil, nil + } + + reviewID := reviewQuery.Repository.PullRequest.Reviews.Nodes[0].ID + + var mutation struct { + SubmitPullRequestReview struct { + PullRequestReview struct { + ID, State, URL string + } + } `graphql:"submitPullRequestReview(input: $input)"` + } + + submitInput := map[string]any{ + "pullRequestId": githubv4.ID(prNodeID), + "pullRequestReviewId": githubv4.ID(reviewID), + "event": githubv4.PullRequestReviewEvent(event), + } + if body != "" { + submitInput["body"] = githubv4.String(body) + } + + if err := gqlClient.Mutate(ctx, &mutation, submitInput, nil); err != nil { + return utils.NewToolResultErrorFromErr("failed to submit review", err), nil, nil + } + + r, err := json.Marshal(mutation.SubmitPullRequestReview.PullRequestReview) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// GranularDeletePendingPullRequestReview creates a tool to delete a pending review. +func GranularDeletePendingPullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataPullRequestsGranular, + mcp.Tool{ + Name: "delete_pending_pull_request_review", + Description: t("TOOL_DELETE_PENDING_PULL_REQUEST_REVIEW_DESCRIPTION", "Delete a pending pull request review."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_DELETE_PENDING_PULL_REQUEST_REVIEW_USER_TITLE", "Delete Pending Pull Request Review"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(true), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": {Type: "string", Description: "Repository owner (username or organization)"}, + "repo": {Type: "string", Description: "Repository name"}, + "pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)}, + }, + Required: []string{"owner", "repo", "pullNumber"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + // Find pending review + var reviewQuery struct { + Repository struct { + PullRequest struct { + Reviews struct { + Nodes []struct{ ID string } + } `graphql:"reviews(first: 1, states: PENDING)"` + } `graphql:"pullRequest(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + vars := map[string]any{ + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + "number": githubv4.Int(pullNumber), // #nosec G115 - PR numbers are always small positive integers + } + if err := gqlClient.Query(ctx, &reviewQuery, vars); err != nil { + return utils.NewToolResultErrorFromErr("failed to find pending review", err), nil, nil + } + + if len(reviewQuery.Repository.PullRequest.Reviews.Nodes) == 0 { + return utils.NewToolResultError("no pending review found for the current user"), nil, nil + } + + reviewID := reviewQuery.Repository.PullRequest.Reviews.Nodes[0].ID + + var mutation struct { + DeletePullRequestReview struct { + PullRequestReview struct{ ID string } + } `graphql:"deletePullRequestReview(input: $input)"` + } + + input := map[string]any{"pullRequestReviewId": githubv4.ID(reviewID)} + + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultErrorFromErr("failed to delete pending review", err), nil, nil + } + + return utils.NewToolResultText(`{"message": "Pending review deleted successfully"}`), nil, nil + }, + ) +} + +// GranularAddPullRequestReviewComment creates a tool to add a review comment. +func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataPullRequestsGranular, + mcp.Tool{ + Name: "add_pull_request_review_comment", + Description: t("TOOL_ADD_PULL_REQUEST_REVIEW_COMMENT_DESCRIPTION", "Add a review comment to the current user's pending pull request review."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_ADD_PULL_REQUEST_REVIEW_COMMENT_USER_TITLE", "Add Pull Request Review Comment"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(false), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": {Type: "string", Description: "Repository owner (username or organization)"}, + "repo": {Type: "string", Description: "Repository name"}, + "pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)}, + "path": {Type: "string", Description: "The relative path of the file to comment on"}, + "body": {Type: "string", Description: "The comment body"}, + "subjectType": {Type: "string", Description: "The subject type of the comment", Enum: []any{"FILE", "LINE"}}, + "line": {Type: "number", Description: "The line number in the diff to comment on (optional)"}, + "side": {Type: "string", Description: "The side of the diff to comment on (optional)", Enum: []any{"LEFT", "RIGHT"}}, + "startLine": {Type: "number", Description: "The start line of a multi-line comment (optional)"}, + "startSide": {Type: "string", Description: "The start side of a multi-line comment (optional)", Enum: []any{"LEFT", "RIGHT"}}, + }, + Required: []string{"owner", "repo", "pullNumber", "path", "body", "subjectType"}, + }, + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumber, err := RequiredInt(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + path, err := RequiredParam[string](args, "path") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + body, err := RequiredParam[string](args, "body") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + subjectType, err := RequiredParam[string](args, "subjectType") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + line, _ := OptionalParam[float64](args, "line") + side, _ := OptionalParam[string](args, "side") + startLine, _ := OptionalParam[float64](args, "startLine") + startSide, _ := OptionalParam[string](args, "startSide") + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + prNodeID, err := getGranularPullRequestNodeID(ctx, gqlClient, owner, repo, pullNumber) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get pull request", err), nil, nil + } + + var mutation struct { + AddPullRequestReviewThread struct { + Thread struct { + ID string + Comments struct { + Nodes []struct { + ID, Body, URL string + } + } `graphql:"comments(first: 1)"` + } + } `graphql:"addPullRequestReviewThread(input: $input)"` + } + + input := map[string]any{ + "pullRequestId": githubv4.ID(prNodeID), + "path": githubv4.String(path), + "body": githubv4.String(body), + "subjectType": githubv4.PullRequestReviewThreadSubjectType(subjectType), + } + if line != 0 { + input["line"] = githubv4.Int(int(line)) // #nosec G115 + } + if side != "" { + input["side"] = githubv4.DiffSide(side) + } + if startLine != 0 { + input["startLine"] = githubv4.Int(int(startLine)) // #nosec G115 + } + if startSide != "" { + input["startSide"] = githubv4.DiffSide(startSide) + } + + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultErrorFromErr("failed to add review comment", err), nil, nil + } + + r, err := json.Marshal(mutation.AddPullRequestReviewThread) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// getGranularPullRequestNodeID fetches the GraphQL node ID for a pull request. +func getGranularPullRequestNodeID(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, pullNumber int) (string, error) { + var query struct { + Repository struct { + PullRequest struct { + ID string + } `graphql:"pullRequest(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + vars := map[string]any{ + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + "number": githubv4.Int(pullNumber), // #nosec G115 - PR numbers are always small positive integers + } + + if err := gqlClient.Query(ctx, &query, vars); err != nil { + return "", fmt.Errorf("failed to query pull request node ID: %w", err) + } + + return query.Repository.PullRequest.ID, nil +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 3f1c291a7..352c9647d 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -141,6 +141,18 @@ var ( Icon: "copilot", } + ToolsetMetadataIssuesGranular = inventory.ToolsetMetadata{ + ID: "issues_granular", + Description: "Granular issue tools with fine-grained control over individual operations", + Icon: "issue-opened", + } + + ToolsetMetadataPullRequestsGranular = inventory.ToolsetMetadata{ + ID: "pull_requests_granular", + Description: "Granular pull request tools with fine-grained control over individual operations", + Icon: "git-pull-request", + } + // Remote-only toolsets - these are only available in the remote MCP server // but are documented here for consistency and to enable automated documentation. ToolsetMetadataCopilotSpaces = inventory.ToolsetMetadata{ @@ -274,6 +286,30 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GetLabelForLabelsToolset(t), ListLabels(t), LabelWrite(t), + + // Granular issue tools (opt-in, non-default) + GranularCreateIssue(t), + GranularUpdateIssueTitle(t), + GranularUpdateIssueBody(t), + GranularUpdateIssueAssignees(t), + GranularUpdateIssueLabels(t), + GranularUpdateIssueMilestone(t), + GranularUpdateIssueType(t), + GranularUpdateIssueState(t), + GranularAddSubIssue(t), + GranularRemoveSubIssue(t), + GranularReprioritizeSubIssue(t), + + // Granular pull request tools (opt-in, non-default) + GranularUpdatePullRequestTitle(t), + GranularUpdatePullRequestBody(t), + GranularUpdatePullRequestState(t), + GranularUpdatePullRequestDraftState(t), + GranularRequestPullRequestReviewers(t), + GranularCreatePullRequestReview(t), + GranularSubmitPendingPullRequestReview(t), + GranularDeletePendingPullRequestReview(t), + GranularAddPullRequestReviewComment(t), } } From bf189570ac01c7d9f6e863ecadd773a1780a6421 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Wed, 8 Apr 2026 15:43:09 +0100 Subject: [PATCH 2/5] update docs --- README.md | 166 +++++++++++++++ docs/remote-server.md | 2 + pkg/github/granular_tools_test.go | 333 ++++++++++++++++++++++++++---- 3 files changed, 457 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 419f89297..879c5bf3b 100644 --- a/README.md +++ b/README.md @@ -566,11 +566,13 @@ The following sets of tools are available: | logo-gist | `gists` | GitHub Gist related tools | | git-branch | `git` | GitHub Git API related tools for low-level Git operations | | issue-opened | `issues` | GitHub Issues related tools | +| issue-opened | `issues_granular` | Granular issue tools with fine-grained control over individual operations | | tag | `labels` | GitHub Labels related tools | | bell | `notifications` | GitHub Notifications related tools | | organization | `orgs` | GitHub Organization related tools | | project | `projects` | GitHub Projects related tools | | git-pull-request | `pull_requests` | GitHub Pull Request related tools | +| git-pull-request | `pull_requests_granular` | Granular pull request tools with fine-grained control over individual operations | | repo | `repos` | GitHub Repository related tools | | shield-lock | `secret_protection` | Secret protection related tools, such as GitHub Secret Scanning | | shield | `security_advisories` | Security advisories related tools | @@ -903,6 +905,93 @@ The following sets of tools are available:
+issue-opened Issues Granular + +- **add_sub_issue** - Add Sub-Issue + - **Required OAuth Scopes**: `repo` + - `issue_number`: The parent issue number (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `replace_parent`: If true, reparent the sub-issue if it already has a parent (boolean, optional) + - `repo`: Repository name (string, required) + - `sub_issue_id`: The global node ID of the issue to add as a sub-issue (number, required) + +- **create_issue** - Create Issue + - **Required OAuth Scopes**: `repo` + - `body`: Issue body content (optional) (string, optional) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - `title`: Issue title (string, required) + +- **remove_sub_issue** - Remove Sub-Issue + - **Required OAuth Scopes**: `repo` + - `issue_number`: The parent issue number (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - `sub_issue_id`: The global node ID of the sub-issue to remove (number, required) + +- **reprioritize_sub_issue** - Reprioritize Sub-Issue + - **Required OAuth Scopes**: `repo` + - `after_id`: The global node ID of the sub-issue to place this after (optional) (number, optional) + - `before_id`: The global node ID of the sub-issue to place this before (optional) (number, optional) + - `issue_number`: The parent issue number (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - `sub_issue_id`: The global node ID of the sub-issue to reorder (number, required) + +- **update_issue_assignees** - Update Issue Assignees + - **Required OAuth Scopes**: `repo` + - `assignees`: GitHub usernames to assign to this issue (string[], required) + - `issue_number`: The issue number to update (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **update_issue_body** - Update Issue Body + - **Required OAuth Scopes**: `repo` + - `body`: The new body content for the issue (string, required) + - `issue_number`: The issue number to update (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **update_issue_labels** - Update Issue Labels + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `labels`: Labels to apply to this issue (string[], required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **update_issue_milestone** - Update Issue Milestone + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `milestone`: The milestone number to set on the issue (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +- **update_issue_state** - Update Issue State + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - `state`: The new state for the issue (string, required) + - `state_reason`: The reason for the state change (only for closed state) (string, optional) + +- **update_issue_title** - Update Issue Title + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - `title`: The new title for the issue (string, required) + +- **update_issue_type** - Update Issue Type + - **Required OAuth Scopes**: `repo` + - `issue_number`: The issue number to update (number, required) + - `issue_type`: The issue type to set (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + +
+ +
+ tag Labels - **get_label** - Get a specific label from a repository. @@ -1155,6 +1244,83 @@ The following sets of tools are available:
+git-pull-request Pull Requests Granular + +- **add_pull_request_review_comment** - Add Pull Request Review Comment + - **Required OAuth Scopes**: `repo` + - `body`: The comment body (string, required) + - `line`: The line number in the diff to comment on (optional) (number, optional) + - `owner`: Repository owner (username or organization) (string, required) + - `path`: The relative path of the file to comment on (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `side`: The side of the diff to comment on (optional) (string, optional) + - `startLine`: The start line of a multi-line comment (optional) (number, optional) + - `startSide`: The start side of a multi-line comment (optional) (string, optional) + - `subjectType`: The subject type of the comment (string, required) + +- **create_pull_request_review** - Create Pull Request Review + - **Required OAuth Scopes**: `repo` + - `body`: The review body text (optional) (string, optional) + - `commitID`: The SHA of the commit to review (optional, defaults to latest) (string, optional) + - `event`: The review action to perform. If omitted, creates a pending review. (string, optional) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **delete_pending_pull_request_review** - Delete Pending Pull Request Review + - **Required OAuth Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **request_pull_request_reviewers** - Request Pull Request Reviewers + - **Required OAuth Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `reviewers`: GitHub usernames to request reviews from (string[], required) + +- **submit_pending_pull_request_review** - Submit Pending Pull Request Review + - **Required OAuth Scopes**: `repo` + - `body`: The review body text (optional) (string, optional) + - `event`: The review action to perform (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **update_pull_request_body** - Update Pull Request Body + - **Required OAuth Scopes**: `repo` + - `body`: The new body content for the pull request (string, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **update_pull_request_draft_state** - Update Pull Request Draft State + - **Required OAuth Scopes**: `repo` + - `draft`: Set to true to convert to draft, false to mark as ready for review (boolean, required) + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + +- **update_pull_request_state** - Update Pull Request State + - **Required OAuth Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `state`: The new state for the pull request (string, required) + +- **update_pull_request_title** - Update Pull Request Title + - **Required OAuth Scopes**: `repo` + - `owner`: Repository owner (username or organization) (string, required) + - `pullNumber`: The pull request number (number, required) + - `repo`: Repository name (string, required) + - `title`: The new title for the pull request (string, required) + +
+ +
+ repo Repositories - **create_branch** - Create branch diff --git a/docs/remote-server.md b/docs/remote-server.md index 5a82f1c2e..215e81af7 100644 --- a/docs/remote-server.md +++ b/docs/remote-server.md @@ -28,11 +28,13 @@ Below is a table of available toolsets for the remote GitHub MCP Server. Each to | logo-gist
`gists` | GitHub Gist related tools | https://api.githubcopilot.com/mcp/x/gists | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-gists&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgists%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/gists/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-gists&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgists%2Freadonly%22%7D) | | git-branch
`git` | GitHub Git API related tools for low-level Git operations | https://api.githubcopilot.com/mcp/x/git | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-git&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgit%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/git/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-git&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgit%2Freadonly%22%7D) | | issue-opened
`issues` | GitHub Issues related tools | https://api.githubcopilot.com/mcp/x/issues | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/issues/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues%2Freadonly%22%7D) | +| issue-opened
`issues_granular` | Granular issue tools with fine-grained control over individual operations | https://api.githubcopilot.com/mcp/x/issues_granular | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues_granular&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues_granular%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/issues_granular/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues_granular&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues_granular%2Freadonly%22%7D) | | tag
`labels` | GitHub Labels related tools | https://api.githubcopilot.com/mcp/x/labels | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-labels&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Flabels%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/labels/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-labels&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Flabels%2Freadonly%22%7D) | | bell
`notifications` | GitHub Notifications related tools | https://api.githubcopilot.com/mcp/x/notifications | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-notifications&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fnotifications%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/notifications/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-notifications&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fnotifications%2Freadonly%22%7D) | | organization
`orgs` | GitHub Organization related tools | https://api.githubcopilot.com/mcp/x/orgs | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-orgs&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Forgs%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/orgs/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-orgs&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Forgs%2Freadonly%22%7D) | | project
`projects` | GitHub Projects related tools | https://api.githubcopilot.com/mcp/x/projects | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-projects&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fprojects%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/projects/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-projects&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fprojects%2Freadonly%22%7D) | | git-pull-request
`pull_requests` | GitHub Pull Request related tools | https://api.githubcopilot.com/mcp/x/pull_requests | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-pull_requests&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fpull_requests%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/pull_requests/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-pull_requests&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fpull_requests%2Freadonly%22%7D) | +| git-pull-request
`pull_requests_granular` | Granular pull request tools with fine-grained control over individual operations | https://api.githubcopilot.com/mcp/x/pull_requests_granular | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-pull_requests_granular&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fpull_requests_granular%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/pull_requests_granular/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-pull_requests_granular&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fpull_requests_granular%2Freadonly%22%7D) | | repo
`repos` | GitHub Repository related tools | https://api.githubcopilot.com/mcp/x/repos | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-repos&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Frepos%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/repos/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-repos&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Frepos%2Freadonly%22%7D) | | shield-lock
`secret_protection` | Secret protection related tools, such as GitHub Secret Scanning | https://api.githubcopilot.com/mcp/x/secret_protection | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-secret_protection&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fsecret_protection%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/secret_protection/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-secret_protection&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fsecret_protection%2Freadonly%22%7D) | | shield
`security_advisories` | Security advisories related tools | https://api.githubcopilot.com/mcp/x/security_advisories | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-security_advisories&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fsecurity_advisories%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/security_advisories/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-security_advisories&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fsecurity_advisories%2Freadonly%22%7D) | diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 465d7164a..40e131596 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -31,37 +31,51 @@ func TestIssuesGranularToolset(t *testing.T) { toolNames = append(toolNames, tool.Tool.Name) } - assert.Contains(t, toolNames, "create_issue") - assert.Contains(t, toolNames, "update_issue_title") - assert.Contains(t, toolNames, "update_issue_body") - assert.Contains(t, toolNames, "update_issue_assignees") - assert.Contains(t, toolNames, "update_issue_labels") - assert.Contains(t, toolNames, "update_issue_milestone") - assert.Contains(t, toolNames, "update_issue_type") - assert.Contains(t, toolNames, "update_issue_state") - assert.Contains(t, toolNames, "add_sub_issue") - assert.Contains(t, toolNames, "remove_sub_issue") - assert.Contains(t, toolNames, "reprioritize_sub_issue") + expected := []string{ + "create_issue", + "update_issue_title", + "update_issue_body", + "update_issue_assignees", + "update_issue_labels", + "update_issue_milestone", + "update_issue_type", + "update_issue_state", + "add_sub_issue", + "remove_sub_issue", + "reprioritize_sub_issue", + } + for _, name := range expected { + assert.Contains(t, toolNames, name) + } + assert.Len(t, tools, len(expected)) }) t.Run("all tools belong to issues_granular toolset", func(t *testing.T) { - tools := granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) - - for _, tool := range tools { - assert.Equal(t, ToolsetMetadataIssuesGranular.ID, tool.Toolset.ID, "tool %s should belong to issues_granular toolset", tool.Tool.Name) + for _, tool := range granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) { + assert.Equal(t, ToolsetMetadataIssuesGranular.ID, tool.Toolset.ID, "tool %s", tool.Tool.Name) } }) - t.Run("all tools have ReadOnlyHint false", func(t *testing.T) { - tools := granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) - - for _, tool := range tools { + t.Run("all tools are write tools", func(t *testing.T) { + for _, tool := range granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) { assert.False(t, tool.Tool.Annotations.ReadOnlyHint, "tool %s should have ReadOnlyHint=false", tool.Tool.Name) } }) t.Run("toolset is non-default", func(t *testing.T) { - assert.False(t, ToolsetMetadataIssuesGranular.Default, "issues_granular toolset should not be default") + assert.False(t, ToolsetMetadataIssuesGranular.Default) + }) + + t.Run("no duplicate names with issues toolset", func(t *testing.T) { + issueTools := make(map[string]bool) + for _, tool := range AllTools(translations.NullTranslationHelper) { + if tool.Toolset.ID == ToolsetMetadataIssues.ID { + issueTools[tool.Tool.Name] = true + } + } + for _, tool := range granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) { + assert.False(t, issueTools[tool.Tool.Name], "tool %s duplicates a tool in the issues toolset", tool.Tool.Name) + } }) } @@ -74,30 +88,48 @@ func TestPullRequestsGranularToolset(t *testing.T) { toolNames = append(toolNames, tool.Tool.Name) } - assert.Contains(t, toolNames, "update_pull_request_title") - assert.Contains(t, toolNames, "update_pull_request_body") - assert.Contains(t, toolNames, "update_pull_request_state") - assert.Contains(t, toolNames, "update_pull_request_draft_state") - assert.Contains(t, toolNames, "request_pull_request_reviewers") - assert.Contains(t, toolNames, "create_pull_request_review") - assert.Contains(t, toolNames, "submit_pending_pull_request_review") - assert.Contains(t, toolNames, "delete_pending_pull_request_review") - assert.Contains(t, toolNames, "add_pull_request_review_comment") + expected := []string{ + "update_pull_request_title", + "update_pull_request_body", + "update_pull_request_state", + "update_pull_request_draft_state", + "request_pull_request_reviewers", + "create_pull_request_review", + "submit_pending_pull_request_review", + "delete_pending_pull_request_review", + "add_pull_request_review_comment", + } + for _, name := range expected { + assert.Contains(t, toolNames, name) + } + assert.Len(t, tools, len(expected)) }) t.Run("all tools belong to pull_requests_granular toolset", func(t *testing.T) { - tools := granularToolsForToolset(ToolsetMetadataPullRequestsGranular.ID) - - for _, tool := range tools { - assert.Equal(t, ToolsetMetadataPullRequestsGranular.ID, tool.Toolset.ID, "tool %s should belong to pull_requests_granular toolset", tool.Tool.Name) + for _, tool := range granularToolsForToolset(ToolsetMetadataPullRequestsGranular.ID) { + assert.Equal(t, ToolsetMetadataPullRequestsGranular.ID, tool.Toolset.ID, "tool %s", tool.Tool.Name) } }) t.Run("toolset is non-default", func(t *testing.T) { - assert.False(t, ToolsetMetadataPullRequestsGranular.Default, "pull_requests_granular toolset should not be default") + assert.False(t, ToolsetMetadataPullRequestsGranular.Default) + }) + + t.Run("no duplicate names with pull_requests toolset", func(t *testing.T) { + prTools := make(map[string]bool) + for _, tool := range AllTools(translations.NullTranslationHelper) { + if tool.Toolset.ID == ToolsetMetadataPullRequests.ID { + prTools[tool.Tool.Name] = true + } + } + for _, tool := range granularToolsForToolset(ToolsetMetadataPullRequestsGranular.ID) { + assert.False(t, prTools[tool.Tool.Name], "tool %s duplicates a tool in the pull_requests toolset", tool.Tool.Name) + } }) } +// --- Issue granular tool handler tests --- + func TestGranularCreateIssue(t *testing.T) { mockIssue := &gogithub.Issue{ Number: gogithub.Ptr(1), @@ -180,6 +212,51 @@ func TestGranularUpdateIssueTitle(t *testing.T) { assert.False(t, result.IsError) } +func TestGranularUpdateIssueBody(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "body": "Updated body", + }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{ + Number: gogithub.Ptr(1), + Body: gogithub.Ptr("Updated body"), + })), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueBody(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "body": "Updated body", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func TestGranularUpdateIssueAssignees(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "assignees": []any{"user1", "user2"}, + }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueAssignees(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "assignees": []string{"user1", "user2"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + func TestGranularUpdateIssueLabels(t *testing.T) { client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ @@ -201,26 +278,171 @@ func TestGranularUpdateIssueLabels(t *testing.T) { assert.False(t, result.IsError) } -func TestGranularUpdateIssueState(t *testing.T) { +func TestGranularUpdateIssueMilestone(t *testing.T) { client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ - "state": "closed", - "state_reason": "completed", - }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{ - Number: gogithub.Ptr(1), - State: gogithub.Ptr("closed"), - })), + "milestone": float64(5), + }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), })) deps := BaseDeps{Client: client} - serverTool := GranularUpdateIssueState(translations.NullTranslationHelper) + serverTool := GranularUpdateIssueMilestone(translations.NullTranslationHelper) handler := serverTool.Handler(deps) request := createMCPRequest(map[string]any{ "owner": "owner", "repo": "repo", "issue_number": float64(1), - "state": "closed", - "state_reason": "completed", + "milestone": float64(5), + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func TestGranularUpdateIssueType(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{ + "type": "bug", + }).andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueType(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "issue_type": "bug", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func TestGranularUpdateIssueState(t *testing.T) { + tests := []struct { + name string + requestArgs map[string]any + expectedReq map[string]any + }{ + { + name: "close with reason", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "closed", + "state_reason": "completed", + }, + expectedReq: map[string]any{ + "state": "closed", + "state_reason": "completed", + }, + }, + { + name: "reopen without reason", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "state": "open", + }, + expectedReq: map[string]any{ + "state": "open", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). + andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{ + Number: gogithub.Ptr(1), + State: gogithub.Ptr(tc.requestArgs["state"].(string)), + })), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdateIssueState(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) + }) + } +} + +// --- Pull request granular tool handler tests --- + +func TestGranularUpdatePullRequestTitle(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposPullsByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{ + "title": "New PR Title", + }).andThen(mockResponse(t, http.StatusOK, &gogithub.PullRequest{ + Number: gogithub.Ptr(1), + Title: gogithub.Ptr("New PR Title"), + })), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdatePullRequestTitle(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(1), + "title": "New PR Title", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func TestGranularUpdatePullRequestBody(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposPullsByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{ + "body": "Updated description", + }).andThen(mockResponse(t, http.StatusOK, &gogithub.PullRequest{ + Number: gogithub.Ptr(1), + Body: gogithub.Ptr("Updated description"), + })), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdatePullRequestBody(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(1), + "body": "Updated description", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} + +func TestGranularUpdatePullRequestState(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposPullsByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{ + "state": "closed", + }).andThen(mockResponse(t, http.StatusOK, &gogithub.PullRequest{ + Number: gogithub.Ptr(1), + State: gogithub.Ptr("closed"), + })), + })) + deps := BaseDeps{Client: client} + serverTool := GranularUpdatePullRequestState(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(1), + "state": "closed", }) result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) @@ -245,3 +467,26 @@ func TestGranularRequestPullRequestReviewers(t *testing.T) { require.NoError(t, err) assert.False(t, result.IsError) } + +func TestGranularCreatePullRequestReview(t *testing.T) { + client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews": mockResponse(t, http.StatusOK, &gogithub.PullRequestReview{ + ID: gogithub.Ptr(int64(1)), + State: gogithub.Ptr("APPROVED"), + }), + })) + deps := BaseDeps{Client: client} + serverTool := GranularCreatePullRequestReview(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(1), + "body": "LGTM", + "event": "APPROVE", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} From 5878c2187d11fbfdd47502e38125cc50e90e8ec3 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 9 Apr 2026 09:56:20 +0200 Subject: [PATCH 3/5] refactor: reuse existing helpers in granular toolsets Refactor granular issue and PR tools to delegate to existing tested helper functions instead of reimplementing logic from scratch: - Sub-issue tools (add/remove/reprioritize) now delegate to existing REST-based AddSubIssue, RemoveSubIssue, ReprioritizeSubIssue helpers - PR review tools (create/submit/delete) now delegate to existing CreatePullRequestReview, SubmitPendingPullRequestReview, DeletePendingPullRequestReview helpers (fixes viewer filtering bug) - Review comment tool now uses viewer-safe pattern from AddCommentToPendingReview (query viewer, filter by author, validate PENDING state, pass PullRequestReviewID) - Fix milestone param to use RequiredInt instead of float64 cast - Fix line/startLine params to use OptionalIntParam - Draft state tool uses typed GraphQL inputs matching existing patterns - Remove duplicate GraphQL types and helper functions - Add toolsnap tests for all 20 granular tools - Update generated docs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 12 +- .../add_pull_request_review_comment.snap | 75 ++++ pkg/github/__toolsnaps__/add_sub_issue.snap | 41 +++ pkg/github/__toolsnaps__/create_issue.snap | 33 +- .../create_pull_request_review.snap | 49 +++ .../delete_pending_pull_request_review.snap | 32 ++ .../__toolsnaps__/remove_sub_issue.snap | 37 ++ .../__toolsnaps__/reprioritize_sub_issue.snap | 45 +++ .../request_pull_request_reviewers.snap | 40 ++ .../submit_pending_pull_request_review.snap | 46 +++ .../__toolsnaps__/update_issue_assignees.snap | 40 ++ .../__toolsnaps__/update_issue_body.snap | 37 ++ .../__toolsnaps__/update_issue_labels.snap | 40 ++ .../__toolsnaps__/update_issue_milestone.snap | 38 ++ .../__toolsnaps__/update_issue_state.snap | 50 +++ .../__toolsnaps__/update_issue_title.snap | 37 ++ .../__toolsnaps__/update_issue_type.snap | 37 ++ .../update_pull_request_body.snap | 37 ++ .../update_pull_request_draft_state.snap | 37 ++ .../update_pull_request_state.snap | 41 +++ .../update_pull_request_title.snap | 37 ++ pkg/github/granular_tools_test.go | 84 ++++- pkg/github/issues_granular.go | 198 ++-------- pkg/github/pullrequests_granular.go | 345 ++++++++---------- 24 files changed, 1058 insertions(+), 410 deletions(-) create mode 100644 pkg/github/__toolsnaps__/add_pull_request_review_comment.snap create mode 100644 pkg/github/__toolsnaps__/add_sub_issue.snap create mode 100644 pkg/github/__toolsnaps__/create_pull_request_review.snap create mode 100644 pkg/github/__toolsnaps__/delete_pending_pull_request_review.snap create mode 100644 pkg/github/__toolsnaps__/remove_sub_issue.snap create mode 100644 pkg/github/__toolsnaps__/reprioritize_sub_issue.snap create mode 100644 pkg/github/__toolsnaps__/request_pull_request_reviewers.snap create mode 100644 pkg/github/__toolsnaps__/submit_pending_pull_request_review.snap create mode 100644 pkg/github/__toolsnaps__/update_issue_assignees.snap create mode 100644 pkg/github/__toolsnaps__/update_issue_body.snap create mode 100644 pkg/github/__toolsnaps__/update_issue_labels.snap create mode 100644 pkg/github/__toolsnaps__/update_issue_milestone.snap create mode 100644 pkg/github/__toolsnaps__/update_issue_state.snap create mode 100644 pkg/github/__toolsnaps__/update_issue_title.snap create mode 100644 pkg/github/__toolsnaps__/update_issue_type.snap create mode 100644 pkg/github/__toolsnaps__/update_pull_request_body.snap create mode 100644 pkg/github/__toolsnaps__/update_pull_request_draft_state.snap create mode 100644 pkg/github/__toolsnaps__/update_pull_request_state.snap create mode 100644 pkg/github/__toolsnaps__/update_pull_request_title.snap diff --git a/README.md b/README.md index 879c5bf3b..43ecf36e3 100644 --- a/README.md +++ b/README.md @@ -913,7 +913,7 @@ The following sets of tools are available: - `owner`: Repository owner (username or organization) (string, required) - `replace_parent`: If true, reparent the sub-issue if it already has a parent (boolean, optional) - `repo`: Repository name (string, required) - - `sub_issue_id`: The global node ID of the issue to add as a sub-issue (number, required) + - `sub_issue_id`: The ID of the sub-issue to add. ID is not the same as issue number (number, required) - **create_issue** - Create Issue - **Required OAuth Scopes**: `repo` @@ -927,16 +927,16 @@ The following sets of tools are available: - `issue_number`: The parent issue number (number, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - - `sub_issue_id`: The global node ID of the sub-issue to remove (number, required) + - `sub_issue_id`: The ID of the sub-issue to remove. ID is not the same as issue number (number, required) - **reprioritize_sub_issue** - Reprioritize Sub-Issue - **Required OAuth Scopes**: `repo` - - `after_id`: The global node ID of the sub-issue to place this after (optional) (number, optional) - - `before_id`: The global node ID of the sub-issue to place this before (optional) (number, optional) + - `after_id`: The ID of the sub-issue to place this after (either after_id OR before_id should be specified) (number, optional) + - `before_id`: The ID of the sub-issue to place this before (either after_id OR before_id should be specified) (number, optional) - `issue_number`: The parent issue number (number, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) - - `sub_issue_id`: The global node ID of the sub-issue to reorder (number, required) + - `sub_issue_id`: The ID of the sub-issue to reorder. ID is not the same as issue number (number, required) - **update_issue_assignees** - Update Issue Assignees - **Required OAuth Scopes**: `repo` @@ -962,7 +962,7 @@ The following sets of tools are available: - **update_issue_milestone** - Update Issue Milestone - **Required OAuth Scopes**: `repo` - `issue_number`: The issue number to update (number, required) - - `milestone`: The milestone number to set on the issue (number, required) + - `milestone`: The milestone number to set on the issue (integer, required) - `owner`: Repository owner (username or organization) (string, required) - `repo`: Repository name (string, required) diff --git a/pkg/github/__toolsnaps__/add_pull_request_review_comment.snap b/pkg/github/__toolsnaps__/add_pull_request_review_comment.snap new file mode 100644 index 000000000..1e27c5645 --- /dev/null +++ b/pkg/github/__toolsnaps__/add_pull_request_review_comment.snap @@ -0,0 +1,75 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Add Pull Request Review Comment" + }, + "description": "Add a review comment to the current user's pending pull request review.", + "inputSchema": { + "properties": { + "body": { + "description": "The comment body", + "type": "string" + }, + "line": { + "description": "The line number in the diff to comment on (optional)", + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "path": { + "description": "The relative path of the file to comment on", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "side": { + "description": "The side of the diff to comment on (optional)", + "enum": [ + "LEFT", + "RIGHT" + ], + "type": "string" + }, + "startLine": { + "description": "The start line of a multi-line comment (optional)", + "type": "number" + }, + "startSide": { + "description": "The start side of a multi-line comment (optional)", + "enum": [ + "LEFT", + "RIGHT" + ], + "type": "string" + }, + "subjectType": { + "description": "The subject type of the comment", + "enum": [ + "FILE", + "LINE" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "path", + "body", + "subjectType" + ], + "type": "object" + }, + "name": "add_pull_request_review_comment" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/add_sub_issue.snap b/pkg/github/__toolsnaps__/add_sub_issue.snap new file mode 100644 index 000000000..ef9df400c --- /dev/null +++ b/pkg/github/__toolsnaps__/add_sub_issue.snap @@ -0,0 +1,41 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Add Sub-Issue" + }, + "description": "Add a sub-issue to a parent issue.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The parent issue number", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "replace_parent": { + "description": "If true, reparent the sub-issue if it already has a parent", + "type": "boolean" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "sub_issue_id": { + "description": "The ID of the sub-issue to add. ID is not the same as issue number", + "type": "number" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "sub_issue_id" + ], + "type": "object" + }, + "name": "add_sub_issue" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/create_issue.snap b/pkg/github/__toolsnaps__/create_issue.snap index d11c41c0e..51923c47c 100644 --- a/pkg/github/__toolsnaps__/create_issue.snap +++ b/pkg/github/__toolsnaps__/create_issue.snap @@ -1,35 +1,18 @@ { "annotations": { - "title": "Open new issue", - "readOnlyHint": false + "destructiveHint": false, + "openWorldHint": true, + "title": "Create Issue" }, - "description": "Create a new issue in a GitHub repository.", + "description": "Create a new issue in a GitHub repository with a title and optional body.", "inputSchema": { "properties": { - "assignees": { - "description": "Usernames to assign to this issue", - "items": { - "type": "string" - }, - "type": "array" - }, "body": { - "description": "Issue body content", + "description": "Issue body content (optional)", "type": "string" }, - "labels": { - "description": "Labels to apply to this issue", - "items": { - "type": "string" - }, - "type": "array" - }, - "milestone": { - "description": "Milestone number", - "type": "number" - }, "owner": { - "description": "Repository owner", + "description": "Repository owner (username or organization)", "type": "string" }, "repo": { @@ -39,10 +22,6 @@ "title": { "description": "Issue title", "type": "string" - }, - "type": { - "description": "Type of this issue", - "type": "string" } }, "required": [ diff --git a/pkg/github/__toolsnaps__/create_pull_request_review.snap b/pkg/github/__toolsnaps__/create_pull_request_review.snap new file mode 100644 index 000000000..1986b2cff --- /dev/null +++ b/pkg/github/__toolsnaps__/create_pull_request_review.snap @@ -0,0 +1,49 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Create Pull Request Review" + }, + "description": "Create a review on a pull request. If event is provided, the review is submitted immediately; otherwise a pending review is created.", + "inputSchema": { + "properties": { + "body": { + "description": "The review body text (optional)", + "type": "string" + }, + "commitID": { + "description": "The SHA of the commit to review (optional, defaults to latest)", + "type": "string" + }, + "event": { + "description": "The review action to perform. If omitted, creates a pending review.", + "enum": [ + "APPROVE", + "REQUEST_CHANGES", + "COMMENT" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber" + ], + "type": "object" + }, + "name": "create_pull_request_review" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/delete_pending_pull_request_review.snap b/pkg/github/__toolsnaps__/delete_pending_pull_request_review.snap new file mode 100644 index 000000000..b457e415a --- /dev/null +++ b/pkg/github/__toolsnaps__/delete_pending_pull_request_review.snap @@ -0,0 +1,32 @@ +{ + "annotations": { + "destructiveHint": true, + "openWorldHint": true, + "title": "Delete Pending Pull Request Review" + }, + "description": "Delete a pending pull request review.", + "inputSchema": { + "properties": { + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber" + ], + "type": "object" + }, + "name": "delete_pending_pull_request_review" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/remove_sub_issue.snap b/pkg/github/__toolsnaps__/remove_sub_issue.snap new file mode 100644 index 000000000..31fdcbb3e --- /dev/null +++ b/pkg/github/__toolsnaps__/remove_sub_issue.snap @@ -0,0 +1,37 @@ +{ + "annotations": { + "destructiveHint": true, + "openWorldHint": true, + "title": "Remove Sub-Issue" + }, + "description": "Remove a sub-issue from a parent issue.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The parent issue number", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "sub_issue_id": { + "description": "The ID of the sub-issue to remove. ID is not the same as issue number", + "type": "number" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "sub_issue_id" + ], + "type": "object" + }, + "name": "remove_sub_issue" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/reprioritize_sub_issue.snap b/pkg/github/__toolsnaps__/reprioritize_sub_issue.snap new file mode 100644 index 000000000..d4e1ea4be --- /dev/null +++ b/pkg/github/__toolsnaps__/reprioritize_sub_issue.snap @@ -0,0 +1,45 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Reprioritize Sub-Issue" + }, + "description": "Reprioritize (reorder) a sub-issue relative to other sub-issues.", + "inputSchema": { + "properties": { + "after_id": { + "description": "The ID of the sub-issue to place this after (either after_id OR before_id should be specified)", + "type": "number" + }, + "before_id": { + "description": "The ID of the sub-issue to place this before (either after_id OR before_id should be specified)", + "type": "number" + }, + "issue_number": { + "description": "The parent issue number", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "sub_issue_id": { + "description": "The ID of the sub-issue to reorder. ID is not the same as issue number", + "type": "number" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "sub_issue_id" + ], + "type": "object" + }, + "name": "reprioritize_sub_issue" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap new file mode 100644 index 000000000..67b701447 --- /dev/null +++ b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Request Pull Request Reviewers" + }, + "description": "Request reviewers for a pull request.", + "inputSchema": { + "properties": { + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "reviewers": { + "description": "GitHub usernames to request reviews from", + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "reviewers" + ], + "type": "object" + }, + "name": "request_pull_request_reviewers" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/submit_pending_pull_request_review.snap b/pkg/github/__toolsnaps__/submit_pending_pull_request_review.snap new file mode 100644 index 000000000..81223e2a9 --- /dev/null +++ b/pkg/github/__toolsnaps__/submit_pending_pull_request_review.snap @@ -0,0 +1,46 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Submit Pending Pull Request Review" + }, + "description": "Submit a pending pull request review.", + "inputSchema": { + "properties": { + "body": { + "description": "The review body text (optional)", + "type": "string" + }, + "event": { + "description": "The review action to perform", + "enum": [ + "APPROVE", + "REQUEST_CHANGES", + "COMMENT" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "event" + ], + "type": "object" + }, + "name": "submit_pending_pull_request_review" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_issue_assignees.snap b/pkg/github/__toolsnaps__/update_issue_assignees.snap new file mode 100644 index 000000000..9c7261c9a --- /dev/null +++ b/pkg/github/__toolsnaps__/update_issue_assignees.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Issue Assignees" + }, + "description": "Update the assignees of an existing issue. This replaces the current assignees with the provided list.", + "inputSchema": { + "properties": { + "assignees": { + "description": "GitHub usernames to assign to this issue", + "items": { + "type": "string" + }, + "type": "array" + }, + "issue_number": { + "description": "The issue number to update", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "assignees" + ], + "type": "object" + }, + "name": "update_issue_assignees" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_issue_body.snap b/pkg/github/__toolsnaps__/update_issue_body.snap new file mode 100644 index 000000000..c54d69172 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_issue_body.snap @@ -0,0 +1,37 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Issue Body" + }, + "description": "Update the body content of an existing issue.", + "inputSchema": { + "properties": { + "body": { + "description": "The new body content for the issue", + "type": "string" + }, + "issue_number": { + "description": "The issue number to update", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "body" + ], + "type": "object" + }, + "name": "update_issue_body" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_issue_labels.snap b/pkg/github/__toolsnaps__/update_issue_labels.snap new file mode 100644 index 000000000..3acf98d93 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_issue_labels.snap @@ -0,0 +1,40 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Issue Labels" + }, + "description": "Update the labels of an existing issue. This replaces the current labels with the provided list.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The issue number to update", + "minimum": 1, + "type": "number" + }, + "labels": { + "description": "Labels to apply to this issue", + "items": { + "type": "string" + }, + "type": "array" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "labels" + ], + "type": "object" + }, + "name": "update_issue_labels" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_issue_milestone.snap b/pkg/github/__toolsnaps__/update_issue_milestone.snap new file mode 100644 index 000000000..62e1d68a8 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_issue_milestone.snap @@ -0,0 +1,38 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Issue Milestone" + }, + "description": "Update the milestone of an existing issue.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The issue number to update", + "minimum": 1, + "type": "number" + }, + "milestone": { + "description": "The milestone number to set on the issue", + "minimum": 0, + "type": "integer" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "milestone" + ], + "type": "object" + }, + "name": "update_issue_milestone" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_issue_state.snap b/pkg/github/__toolsnaps__/update_issue_state.snap new file mode 100644 index 000000000..b14d737b7 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_issue_state.snap @@ -0,0 +1,50 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Issue State" + }, + "description": "Update the state of an existing issue (open or closed), with an optional state reason.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The issue number to update", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "state": { + "description": "The new state for the issue", + "enum": [ + "open", + "closed" + ], + "type": "string" + }, + "state_reason": { + "description": "The reason for the state change (only for closed state)", + "enum": [ + "completed", + "not_planned", + "duplicate" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "state" + ], + "type": "object" + }, + "name": "update_issue_state" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_issue_title.snap b/pkg/github/__toolsnaps__/update_issue_title.snap new file mode 100644 index 000000000..825fab065 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_issue_title.snap @@ -0,0 +1,37 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Issue Title" + }, + "description": "Update the title of an existing issue.", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The issue number to update", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "title": { + "description": "The new title for the issue", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "title" + ], + "type": "object" + }, + "name": "update_issue_title" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_issue_type.snap b/pkg/github/__toolsnaps__/update_issue_type.snap new file mode 100644 index 000000000..6354a42e1 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_issue_type.snap @@ -0,0 +1,37 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Issue Type" + }, + "description": "Update the type of an existing issue (e.g. 'bug', 'feature').", + "inputSchema": { + "properties": { + "issue_number": { + "description": "The issue number to update", + "minimum": 1, + "type": "number" + }, + "issue_type": { + "description": "The issue type to set", + "type": "string" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "issue_type" + ], + "type": "object" + }, + "name": "update_issue_type" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_pull_request_body.snap b/pkg/github/__toolsnaps__/update_pull_request_body.snap new file mode 100644 index 000000000..1e6040bd4 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_pull_request_body.snap @@ -0,0 +1,37 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Pull Request Body" + }, + "description": "Update the body description of an existing pull request.", + "inputSchema": { + "properties": { + "body": { + "description": "The new body content for the pull request", + "type": "string" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "body" + ], + "type": "object" + }, + "name": "update_pull_request_body" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_pull_request_draft_state.snap b/pkg/github/__toolsnaps__/update_pull_request_draft_state.snap new file mode 100644 index 000000000..2a397951a --- /dev/null +++ b/pkg/github/__toolsnaps__/update_pull_request_draft_state.snap @@ -0,0 +1,37 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Pull Request Draft State" + }, + "description": "Mark a pull request as draft or ready for review.", + "inputSchema": { + "properties": { + "draft": { + "description": "Set to true to convert to draft, false to mark as ready for review", + "type": "boolean" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "draft" + ], + "type": "object" + }, + "name": "update_pull_request_draft_state" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_pull_request_state.snap b/pkg/github/__toolsnaps__/update_pull_request_state.snap new file mode 100644 index 000000000..9cbdb8112 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_pull_request_state.snap @@ -0,0 +1,41 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Pull Request State" + }, + "description": "Update the state of an existing pull request (open or closed).", + "inputSchema": { + "properties": { + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "state": { + "description": "The new state for the pull request", + "enum": [ + "open", + "closed" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "state" + ], + "type": "object" + }, + "name": "update_pull_request_state" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/update_pull_request_title.snap b/pkg/github/__toolsnaps__/update_pull_request_title.snap new file mode 100644 index 000000000..e6398ed40 --- /dev/null +++ b/pkg/github/__toolsnaps__/update_pull_request_title.snap @@ -0,0 +1,37 @@ +{ + "annotations": { + "destructiveHint": false, + "openWorldHint": true, + "title": "Update Pull Request Title" + }, + "description": "Update the title of an existing pull request.", + "inputSchema": { + "properties": { + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "pullNumber": { + "description": "The pull request number", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "title": { + "description": "The new title for the pull request", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "pullNumber", + "title" + ], + "type": "object" + }, + "name": "update_pull_request_title" +} \ No newline at end of file diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 40e131596..231303440 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -5,9 +5,12 @@ import ( "net/http" "testing" + "github.com/github/github-mcp-server/internal/githubv4mock" + "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/translations" gogithub "github.com/google/go-github/v82/github" + "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -22,6 +25,39 @@ func granularToolsForToolset(id inventory.ToolsetID) []inventory.ServerTool { return result } +func TestGranularToolSnaps(t *testing.T) { + // Test toolsnaps for all granular tools + toolConstructors := []func(translations.TranslationHelperFunc) inventory.ServerTool{ + GranularCreateIssue, + GranularUpdateIssueTitle, + GranularUpdateIssueBody, + GranularUpdateIssueAssignees, + GranularUpdateIssueLabels, + GranularUpdateIssueMilestone, + GranularUpdateIssueType, + GranularUpdateIssueState, + GranularAddSubIssue, + GranularRemoveSubIssue, + GranularReprioritizeSubIssue, + GranularUpdatePullRequestTitle, + GranularUpdatePullRequestBody, + GranularUpdatePullRequestState, + GranularUpdatePullRequestDraftState, + GranularRequestPullRequestReviewers, + GranularCreatePullRequestReview, + GranularSubmitPendingPullRequestReview, + GranularDeletePendingPullRequestReview, + GranularAddPullRequestReviewComment, + } + + for _, constructor := range toolConstructors { + serverTool := constructor(translations.NullTranslationHelper) + t.Run(serverTool.Tool.Name, func(t *testing.T) { + require.NoError(t, toolsnaps.Test(serverTool.Tool.Name, serverTool.Tool)) + }) + } +} + func TestIssuesGranularToolset(t *testing.T) { t.Run("toolset contains expected tools", func(t *testing.T) { tools := granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) @@ -469,13 +505,47 @@ func TestGranularRequestPullRequestReviewers(t *testing.T) { } func TestGranularCreatePullRequestReview(t *testing.T) { - client := gogithub.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - "POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews": mockResponse(t, http.StatusOK, &gogithub.PullRequestReview{ - ID: gogithub.Ptr(int64(1)), - State: gogithub.Ptr("APPROVED"), - }), - })) - deps := BaseDeps{Client: client} + mockedClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + PullRequest struct { + ID githubv4.ID + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "prNum": githubv4.Int(1), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "pullRequest": map[string]any{ + "id": "PR_123", + }, + }, + }), + ), + githubv4mock.NewMutationMatcher( + struct { + AddPullRequestReview struct { + PullRequestReview struct { + ID githubv4.ID + } + } `graphql:"addPullRequestReview(input: $input)"` + }{}, + githubv4.AddPullRequestReviewInput{ + PullRequestID: githubv4.ID("PR_123"), + Body: githubv4.NewString("LGTM"), + Event: githubv4mock.Ptr(githubv4.PullRequestReviewEventApprove), + }, + nil, + githubv4mock.DataResponse(map[string]any{}), + ), + ) + gqlClient := githubv4.NewClient(mockedClient) + deps := BaseDeps{GQLClient: gqlClient} serverTool := GranularCreatePullRequestReview(translations.NullTranslationHelper) handler := serverTool.Handler(deps) diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index ae267d79d..7dd157839 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -13,7 +13,6 @@ import ( "github.com/google/go-github/v82/github" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "github.com/shurcooL/githubv4" ) // issueUpdateTool is a helper to create single-field issue update tools. @@ -285,19 +284,18 @@ func GranularUpdateIssueMilestone(t translations.TranslationHelperFunc) inventor "Update Issue Milestone", map[string]*jsonschema.Schema{ "milestone": { - Type: "number", + Type: "integer", Description: "The milestone number to set on the issue", Minimum: jsonschema.Ptr(0.0), }, }, []string{"milestone"}, func(args map[string]any) (*github.IssueRequest, error) { - milestone, err := RequiredParam[float64](args, "milestone") + milestone, err := RequiredInt(args, "milestone") if err != nil { return nil, err } - m := int(milestone) - return &github.IssueRequest{Milestone: &m}, nil + return &github.IssueRequest{Milestone: &milestone}, nil }, ) } @@ -391,7 +389,7 @@ func GranularAddSubIssue(t translations.TranslationHelperFunc) inventory.ServerT }, "sub_issue_id": { Type: "number", - Description: "The global node ID of the issue to add as a sub-issue", + Description: "The ID of the sub-issue to add. ID is not the same as issue number", }, "replace_parent": { Type: "boolean", @@ -415,52 +413,19 @@ func GranularAddSubIssue(t translations.TranslationHelperFunc) inventory.ServerT if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - subIssueID, err := RequiredParam[float64](args, "sub_issue_id") + subIssueID, err := RequiredInt(args, "sub_issue_id") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } replaceParent, _ := OptionalParam[bool](args, "replace_parent") - gqlClient, err := deps.GetGQLClient(ctx) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil - } - - parentNodeID, err := getGranularIssueNodeID(ctx, gqlClient, owner, repo, issueNumber) + client, err := deps.GetClient(ctx) if err != nil { - return utils.NewToolResultErrorFromErr("failed to get parent issue", err), nil, nil - } - - var mutation struct { - AddSubIssue struct { - Issue struct { - ID string - Title string - URL string - } - SubIssue struct { - ID string - Title string - URL string - } - } `graphql:"addSubIssue(input: $input)"` - } - - input := GranularAddSubIssueInput{ - IssueID: parentNodeID, - SubIssueID: fmt.Sprintf("%d", int(subIssueID)), - ReplaceParent: githubv4.Boolean(replaceParent), - } - - if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { - return utils.NewToolResultErrorFromErr("failed to add sub-issue", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - r, err := json.Marshal(mutation.AddSubIssue) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil + result, err := AddSubIssue(ctx, client, owner, repo, issueNumber, subIssueID, replaceParent) + return result, nil, err }, ) } @@ -496,7 +461,7 @@ func GranularRemoveSubIssue(t translations.TranslationHelperFunc) inventory.Serv }, "sub_issue_id": { Type: "number", - Description: "The global node ID of the sub-issue to remove", + Description: "The ID of the sub-issue to remove. ID is not the same as issue number", }, }, Required: []string{"owner", "repo", "issue_number", "sub_issue_id"}, @@ -516,50 +481,18 @@ func GranularRemoveSubIssue(t translations.TranslationHelperFunc) inventory.Serv if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - subIssueID, err := RequiredParam[float64](args, "sub_issue_id") + subIssueID, err := RequiredInt(args, "sub_issue_id") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - gqlClient, err := deps.GetGQLClient(ctx) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil - } - - parentNodeID, err := getGranularIssueNodeID(ctx, gqlClient, owner, repo, issueNumber) + client, err := deps.GetClient(ctx) if err != nil { - return utils.NewToolResultErrorFromErr("failed to get parent issue", err), nil, nil - } - - var mutation struct { - RemoveSubIssue struct { - Issue struct { - ID string - Title string - URL string - } - SubIssue struct { - ID string - Title string - URL string - } - } `graphql:"removeSubIssue(input: $input)"` - } - - input := GranularRemoveSubIssueInput{ - IssueID: parentNodeID, - SubIssueID: fmt.Sprintf("%d", int(subIssueID)), - } - - if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { - return utils.NewToolResultErrorFromErr("failed to remove sub-issue", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - r, err := json.Marshal(mutation.RemoveSubIssue) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil + result, err := RemoveSubIssue(ctx, client, owner, repo, issueNumber, subIssueID) + return result, nil, err }, ) } @@ -595,15 +528,15 @@ func GranularReprioritizeSubIssue(t translations.TranslationHelperFunc) inventor }, "sub_issue_id": { Type: "number", - Description: "The global node ID of the sub-issue to reorder", + Description: "The ID of the sub-issue to reorder. ID is not the same as issue number", }, "after_id": { Type: "number", - Description: "The global node ID of the sub-issue to place this after (optional)", + Description: "The ID of the sub-issue to place this after (either after_id OR before_id should be specified)", }, "before_id": { Type: "number", - Description: "The global node ID of the sub-issue to place this before (optional)", + Description: "The ID of the sub-issue to place this before (either after_id OR before_id should be specified)", }, }, Required: []string{"owner", "repo", "issue_number", "sub_issue_id"}, @@ -623,101 +556,26 @@ func GranularReprioritizeSubIssue(t translations.TranslationHelperFunc) inventor if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - subIssueID, err := RequiredParam[float64](args, "sub_issue_id") + subIssueID, err := RequiredInt(args, "sub_issue_id") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - afterID, _ := OptionalParam[float64](args, "after_id") - beforeID, _ := OptionalParam[float64](args, "before_id") - - gqlClient, err := deps.GetGQLClient(ctx) + afterID, err := OptionalIntParam(args, "after_id") if err != nil { - return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } - - parentNodeID, err := getGranularIssueNodeID(ctx, gqlClient, owner, repo, issueNumber) + beforeID, err := OptionalIntParam(args, "before_id") if err != nil { - return utils.NewToolResultErrorFromErr("failed to get parent issue", err), nil, nil - } - - var mutation struct { - ReprioritizeSubIssue struct { - Issue struct { - ID string - Title string - URL string - } - } `graphql:"reprioritizeSubIssue(input: $input)"` - } - - input := GranularReprioritizeSubIssueInput{ - IssueID: parentNodeID, - SubIssueID: fmt.Sprintf("%d", int(subIssueID)), - } - if afterID != 0 { - id := githubv4.ID(fmt.Sprintf("%d", int(afterID))) - input.AfterID = &id - } - if beforeID != 0 { - id := githubv4.ID(fmt.Sprintf("%d", int(beforeID))) - input.BeforeID = &id - } - - if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { - return utils.NewToolResultErrorFromErr("failed to reprioritize sub-issue", err), nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } - r, err := json.Marshal(mutation.ReprioritizeSubIssue) + client, err := deps.GetClient(ctx) if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - return utils.NewToolResultText(string(r)), nil, nil + + result, err := ReprioritizeSubIssue(ctx, client, owner, repo, issueNumber, subIssueID, afterID, beforeID) + return result, nil, err }, ) } - -// GraphQL input types for sub-issue mutations. - -// GranularAddSubIssueInput is the input for the addSubIssue GraphQL mutation. -type GranularAddSubIssueInput struct { - IssueID string `json:"issueId"` - SubIssueID string `json:"subIssueId"` - ReplaceParent githubv4.Boolean `json:"replaceParent"` -} - -// GranularRemoveSubIssueInput is the input for the removeSubIssue GraphQL mutation. -type GranularRemoveSubIssueInput struct { - IssueID string `json:"issueId"` - SubIssueID string `json:"subIssueId"` -} - -// GranularReprioritizeSubIssueInput is the input for the reprioritizeSubIssue GraphQL mutation. -type GranularReprioritizeSubIssueInput struct { - IssueID string `json:"issueId"` - SubIssueID string `json:"subIssueId"` - AfterID *githubv4.ID `json:"afterId,omitempty"` - BeforeID *githubv4.ID `json:"beforeId,omitempty"` -} - -// getGranularIssueNodeID fetches the GraphQL node ID for an issue. -func getGranularIssueNodeID(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueNumber int) (string, error) { - var query struct { - Repository struct { - Issue struct { - ID string - } `graphql:"issue(number: $number)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - vars := map[string]any{ - "owner": githubv4.String(owner), - "name": githubv4.String(repo), - "number": githubv4.Int(issueNumber), // #nosec G115 - issue numbers are always small positive integers - } - - if err := gqlClient.Query(ctx, &query, vars); err != nil { - return "", fmt.Errorf("failed to query issue node ID: %w", err) - } - - return query.Repository.Issue.ID, nil -} diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index e761516d6..b694ff0be 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -6,6 +6,7 @@ import ( "fmt" "maps" + ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" @@ -211,48 +212,53 @@ func GranularUpdatePullRequestDraftState(t translations.TranslationHelperFunc) i return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil } - prNodeID, err := getGranularPullRequestNodeID(ctx, gqlClient, owner, repo, pullNumber) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to get pull request", err), nil, nil + // Get PR node ID + var prQuery struct { + Repository struct { + PullRequest struct { + ID githubv4.ID + } `graphql:"pullRequest(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + if err := gqlClient.Query(ctx, &prQuery, map[string]any{ + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + "number": githubv4.Int(pullNumber), // #nosec G115 - PR numbers are always small positive integers + }); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to get pull request", err), nil, nil } if draft { var mutation struct { ConvertPullRequestToDraft struct { PullRequest struct { - ID, Title, URL string - IsDraft bool + ID githubv4.ID + IsDraft githubv4.Boolean } } `graphql:"convertPullRequestToDraft(input: $input)"` } - input := map[string]any{"pullRequestId": githubv4.ID(prNodeID)} - if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { - return utils.NewToolResultErrorFromErr("failed to convert to draft", err), nil, nil + if err := gqlClient.Mutate(ctx, &mutation, githubv4.ConvertPullRequestToDraftInput{ + PullRequestID: prQuery.Repository.PullRequest.ID, + }, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to convert to draft", err), nil, nil } - r, err := json.Marshal(mutation.ConvertPullRequestToDraft.PullRequest) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil + return utils.NewToolResultText("pull request converted to draft"), nil, nil } var mutation struct { MarkPullRequestReadyForReview struct { PullRequest struct { - ID, Title, URL string - IsDraft bool + ID githubv4.ID + IsDraft githubv4.Boolean } } `graphql:"markPullRequestReadyForReview(input: $input)"` } - input := map[string]any{"pullRequestId": githubv4.ID(prNodeID)} - if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { - return utils.NewToolResultErrorFromErr("failed to mark ready for review", err), nil, nil + if err := gqlClient.Mutate(ctx, &mutation, githubv4.MarkPullRequestReadyForReviewInput{ + PullRequestID: prQuery.Repository.PullRequest.ID, + }, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to mark ready for review", err), nil, nil } - r, err := json.Marshal(mutation.MarkPullRequestReadyForReview.PullRequest) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil + return utils.NewToolResultText("pull request marked as ready for review"), nil, nil }, ) } @@ -373,32 +379,25 @@ func GranularCreatePullRequestReview(t translations.TranslationHelperFunc) inven event, _ := OptionalParam[string](args, "event") commitID, _ := OptionalParam[string](args, "commitID") - reviewReq := &gogithub.PullRequestReviewRequest{} - if body != "" { - reviewReq.Body = &body - } - if event != "" { - reviewReq.Event = &event - } - if commitID != "" { - reviewReq.CommitID = &commitID - } - - client, err := deps.GetClient(ctx) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil - } - - review, _, err := client.PullRequests.CreateReview(ctx, owner, repo, pullNumber, reviewReq) + gqlClient, err := deps.GetGQLClient(ctx) if err != nil { - return utils.NewToolResultErrorFromErr("failed to create review", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil } - r, err := json.Marshal(review) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil + var commitIDPtr *string + if commitID != "" { + commitIDPtr = &commitID + } + + result, err := CreatePullRequestReview(ctx, gqlClient, PullRequestReviewWriteParams{ + Owner: owner, + Repo: repo, + PullNumber: int32(pullNumber), // #nosec G115 - PR numbers are always small positive integers + Body: body, + Event: event, + CommitID: commitIDPtr, + }) + return result, nil, err }, ) } @@ -453,65 +452,14 @@ func GranularSubmitPendingPullRequestReview(t translations.TranslationHelperFunc return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil } - prNodeID, err := getGranularPullRequestNodeID(ctx, gqlClient, owner, repo, pullNumber) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to get pull request", err), nil, nil - } - - // Find pending review - var reviewQuery struct { - Repository struct { - PullRequest struct { - Reviews struct { - Nodes []struct { - ID, State string - } - } `graphql:"reviews(first: 1, states: PENDING)"` - } `graphql:"pullRequest(number: $number)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - vars := map[string]any{ - "owner": githubv4.String(owner), - "name": githubv4.String(repo), - "number": githubv4.Int(pullNumber), // #nosec G115 - PR numbers are always small positive integers - } - if err := gqlClient.Query(ctx, &reviewQuery, vars); err != nil { - return utils.NewToolResultErrorFromErr("failed to find pending review", err), nil, nil - } - - if len(reviewQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return utils.NewToolResultError("no pending review found for the current user"), nil, nil - } - - reviewID := reviewQuery.Repository.PullRequest.Reviews.Nodes[0].ID - - var mutation struct { - SubmitPullRequestReview struct { - PullRequestReview struct { - ID, State, URL string - } - } `graphql:"submitPullRequestReview(input: $input)"` - } - - submitInput := map[string]any{ - "pullRequestId": githubv4.ID(prNodeID), - "pullRequestReviewId": githubv4.ID(reviewID), - "event": githubv4.PullRequestReviewEvent(event), - } - if body != "" { - submitInput["body"] = githubv4.String(body) - } - - if err := gqlClient.Mutate(ctx, &mutation, submitInput, nil); err != nil { - return utils.NewToolResultErrorFromErr("failed to submit review", err), nil, nil - } - - r, err := json.Marshal(mutation.SubmitPullRequestReview.PullRequestReview) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil + result, err := SubmitPendingPullRequestReview(ctx, gqlClient, PullRequestReviewWriteParams{ + Owner: owner, + Repo: repo, + PullNumber: int32(pullNumber), // #nosec G115 - PR numbers are always small positive integers + Event: event, + Body: body, + }) + return result, nil, err }, ) } @@ -559,45 +507,12 @@ func GranularDeletePendingPullRequestReview(t translations.TranslationHelperFunc return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil } - // Find pending review - var reviewQuery struct { - Repository struct { - PullRequest struct { - Reviews struct { - Nodes []struct{ ID string } - } `graphql:"reviews(first: 1, states: PENDING)"` - } `graphql:"pullRequest(number: $number)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - vars := map[string]any{ - "owner": githubv4.String(owner), - "name": githubv4.String(repo), - "number": githubv4.Int(pullNumber), // #nosec G115 - PR numbers are always small positive integers - } - if err := gqlClient.Query(ctx, &reviewQuery, vars); err != nil { - return utils.NewToolResultErrorFromErr("failed to find pending review", err), nil, nil - } - - if len(reviewQuery.Repository.PullRequest.Reviews.Nodes) == 0 { - return utils.NewToolResultError("no pending review found for the current user"), nil, nil - } - - reviewID := reviewQuery.Repository.PullRequest.Reviews.Nodes[0].ID - - var mutation struct { - DeletePullRequestReview struct { - PullRequestReview struct{ ID string } - } `graphql:"deletePullRequestReview(input: $input)"` - } - - input := map[string]any{"pullRequestReviewId": githubv4.ID(reviewID)} - - if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { - return utils.NewToolResultErrorFromErr("failed to delete pending review", err), nil, nil - } - - return utils.NewToolResultText(`{"message": "Pending review deleted successfully"}`), nil, nil + result, err := DeletePendingPullRequestReview(ctx, gqlClient, PullRequestReviewWriteParams{ + Owner: owner, + Repo: repo, + PullNumber: int32(pullNumber), // #nosec G115 - PR numbers are always small positive integers + }) + return result, nil, err }, ) } @@ -658,9 +573,15 @@ func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) i if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - line, _ := OptionalParam[float64](args, "line") + line, err := OptionalIntParam(args, "line") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } side, _ := OptionalParam[string](args, "side") - startLine, _ := OptionalParam[float64](args, "startLine") + startLine, err := OptionalIntParam(args, "startLine") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } startSide, _ := OptionalParam[string](args, "startSide") gqlClient, err := deps.GetGQLClient(ctx) @@ -668,75 +589,99 @@ func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) i return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil } - prNodeID, err := getGranularPullRequestNodeID(ctx, gqlClient, owner, repo, pullNumber) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to get pull request", err), nil, nil + // Get the current user to find their pending review + var getViewerQuery struct { + Viewer struct { + Login githubv4.String + } + } + if err := gqlClient.Query(ctx, &getViewerQuery, nil); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to get current user", err), nil, nil } - var mutation struct { - AddPullRequestReviewThread struct { - Thread struct { - ID string - Comments struct { + // Find the viewer's latest review (must be pending) + var getLatestReviewForViewerQuery struct { + Repository struct { + PullRequest struct { + Reviews struct { Nodes []struct { - ID, Body, URL string + ID githubv4.ID + State githubv4.PullRequestReviewState + URL githubv4.URI } - } `graphql:"comments(first: 1)"` - } - } `graphql:"addPullRequestReviewThread(input: $input)"` + } `graphql:"reviews(first: 1, author: $author)"` + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $name)"` } - input := map[string]any{ - "pullRequestId": githubv4.ID(prNodeID), - "path": githubv4.String(path), - "body": githubv4.String(body), - "subjectType": githubv4.PullRequestReviewThreadSubjectType(subjectType), + vars := map[string]any{ + "author": githubv4.String(getViewerQuery.Viewer.Login), + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + "prNum": githubv4.Int(pullNumber), // #nosec G115 - PR numbers are always small positive integers } - if line != 0 { - input["line"] = githubv4.Int(int(line)) // #nosec G115 + + if err := gqlClient.Query(ctx, &getLatestReviewForViewerQuery, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to get latest review for current user", err), nil, nil } - if side != "" { - input["side"] = githubv4.DiffSide(side) + + if len(getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes) == 0 { + return utils.NewToolResultError("No pending review found for the viewer"), nil, nil } - if startLine != 0 { - input["startLine"] = githubv4.Int(int(startLine)) // #nosec G115 + + review := getLatestReviewForViewerQuery.Repository.PullRequest.Reviews.Nodes[0] + if review.State != githubv4.PullRequestReviewStatePending { + errText := fmt.Sprintf("The latest review, found at %s is not pending", review.URL) + return utils.NewToolResultError(errText), nil, nil } - if startSide != "" { - input["startSide"] = githubv4.DiffSide(startSide) + + // Add review thread comment to the pending review + var addPullRequestReviewThreadMutation struct { + AddPullRequestReviewThread struct { + Thread struct { + ID githubv4.ID + } + } `graphql:"addPullRequestReviewThread(input: $input)"` } - if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { - return utils.NewToolResultErrorFromErr("failed to add review comment", err), nil, nil + // Convert optional int params to *int32 for GraphQL helper + var linePtr, startLinePtr *int32 + if line != 0 { + l := int32(line) // #nosec G115 + linePtr = &l + } + if startLine != 0 { + sl := int32(startLine) // #nosec G115 + startLinePtr = &sl + } + + if err := gqlClient.Mutate( + ctx, + &addPullRequestReviewThreadMutation, + githubv4.AddPullRequestReviewThreadInput{ + Path: githubv4.String(path), + Body: githubv4.String(body), + SubjectType: newGQLStringlikePtr[githubv4.PullRequestReviewThreadSubjectType](&subjectType), + Line: newGQLIntPtr(linePtr), + Side: newGQLStringlikePtr[githubv4.DiffSide](&side), + StartLine: newGQLIntPtr(startLinePtr), + StartSide: newGQLStringlikePtr[githubv4.DiffSide](&startSide), + PullRequestReviewID: &review.ID, + }, + nil, + ); err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } - r, err := json.Marshal(mutation.AddPullRequestReviewThread) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + if addPullRequestReviewThreadMutation.AddPullRequestReviewThread.Thread.ID == nil { + return utils.NewToolResultError(`Failed to add comment to pending review. Possible reasons: + - The line number doesn't exist in the pull request diff + - The file path is incorrect + - The side (LEFT/RIGHT) is invalid for the specified line +`), nil, nil } - return utils.NewToolResultText(string(r)), nil, nil + + return utils.NewToolResultText("pull request review comment successfully added to pending review"), nil, nil }, ) } - -// getGranularPullRequestNodeID fetches the GraphQL node ID for a pull request. -func getGranularPullRequestNodeID(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, pullNumber int) (string, error) { - var query struct { - Repository struct { - PullRequest struct { - ID string - } `graphql:"pullRequest(number: $number)"` - } `graphql:"repository(owner: $owner, name: $name)"` - } - - vars := map[string]any{ - "owner": githubv4.String(owner), - "name": githubv4.String(repo), - "number": githubv4.Int(pullNumber), // #nosec G115 - PR numbers are always small positive integers - } - - if err := gqlClient.Query(ctx, &query, vars); err != nil { - return "", fmt.Errorf("failed to query pull request node ID: %w", err) - } - - return query.Repository.PullRequest.ID, nil -} From ee1fb9fce8c2d1a9781c4ed8d7e7c23352b58ab5 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 9 Apr 2026 10:20:05 +0200 Subject: [PATCH 4/5] refactor: use feature flags instead of separate granular toolsets Place granular tools in existing issues/pull_requests toolsets with FeatureFlagEnable, instead of creating separate issues_granular and pull_requests_granular toolsets. This is simpler and uses the existing feature flag infrastructure to switch between consolidated and granular tool variants at runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 166 ---------------------------- docs/remote-server.md | 2 - pkg/github/granular_tools_test.go | 62 ++--------- pkg/github/issues_granular.go | 30 +++-- pkg/github/pullrequests_granular.go | 42 ++++--- pkg/github/tools.go | 19 +--- 6 files changed, 66 insertions(+), 255 deletions(-) diff --git a/README.md b/README.md index 43ecf36e3..419f89297 100644 --- a/README.md +++ b/README.md @@ -566,13 +566,11 @@ The following sets of tools are available: | logo-gist | `gists` | GitHub Gist related tools | | git-branch | `git` | GitHub Git API related tools for low-level Git operations | | issue-opened | `issues` | GitHub Issues related tools | -| issue-opened | `issues_granular` | Granular issue tools with fine-grained control over individual operations | | tag | `labels` | GitHub Labels related tools | | bell | `notifications` | GitHub Notifications related tools | | organization | `orgs` | GitHub Organization related tools | | project | `projects` | GitHub Projects related tools | | git-pull-request | `pull_requests` | GitHub Pull Request related tools | -| git-pull-request | `pull_requests_granular` | Granular pull request tools with fine-grained control over individual operations | | repo | `repos` | GitHub Repository related tools | | shield-lock | `secret_protection` | Secret protection related tools, such as GitHub Secret Scanning | | shield | `security_advisories` | Security advisories related tools | @@ -905,93 +903,6 @@ The following sets of tools are available:
-issue-opened Issues Granular - -- **add_sub_issue** - Add Sub-Issue - - **Required OAuth Scopes**: `repo` - - `issue_number`: The parent issue number (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `replace_parent`: If true, reparent the sub-issue if it already has a parent (boolean, optional) - - `repo`: Repository name (string, required) - - `sub_issue_id`: The ID of the sub-issue to add. ID is not the same as issue number (number, required) - -- **create_issue** - Create Issue - - **Required OAuth Scopes**: `repo` - - `body`: Issue body content (optional) (string, optional) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - `title`: Issue title (string, required) - -- **remove_sub_issue** - Remove Sub-Issue - - **Required OAuth Scopes**: `repo` - - `issue_number`: The parent issue number (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - `sub_issue_id`: The ID of the sub-issue to remove. ID is not the same as issue number (number, required) - -- **reprioritize_sub_issue** - Reprioritize Sub-Issue - - **Required OAuth Scopes**: `repo` - - `after_id`: The ID of the sub-issue to place this after (either after_id OR before_id should be specified) (number, optional) - - `before_id`: The ID of the sub-issue to place this before (either after_id OR before_id should be specified) (number, optional) - - `issue_number`: The parent issue number (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - `sub_issue_id`: The ID of the sub-issue to reorder. ID is not the same as issue number (number, required) - -- **update_issue_assignees** - Update Issue Assignees - - **Required OAuth Scopes**: `repo` - - `assignees`: GitHub usernames to assign to this issue (string[], required) - - `issue_number`: The issue number to update (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -- **update_issue_body** - Update Issue Body - - **Required OAuth Scopes**: `repo` - - `body`: The new body content for the issue (string, required) - - `issue_number`: The issue number to update (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -- **update_issue_labels** - Update Issue Labels - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `labels`: Labels to apply to this issue (string[], required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -- **update_issue_milestone** - Update Issue Milestone - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `milestone`: The milestone number to set on the issue (integer, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -- **update_issue_state** - Update Issue State - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - `state`: The new state for the issue (string, required) - - `state_reason`: The reason for the state change (only for closed state) (string, optional) - -- **update_issue_title** - Update Issue Title - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - - `title`: The new title for the issue (string, required) - -- **update_issue_type** - Update Issue Type - - **Required OAuth Scopes**: `repo` - - `issue_number`: The issue number to update (number, required) - - `issue_type`: The issue type to set (string, required) - - `owner`: Repository owner (username or organization) (string, required) - - `repo`: Repository name (string, required) - -
- -
- tag Labels - **get_label** - Get a specific label from a repository. @@ -1244,83 +1155,6 @@ The following sets of tools are available:
-git-pull-request Pull Requests Granular - -- **add_pull_request_review_comment** - Add Pull Request Review Comment - - **Required OAuth Scopes**: `repo` - - `body`: The comment body (string, required) - - `line`: The line number in the diff to comment on (optional) (number, optional) - - `owner`: Repository owner (username or organization) (string, required) - - `path`: The relative path of the file to comment on (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - `side`: The side of the diff to comment on (optional) (string, optional) - - `startLine`: The start line of a multi-line comment (optional) (number, optional) - - `startSide`: The start side of a multi-line comment (optional) (string, optional) - - `subjectType`: The subject type of the comment (string, required) - -- **create_pull_request_review** - Create Pull Request Review - - **Required OAuth Scopes**: `repo` - - `body`: The review body text (optional) (string, optional) - - `commitID`: The SHA of the commit to review (optional, defaults to latest) (string, optional) - - `event`: The review action to perform. If omitted, creates a pending review. (string, optional) - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - -- **delete_pending_pull_request_review** - Delete Pending Pull Request Review - - **Required OAuth Scopes**: `repo` - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - -- **request_pull_request_reviewers** - Request Pull Request Reviewers - - **Required OAuth Scopes**: `repo` - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - `reviewers`: GitHub usernames to request reviews from (string[], required) - -- **submit_pending_pull_request_review** - Submit Pending Pull Request Review - - **Required OAuth Scopes**: `repo` - - `body`: The review body text (optional) (string, optional) - - `event`: The review action to perform (string, required) - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - -- **update_pull_request_body** - Update Pull Request Body - - **Required OAuth Scopes**: `repo` - - `body`: The new body content for the pull request (string, required) - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - -- **update_pull_request_draft_state** - Update Pull Request Draft State - - **Required OAuth Scopes**: `repo` - - `draft`: Set to true to convert to draft, false to mark as ready for review (boolean, required) - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - -- **update_pull_request_state** - Update Pull Request State - - **Required OAuth Scopes**: `repo` - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - `state`: The new state for the pull request (string, required) - -- **update_pull_request_title** - Update Pull Request Title - - **Required OAuth Scopes**: `repo` - - `owner`: Repository owner (username or organization) (string, required) - - `pullNumber`: The pull request number (number, required) - - `repo`: Repository name (string, required) - - `title`: The new title for the pull request (string, required) - -
- -
- repo Repositories - **create_branch** - Create branch diff --git a/docs/remote-server.md b/docs/remote-server.md index 215e81af7..5a82f1c2e 100644 --- a/docs/remote-server.md +++ b/docs/remote-server.md @@ -28,13 +28,11 @@ Below is a table of available toolsets for the remote GitHub MCP Server. Each to | logo-gist
`gists` | GitHub Gist related tools | https://api.githubcopilot.com/mcp/x/gists | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-gists&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgists%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/gists/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-gists&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgists%2Freadonly%22%7D) | | git-branch
`git` | GitHub Git API related tools for low-level Git operations | https://api.githubcopilot.com/mcp/x/git | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-git&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgit%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/git/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-git&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgit%2Freadonly%22%7D) | | issue-opened
`issues` | GitHub Issues related tools | https://api.githubcopilot.com/mcp/x/issues | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/issues/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues%2Freadonly%22%7D) | -| issue-opened
`issues_granular` | Granular issue tools with fine-grained control over individual operations | https://api.githubcopilot.com/mcp/x/issues_granular | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues_granular&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues_granular%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/issues_granular/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues_granular&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues_granular%2Freadonly%22%7D) | | tag
`labels` | GitHub Labels related tools | https://api.githubcopilot.com/mcp/x/labels | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-labels&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Flabels%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/labels/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-labels&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Flabels%2Freadonly%22%7D) | | bell
`notifications` | GitHub Notifications related tools | https://api.githubcopilot.com/mcp/x/notifications | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-notifications&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fnotifications%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/notifications/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-notifications&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fnotifications%2Freadonly%22%7D) | | organization
`orgs` | GitHub Organization related tools | https://api.githubcopilot.com/mcp/x/orgs | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-orgs&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Forgs%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/orgs/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-orgs&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Forgs%2Freadonly%22%7D) | | project
`projects` | GitHub Projects related tools | https://api.githubcopilot.com/mcp/x/projects | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-projects&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fprojects%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/projects/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-projects&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fprojects%2Freadonly%22%7D) | | git-pull-request
`pull_requests` | GitHub Pull Request related tools | https://api.githubcopilot.com/mcp/x/pull_requests | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-pull_requests&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fpull_requests%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/pull_requests/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-pull_requests&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fpull_requests%2Freadonly%22%7D) | -| git-pull-request
`pull_requests_granular` | Granular pull request tools with fine-grained control over individual operations | https://api.githubcopilot.com/mcp/x/pull_requests_granular | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-pull_requests_granular&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fpull_requests_granular%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/pull_requests_granular/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-pull_requests_granular&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fpull_requests_granular%2Freadonly%22%7D) | | repo
`repos` | GitHub Repository related tools | https://api.githubcopilot.com/mcp/x/repos | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-repos&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Frepos%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/repos/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-repos&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Frepos%2Freadonly%22%7D) | | shield-lock
`secret_protection` | Secret protection related tools, such as GitHub Secret Scanning | https://api.githubcopilot.com/mcp/x/secret_protection | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-secret_protection&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fsecret_protection%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/secret_protection/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-secret_protection&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fsecret_protection%2Freadonly%22%7D) | | shield
`security_advisories` | Security advisories related tools | https://api.githubcopilot.com/mcp/x/security_advisories | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-security_advisories&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fsecurity_advisories%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/security_advisories/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-security_advisories&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fsecurity_advisories%2Freadonly%22%7D) | diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 231303440..37b3fe3c8 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -15,10 +15,10 @@ import ( "github.com/stretchr/testify/require" ) -func granularToolsForToolset(id inventory.ToolsetID) []inventory.ServerTool { +func granularToolsForToolset(toolsetID inventory.ToolsetID, featureFlag string) []inventory.ServerTool { var result []inventory.ServerTool for _, tool := range AllTools(translations.NullTranslationHelper) { - if tool.Toolset.ID == id { + if tool.Toolset.ID == toolsetID && tool.FeatureFlagEnable == featureFlag { result = append(result, tool) } } @@ -59,8 +59,8 @@ func TestGranularToolSnaps(t *testing.T) { } func TestIssuesGranularToolset(t *testing.T) { - t.Run("toolset contains expected tools", func(t *testing.T) { - tools := granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) + t.Run("toolset contains expected granular tools", func(t *testing.T) { + tools := granularToolsForToolset(ToolsetMetadataIssues.ID, FeatureFlagIssuesGranular) toolNames := make([]string, 0, len(tools)) for _, tool := range tools { @@ -86,38 +86,16 @@ func TestIssuesGranularToolset(t *testing.T) { assert.Len(t, tools, len(expected)) }) - t.Run("all tools belong to issues_granular toolset", func(t *testing.T) { - for _, tool := range granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) { - assert.Equal(t, ToolsetMetadataIssuesGranular.ID, tool.Toolset.ID, "tool %s", tool.Tool.Name) - } - }) - - t.Run("all tools are write tools", func(t *testing.T) { - for _, tool := range granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) { - assert.False(t, tool.Tool.Annotations.ReadOnlyHint, "tool %s should have ReadOnlyHint=false", tool.Tool.Name) - } - }) - - t.Run("toolset is non-default", func(t *testing.T) { - assert.False(t, ToolsetMetadataIssuesGranular.Default) - }) - - t.Run("no duplicate names with issues toolset", func(t *testing.T) { - issueTools := make(map[string]bool) - for _, tool := range AllTools(translations.NullTranslationHelper) { - if tool.Toolset.ID == ToolsetMetadataIssues.ID { - issueTools[tool.Tool.Name] = true - } - } - for _, tool := range granularToolsForToolset(ToolsetMetadataIssuesGranular.ID) { - assert.False(t, issueTools[tool.Tool.Name], "tool %s duplicates a tool in the issues toolset", tool.Tool.Name) + t.Run("all granular tools have correct feature flag", func(t *testing.T) { + for _, tool := range granularToolsForToolset(ToolsetMetadataIssues.ID, FeatureFlagIssuesGranular) { + assert.Equal(t, FeatureFlagIssuesGranular, tool.FeatureFlagEnable, "tool %s", tool.Tool.Name) } }) } func TestPullRequestsGranularToolset(t *testing.T) { - t.Run("toolset contains expected tools", func(t *testing.T) { - tools := granularToolsForToolset(ToolsetMetadataPullRequestsGranular.ID) + t.Run("toolset contains expected granular tools", func(t *testing.T) { + tools := granularToolsForToolset(ToolsetMetadataPullRequests.ID, FeatureFlagPullRequestsGranular) toolNames := make([]string, 0, len(tools)) for _, tool := range tools { @@ -141,25 +119,9 @@ func TestPullRequestsGranularToolset(t *testing.T) { assert.Len(t, tools, len(expected)) }) - t.Run("all tools belong to pull_requests_granular toolset", func(t *testing.T) { - for _, tool := range granularToolsForToolset(ToolsetMetadataPullRequestsGranular.ID) { - assert.Equal(t, ToolsetMetadataPullRequestsGranular.ID, tool.Toolset.ID, "tool %s", tool.Tool.Name) - } - }) - - t.Run("toolset is non-default", func(t *testing.T) { - assert.False(t, ToolsetMetadataPullRequestsGranular.Default) - }) - - t.Run("no duplicate names with pull_requests toolset", func(t *testing.T) { - prTools := make(map[string]bool) - for _, tool := range AllTools(translations.NullTranslationHelper) { - if tool.Toolset.ID == ToolsetMetadataPullRequests.ID { - prTools[tool.Tool.Name] = true - } - } - for _, tool := range granularToolsForToolset(ToolsetMetadataPullRequestsGranular.ID) { - assert.False(t, prTools[tool.Tool.Name], "tool %s duplicates a tool in the pull_requests toolset", tool.Tool.Name) + t.Run("all granular tools have correct feature flag", func(t *testing.T) { + for _, tool := range granularToolsForToolset(ToolsetMetadataPullRequests.ID, FeatureFlagPullRequestsGranular) { + assert.Equal(t, FeatureFlagPullRequestsGranular, tool.FeatureFlagEnable, "tool %s", tool.Tool.Name) } }) } diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 7dd157839..e0338e409 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -42,8 +42,8 @@ func issueUpdateTool( required := append([]string{"owner", "repo", "issue_number"}, extraRequired...) - return NewTool( - ToolsetMetadataIssuesGranular, + st := NewTool( + ToolsetMetadataIssues, mcp.Tool{ Name: name, Description: t("TOOL_"+name+"_DESCRIPTION", description), @@ -96,12 +96,14 @@ func issueUpdateTool( return utils.NewToolResultText(string(r)), nil, nil }, ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st } // GranularCreateIssue creates a tool to create a new issue. func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataIssuesGranular, + st := NewTool( + ToolsetMetadataIssues, mcp.Tool{ Name: "create_issue", Description: t("TOOL_CREATE_ISSUE_DESCRIPTION", "Create a new issue in a GitHub repository with a title and optional body."), @@ -174,6 +176,8 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT return utils.NewToolResultText(string(r)), nil, nil }, ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st } // GranularUpdateIssueTitle creates a tool to update an issue's title. @@ -360,8 +364,8 @@ func GranularUpdateIssueState(t translations.TranslationHelperFunc) inventory.Se // GranularAddSubIssue creates a tool to add a sub-issue. func GranularAddSubIssue(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataIssuesGranular, + st := NewTool( + ToolsetMetadataIssues, mcp.Tool{ Name: "add_sub_issue", Description: t("TOOL_ADD_SUB_ISSUE_DESCRIPTION", "Add a sub-issue to a parent issue."), @@ -428,12 +432,14 @@ func GranularAddSubIssue(t translations.TranslationHelperFunc) inventory.ServerT return result, nil, err }, ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st } // GranularRemoveSubIssue creates a tool to remove a sub-issue. func GranularRemoveSubIssue(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataIssuesGranular, + st := NewTool( + ToolsetMetadataIssues, mcp.Tool{ Name: "remove_sub_issue", Description: t("TOOL_REMOVE_SUB_ISSUE_DESCRIPTION", "Remove a sub-issue from a parent issue."), @@ -495,12 +501,14 @@ func GranularRemoveSubIssue(t translations.TranslationHelperFunc) inventory.Serv return result, nil, err }, ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st } // GranularReprioritizeSubIssue creates a tool to reorder a sub-issue. func GranularReprioritizeSubIssue(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataIssuesGranular, + st := NewTool( + ToolsetMetadataIssues, mcp.Tool{ Name: "reprioritize_sub_issue", Description: t("TOOL_REPRIORITIZE_SUB_ISSUE_DESCRIPTION", "Reprioritize (reorder) a sub-issue relative to other sub-issues."), @@ -578,4 +586,6 @@ func GranularReprioritizeSubIssue(t translations.TranslationHelperFunc) inventor return result, nil, err }, ) + st.FeatureFlagEnable = FeatureFlagIssuesGranular + return st } diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index b694ff0be..661f2b4f5 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -44,8 +44,8 @@ func prUpdateTool( required := append([]string{"owner", "repo", "pullNumber"}, extraRequired...) - return NewTool( - ToolsetMetadataPullRequestsGranular, + st := NewTool( + ToolsetMetadataPullRequests, mcp.Tool{ Name: name, Description: t("TOOL_"+name+"_DESCRIPTION", description), @@ -98,6 +98,8 @@ func prUpdateTool( return utils.NewToolResultText(string(r)), nil, nil }, ) + st.FeatureFlagEnable = FeatureFlagPullRequestsGranular + return st } // GranularUpdatePullRequestTitle creates a tool to update a PR's title. @@ -166,8 +168,8 @@ func GranularUpdatePullRequestState(t translations.TranslationHelperFunc) invent // GranularUpdatePullRequestDraftState creates a tool to toggle draft state. func GranularUpdatePullRequestDraftState(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataPullRequestsGranular, + st := NewTool( + ToolsetMetadataPullRequests, mcp.Tool{ Name: "update_pull_request_draft_state", Description: t("TOOL_UPDATE_PULL_REQUEST_DRAFT_STATE_DESCRIPTION", "Mark a pull request as draft or ready for review."), @@ -261,12 +263,14 @@ func GranularUpdatePullRequestDraftState(t translations.TranslationHelperFunc) i return utils.NewToolResultText("pull request marked as ready for review"), nil, nil }, ) + st.FeatureFlagEnable = FeatureFlagPullRequestsGranular + return st } // GranularRequestPullRequestReviewers creates a tool to request reviewers. func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataPullRequestsGranular, + st := NewTool( + ToolsetMetadataPullRequests, mcp.Tool{ Name: "request_pull_request_reviewers", Description: t("TOOL_REQUEST_PULL_REQUEST_REVIEWERS_DESCRIPTION", "Request reviewers for a pull request."), @@ -333,12 +337,14 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i return utils.NewToolResultText(string(r)), nil, nil }, ) + st.FeatureFlagEnable = FeatureFlagPullRequestsGranular + return st } // GranularCreatePullRequestReview creates a tool to create a PR review. func GranularCreatePullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataPullRequestsGranular, + st := NewTool( + ToolsetMetadataPullRequests, mcp.Tool{ Name: "create_pull_request_review", Description: t("TOOL_CREATE_PULL_REQUEST_REVIEW_DESCRIPTION", "Create a review on a pull request. If event is provided, the review is submitted immediately; otherwise a pending review is created."), @@ -400,12 +406,14 @@ func GranularCreatePullRequestReview(t translations.TranslationHelperFunc) inven return result, nil, err }, ) + st.FeatureFlagEnable = FeatureFlagPullRequestsGranular + return st } // GranularSubmitPendingPullRequestReview creates a tool to submit a pending review. func GranularSubmitPendingPullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataPullRequestsGranular, + st := NewTool( + ToolsetMetadataPullRequests, mcp.Tool{ Name: "submit_pending_pull_request_review", Description: t("TOOL_SUBMIT_PENDING_PULL_REQUEST_REVIEW_DESCRIPTION", "Submit a pending pull request review."), @@ -462,12 +470,14 @@ func GranularSubmitPendingPullRequestReview(t translations.TranslationHelperFunc return result, nil, err }, ) + st.FeatureFlagEnable = FeatureFlagPullRequestsGranular + return st } // GranularDeletePendingPullRequestReview creates a tool to delete a pending review. func GranularDeletePendingPullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataPullRequestsGranular, + st := NewTool( + ToolsetMetadataPullRequests, mcp.Tool{ Name: "delete_pending_pull_request_review", Description: t("TOOL_DELETE_PENDING_PULL_REQUEST_REVIEW_DESCRIPTION", "Delete a pending pull request review."), @@ -515,12 +525,14 @@ func GranularDeletePendingPullRequestReview(t translations.TranslationHelperFunc return result, nil, err }, ) + st.FeatureFlagEnable = FeatureFlagPullRequestsGranular + return st } // GranularAddPullRequestReviewComment creates a tool to add a review comment. func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( - ToolsetMetadataPullRequestsGranular, + st := NewTool( + ToolsetMetadataPullRequests, mcp.Tool{ Name: "add_pull_request_review_comment", Description: t("TOOL_ADD_PULL_REQUEST_REVIEW_COMMENT_DESCRIPTION", "Add a review comment to the current user's pending pull request review."), @@ -684,4 +696,6 @@ func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) i return utils.NewToolResultText("pull request review comment successfully added to pending review"), nil, nil }, ) + st.FeatureFlagEnable = FeatureFlagPullRequestsGranular + return st } diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 352c9647d..ae5e286e3 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -141,17 +141,10 @@ var ( Icon: "copilot", } - ToolsetMetadataIssuesGranular = inventory.ToolsetMetadata{ - ID: "issues_granular", - Description: "Granular issue tools with fine-grained control over individual operations", - Icon: "issue-opened", - } - - ToolsetMetadataPullRequestsGranular = inventory.ToolsetMetadata{ - ID: "pull_requests_granular", - Description: "Granular pull request tools with fine-grained control over individual operations", - Icon: "git-pull-request", - } + // Feature flag names for granular tool variants. + // When active, consolidated tools are replaced by single-purpose granular tools. + FeatureFlagIssuesGranular = "issues_granular" + FeatureFlagPullRequestsGranular = "pull_requests_granular" // Remote-only toolsets - these are only available in the remote MCP server // but are documented here for consistency and to enable automated documentation. @@ -287,7 +280,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { ListLabels(t), LabelWrite(t), - // Granular issue tools (opt-in, non-default) + // Granular issue tools (feature-flagged, replace consolidated issue_write/sub_issue_write) GranularCreateIssue(t), GranularUpdateIssueTitle(t), GranularUpdateIssueBody(t), @@ -300,7 +293,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { GranularRemoveSubIssue(t), GranularReprioritizeSubIssue(t), - // Granular pull request tools (opt-in, non-default) + // Granular pull request tools (feature-flagged, replace consolidated update_pull_request/pull_request_review_write) GranularUpdatePullRequestTitle(t), GranularUpdatePullRequestBody(t), GranularUpdatePullRequestState(t), From ad8da501dcbced8bf835daa7322434635b5a9b3e Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 10 Apr 2026 10:20:36 +0200 Subject: [PATCH 5/5] fix: address review feedback on granular toolsets - Fix REST response handling: capture resp, close body, use ghErrors helpers in issueUpdateTool, prUpdateTool, GranularCreateIssue, and GranularRequestPullRequestReviewers - Add FeatureFlagDisable on consolidated tools (IssueWrite, SubIssueWrite, UpdatePullRequest, PullRequestReviewWrite, AddCommentToPendingReview) so they are hidden when granular variants are active - Use OptionalStringArrayParam for assignees, labels, reviewers instead of manual loop that silently dropped non-string elements - Fix side/startSide empty string leak: pass nil pointer when absent instead of pointer to empty string in GraphQL mutations - Fix milestone minimum from 0 to 1 to match RequiredInt rejection of 0 - Return MinimalResponse {id, url} instead of full JSON objects - Fix RequiredParam[bool] rejecting draft=false by using presence check - Add handler tests for update_pull_request_draft_state (draft + ready) and add_pull_request_review_comment with full GraphQL mocking Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../__toolsnaps__/update_issue_milestone.snap | 2 +- pkg/github/granular_tools_test.go | 178 ++++++++++++++++++ pkg/github/issues.go | 8 +- pkg/github/issues_granular.go | 49 ++--- pkg/github/pullrequests.go | 12 +- pkg/github/pullrequests_granular.go | 52 +++-- 6 files changed, 255 insertions(+), 46 deletions(-) diff --git a/pkg/github/__toolsnaps__/update_issue_milestone.snap b/pkg/github/__toolsnaps__/update_issue_milestone.snap index 62e1d68a8..9188779f0 100644 --- a/pkg/github/__toolsnaps__/update_issue_milestone.snap +++ b/pkg/github/__toolsnaps__/update_issue_milestone.snap @@ -14,7 +14,7 @@ }, "milestone": { "description": "The milestone number to set on the issue", - "minimum": 0, + "minimum": 1, "type": "integer" }, "owner": { diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 37b3fe3c8..ff9b85f20 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -522,3 +522,181 @@ func TestGranularCreatePullRequestReview(t *testing.T) { require.NoError(t, err) assert.False(t, result.IsError) } + +func TestGranularUpdatePullRequestDraftState(t *testing.T) { + tests := []struct { + name string + draft bool + }{ + {name: "convert to draft", draft: true}, + {name: "mark ready for review", draft: false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var matchers []githubv4mock.Matcher + + matchers = append(matchers, githubv4mock.NewQueryMatcher( + struct { + Repository struct { + PullRequest struct { + ID githubv4.ID + } `graphql:"pullRequest(number: $number)"` + } `graphql:"repository(owner: $owner, name: $name)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "name": githubv4.String("repo"), + "number": githubv4.Int(1), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "pullRequest": map[string]any{"id": "PR_123"}, + }, + }), + )) + + if tc.draft { + matchers = append(matchers, githubv4mock.NewMutationMatcher( + struct { + ConvertPullRequestToDraft struct { + PullRequest struct { + ID githubv4.ID + IsDraft githubv4.Boolean + } + } `graphql:"convertPullRequestToDraft(input: $input)"` + }{}, + githubv4.ConvertPullRequestToDraftInput{PullRequestID: githubv4.ID("PR_123")}, + nil, + githubv4mock.DataResponse(map[string]any{ + "convertPullRequestToDraft": map[string]any{ + "pullRequest": map[string]any{"id": "PR_123", "isDraft": true}, + }, + }), + )) + } else { + matchers = append(matchers, githubv4mock.NewMutationMatcher( + struct { + MarkPullRequestReadyForReview struct { + PullRequest struct { + ID githubv4.ID + IsDraft githubv4.Boolean + } + } `graphql:"markPullRequestReadyForReview(input: $input)"` + }{}, + githubv4.MarkPullRequestReadyForReviewInput{PullRequestID: githubv4.ID("PR_123")}, + nil, + githubv4mock.DataResponse(map[string]any{ + "markPullRequestReadyForReview": map[string]any{ + "pullRequest": map[string]any{"id": "PR_123", "isDraft": false}, + }, + }), + )) + } + + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matchers...)) + deps := BaseDeps{GQLClient: gqlClient} + serverTool := GranularUpdatePullRequestDraftState(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(1), + "draft": tc.draft, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) + }) + } +} + +func TestGranularAddPullRequestReviewComment(t *testing.T) { + mockedClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + struct { + Viewer struct { + Login githubv4.String + } + }{}, + nil, + githubv4mock.DataResponse(map[string]any{ + "viewer": map[string]any{"login": "testuser"}, + }), + ), + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + PullRequest struct { + Reviews struct { + Nodes []struct { + ID githubv4.ID + State githubv4.PullRequestReviewState + URL githubv4.URI + } + } `graphql:"reviews(first: 1, author: $author)"` + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $name)"` + }{}, + map[string]any{ + "author": githubv4.String("testuser"), + "owner": githubv4.String("owner"), + "name": githubv4.String("repo"), + "prNum": githubv4.Int(1), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "pullRequest": map[string]any{ + "reviews": map[string]any{ + "nodes": []map[string]any{ + {"id": "PRR_123", "state": "PENDING", "url": "https://github.com/owner/repo/pull/1#pullrequestreview-123"}, + }, + }, + }, + }, + }), + ), + githubv4mock.NewMutationMatcher( + struct { + AddPullRequestReviewThread struct { + Thread struct { + ID githubv4.ID + } + } `graphql:"addPullRequestReviewThread(input: $input)"` + }{}, + githubv4.AddPullRequestReviewThreadInput{ + Path: githubv4.String("src/main.go"), + Body: githubv4.String("This needs a fix"), + SubjectType: githubv4mock.Ptr(githubv4.PullRequestReviewThreadSubjectTypeLine), + Line: githubv4mock.Ptr(githubv4.Int(42)), + Side: githubv4mock.Ptr(githubv4.DiffSideRight), + PullRequestReviewID: githubv4mock.Ptr(githubv4.ID("PRR_123")), + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "addPullRequestReviewThread": map[string]any{ + "thread": map[string]any{"id": "PRRT_456"}, + }, + }), + ), + ) + gqlClient := githubv4.NewClient(mockedClient) + deps := BaseDeps{GQLClient: gqlClient} + serverTool := GranularAddPullRequestReviewComment(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(1), + "path": "src/main.go", + "body": "This needs a fix", + "subjectType": "LINE", + "line": float64(42), + "side": "RIGHT", + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + assert.False(t, result.IsError) +} diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 05af64cab..81161626b 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -677,7 +677,7 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool // SubIssueWrite creates a tool to add a sub-issue to a parent issue. func SubIssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "sub_issue_write", @@ -787,6 +787,8 @@ Options are: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } }) + st.FeatureFlagDisable = FeatureFlagIssuesGranular + return st } func AddSubIssue(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, subIssueID int, replaceParent bool) (*mcp.CallToolResult, error) { @@ -970,7 +972,7 @@ func SearchIssues(t translations.TranslationHelperFunc) inventory.ServerTool { const IssueWriteUIResourceURI = "ui://github-mcp-server/issue-write" func IssueWrite(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + st := NewTool( ToolsetMetadataIssues, mcp.Tool{ Name: "issue_write", @@ -1179,6 +1181,8 @@ Options are: return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil } }) + st.FeatureFlagDisable = FeatureFlagIssuesGranular + return st } func CreateIssue(ctx context.Context, client *github.Client, owner string, repo string, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string) (*mcp.CallToolResult, error) { diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index e0338e409..ffe1bf7e3 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -6,6 +6,7 @@ import ( "fmt" "maps" + ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" @@ -84,12 +85,16 @@ func issueUpdateTool( return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - issue, _, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueReq) + issue, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueReq) if err != nil { - return utils.NewToolResultErrorFromErr("failed to update issue", err), nil, nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update issue", resp, err), nil, nil } + defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(issue) + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", issue.GetID()), + URL: issue.GetHTMLURL(), + }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } @@ -164,12 +169,16 @@ func GranularCreateIssue(t translations.TranslationHelperFunc) inventory.ServerT return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - issue, _, err := client.Issues.Create(ctx, owner, repo, issueReq) + issue, resp, err := client.Issues.Create(ctx, owner, repo, issueReq) if err != nil { - return utils.NewToolResultErrorFromErr("failed to create issue", err), nil, nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create issue", resp, err), nil, nil } + defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(issue) + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", issue.GetID()), + URL: issue.GetHTMLURL(), + }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } @@ -235,15 +244,12 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor }, []string{"assignees"}, func(args map[string]any) (*github.IssueRequest, error) { - raw, _ := OptionalParam[[]any](args, "assignees") - if len(raw) == 0 { - return nil, fmt.Errorf("missing required parameter: assignees") + assignees, err := OptionalStringArrayParam(args, "assignees") + if err != nil { + return nil, err } - assignees := make([]string, 0, len(raw)) - for _, v := range raw { - if s, ok := v.(string); ok { - assignees = append(assignees, s) - } + if len(assignees) == 0 { + return nil, fmt.Errorf("missing required parameter: assignees") } return &github.IssueRequest{Assignees: &assignees}, nil }, @@ -265,15 +271,12 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S }, []string{"labels"}, func(args map[string]any) (*github.IssueRequest, error) { - raw, _ := OptionalParam[[]any](args, "labels") - if len(raw) == 0 { - return nil, fmt.Errorf("missing required parameter: labels") + labels, err := OptionalStringArrayParam(args, "labels") + if err != nil { + return nil, err } - labels := make([]string, 0, len(raw)) - for _, v := range raw { - if s, ok := v.(string); ok { - labels = append(labels, s) - } + if len(labels) == 0 { + return nil, fmt.Errorf("missing required parameter: labels") } return &github.IssueRequest{Labels: &labels}, nil }, @@ -290,7 +293,7 @@ func GranularUpdateIssueMilestone(t translations.TranslationHelperFunc) inventor "milestone": { Type: "integer", Description: "The milestone number to set on the issue", - Minimum: jsonschema.Ptr(0.0), + Minimum: jsonschema.Ptr(1.0), }, }, []string{"milestone"}, diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 731db4931..89578f151 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -751,7 +751,7 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo Required: []string{"owner", "repo", "pullNumber"}, } - return NewTool( + st := NewTool( ToolsetMetadataPullRequests, mcp.Tool{ Name: "update_pull_request", @@ -990,6 +990,8 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo return utils.NewToolResultText(string(r)), nil, nil }) + st.FeatureFlagDisable = FeatureFlagPullRequestsGranular + return st } // AddReplyToPullRequestComment creates a tool to add a reply to an existing pull request comment. @@ -1555,7 +1557,7 @@ func PullRequestReviewWrite(t translations.TranslationHelperFunc) inventory.Serv Required: []string{"method", "owner", "repo", "pullNumber"}, } - return NewTool( + st := NewTool( ToolsetMetadataPullRequests, mcp.Tool{ Name: "pull_request_review_write", @@ -1607,6 +1609,8 @@ Available methods: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", params.Method)), nil, nil } }) + st.FeatureFlagDisable = FeatureFlagPullRequestsGranular + return st } func CreatePullRequestReview(ctx context.Context, client *githubv4.Client, params PullRequestReviewWriteParams) (*mcp.CallToolResult, error) { @@ -1947,7 +1951,7 @@ func AddCommentToPendingReview(t translations.TranslationHelperFunc) inventory.S Required: []string{"owner", "repo", "pullNumber", "path", "body", "subjectType"}, } - return NewTool( + st := NewTool( ToolsetMetadataPullRequests, mcp.Tool{ Name: "add_comment_to_pending_review", @@ -2074,6 +2078,8 @@ func AddCommentToPendingReview(t translations.TranslationHelperFunc) inventory.S // API implementation details to the LLM. return utils.NewToolResultText("pull request review comment successfully added to pending review"), nil, nil }) + st.FeatureFlagDisable = FeatureFlagPullRequestsGranular + return st } // newGQLString like takes something that approximates a string (of which there are many types in shurcooL/githubv4) diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index 661f2b4f5..f5a4a49ec 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -86,12 +86,16 @@ func prUpdateTool( return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - pr, _, err := client.PullRequests.Edit(ctx, owner, repo, pullNumber, prReq) + pr, resp, err := client.PullRequests.Edit(ctx, owner, repo, pullNumber, prReq) if err != nil { - return utils.NewToolResultErrorFromErr("failed to update pull request", err), nil, nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update pull request", resp, err), nil, nil } + defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(pr) + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", pr.GetID()), + URL: pr.GetHTMLURL(), + }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } @@ -204,7 +208,11 @@ func GranularUpdatePullRequestDraftState(t translations.TranslationHelperFunc) i if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - draft, err := RequiredParam[bool](args, "draft") + // Use presence check + OptionalParam since RequiredParam rejects false (zero-value for bool) + if _, ok := args["draft"]; !ok { + return utils.NewToolResultError("missing required parameter: draft"), nil, nil + } + draft, err := OptionalParam[bool](args, "draft") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } @@ -309,15 +317,12 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - rawReviewers, _ := OptionalParam[[]any](args, "reviewers") - if len(rawReviewers) == 0 { - return utils.NewToolResultError("missing required parameter: reviewers"), nil, nil + reviewers, err := OptionalStringArrayParam(args, "reviewers") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } - reviewers := make([]string, 0, len(rawReviewers)) - for _, v := range rawReviewers { - if s, ok := v.(string); ok { - reviewers = append(reviewers, s) - } + if len(reviewers) == 0 { + return utils.NewToolResultError("missing required parameter: reviewers"), nil, nil } client, err := deps.GetClient(ctx) @@ -325,12 +330,16 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - pr, _, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{Reviewers: reviewers}) + pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{Reviewers: reviewers}) if err != nil { - return utils.NewToolResultErrorFromErr("failed to request reviewers", err), nil, nil + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to request reviewers", resp, err), nil, nil } + defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(pr) + r, err := json.Marshal(MinimalResponse{ + ID: fmt.Sprintf("%d", pr.GetID()), + URL: pr.GetHTMLURL(), + }) if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } @@ -667,6 +676,15 @@ func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) i startLinePtr = &sl } + // Convert optional string params: pass nil (not empty string) when absent + var sidePtr, startSidePtr *string + if side != "" { + sidePtr = &side + } + if startSide != "" { + startSidePtr = &startSide + } + if err := gqlClient.Mutate( ctx, &addPullRequestReviewThreadMutation, @@ -675,9 +693,9 @@ func GranularAddPullRequestReviewComment(t translations.TranslationHelperFunc) i Body: githubv4.String(body), SubjectType: newGQLStringlikePtr[githubv4.PullRequestReviewThreadSubjectType](&subjectType), Line: newGQLIntPtr(linePtr), - Side: newGQLStringlikePtr[githubv4.DiffSide](&side), + Side: newGQLStringlikePtr[githubv4.DiffSide](sidePtr), StartLine: newGQLIntPtr(startLinePtr), - StartSide: newGQLStringlikePtr[githubv4.DiffSide](&startSide), + StartSide: newGQLStringlikePtr[githubv4.DiffSide](startSidePtr), PullRequestReviewID: &review.ID, }, nil,