From 39c28df52f96d81fb755e22e5af9f75a65894aa0 Mon Sep 17 00:00:00 2001 From: Ruhaan_Pathan <139214773+ruhaanpathan@users.noreply.github.com> Date: Sun, 19 Jul 2026 16:55:11 +0530 Subject: [PATCH 1/3] feat(pull_requests): add MCP tools for GitHub Stacked PRs (#2905) --- pkg/github/pullrequests_stacks.go | 519 +++++++++++++++++++++++++ pkg/github/pullrequests_stacks_test.go | 213 ++++++++++ pkg/github/tools.go | 5 + pkg/github/toolset_instructions.go | 4 +- 4 files changed, 740 insertions(+), 1 deletion(-) create mode 100644 pkg/github/pullrequests_stacks.go create mode 100644 pkg/github/pullrequests_stacks_test.go diff --git a/pkg/github/pullrequests_stacks.go b/pkg/github/pullrequests_stacks.go new file mode 100644 index 0000000000..cdb53ddeae --- /dev/null +++ b/pkg/github/pullrequests_stacks.go @@ -0,0 +1,519 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + + 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" + "github.com/github/github-mcp-server/pkg/utils" +) + +// StackLayer represents a pull request layer inside a stack. +type StackLayer struct { + PullNumber int `json:"pull_number,omitempty"` + Head string `json:"head,omitempty"` + Base string `json:"base,omitempty"` + Title string `json:"title,omitempty"` + State string `json:"state,omitempty"` + Mergeable *bool `json:"mergeable,omitempty"` + ReviewDecision string `json:"review_decision,omitempty"` +} + +// Stack represents a GitHub native pull request stack. +type Stack struct { + ID int64 `json:"id,omitempty"` + StackNumber int `json:"stack_number,omitempty"` + Title string `json:"title,omitempty"` + Base string `json:"base,omitempty"` + PullRequests []StackLayer `json:"pull_requests,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` +} + +// LinkStackInput represents the JSON payload to create/link a stack. +type LinkStackInput struct { + Base string `json:"base,omitempty"` + PullNumbers []int `json:"pull_numbers"` +} + +// UpdateStackInput represents the JSON payload to update a stack. +type UpdateStackInput struct { + Base string `json:"base,omitempty"` + PullNumbers []int `json:"pull_numbers,omitempty"` +} + +func parseIntArray(args map[string]any, p string) ([]int, error) { + val, ok := args[p] + if !ok { + return nil, nil + } + switch v := val.(type) { + case []any: + res := make([]int, len(v)) + for i, item := range v { + num, err := toInt(item) + if err != nil { + return nil, fmt.Errorf("item at index %d in %s is invalid: %w", i, p, err) + } + res[i] = num + } + return res, nil + case []int: + return v, nil + case []float64: + res := make([]int, len(v)) + for i, num := range v { + res[i] = int(num) + } + return res, nil + default: + return nil, fmt.Errorf("parameter %s is not an array", p) + } +} + +// GetStack creates a tool to fetch details for a pull request stack. +func GetStack(t translations.TranslationHelperFunc) inventory.ServerTool { + schema := &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "stackNumber": { + Type: "number", + Description: "Stack number", + }, + "pullNumber": { + Type: "number", + Description: "Pull request number contained within the target stack", + }, + }, + Required: []string{"owner", "repo"}, + } + + return NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "get_stack", + Description: t("TOOL_GET_STACK_DESCRIPTION", "Get details of a specific pull request stack in a GitHub repository."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_GET_STACK_TITLE", "Get pull request stack details"), + ReadOnlyHint: true, + }, + InputSchema: schema, + }, + []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 + } + + stackNumber, err := OptionalIntParam(args, "stackNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + pullNumber, err := OptionalIntParam(args, "pullNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + if stackNumber == 0 && pullNumber == 0 { + return utils.NewToolResultError("must provide either stackNumber or pullNumber"), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + var urlStr string + if stackNumber != 0 { + urlStr = fmt.Sprintf("repos/%s/%s/stacks/%d", owner, repo, stackNumber) + } else { + urlStr = fmt.Sprintf("repos/%s/%s/stacks?pull_request=%d", owner, repo, pullNumber) + } + + req, err := client.NewRequest(http.MethodGet, urlStr, nil) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil + } + + if stackNumber != 0 { + var stack Stack + resp, err := client.Do(ctx, req, &stack) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get pull request stack", resp, err), nil, nil + } + + r, err := json.Marshal(stack) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + } + + var stacks []Stack + resp, err := client.Do(ctx, req, &stacks) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get pull request stack", resp, err), nil, nil + } + + r, err := json.Marshal(stacks) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// ListStacks creates a tool to list pull request stacks in a repository. +func ListStacks(t translations.TranslationHelperFunc) inventory.ServerTool { + schema := &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + }, + Required: []string{"owner", "repo"}, + } + WithPagination(schema) + + return NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "list_stacks", + Description: t("TOOL_LIST_STACKS_DESCRIPTION", "List pull request stacks in a GitHub repository."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_LIST_STACKS_TITLE", "List pull request stacks"), + ReadOnlyHint: true, + }, + InputSchema: schema, + }, + []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 + } + + pagination, err := OptionalPaginationParams(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 + } + + urlStr := fmt.Sprintf("repos/%s/%s/stacks?page=%d&per_page=%d", owner, repo, pagination.Page, pagination.PerPage) + req, err := client.NewRequest(http.MethodGet, urlStr, nil) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil + } + + var stacks []Stack + resp, err := client.Do(ctx, req, &stacks) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list pull request stacks", resp, err), nil, nil + } + + r, err := json.Marshal(stacks) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// LinkStack creates a tool to link PRs into a new stack. +func LinkStack(t translations.TranslationHelperFunc) inventory.ServerTool { + schema := &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "pullNumbers": { + Type: "array", + Description: "Ordered list of pull request numbers (bottom to top)", + Items: &jsonschema.Schema{ + Type: "number", + }, + }, + "base": { + Type: "string", + Description: "Base/trunk branch name", + }, + }, + Required: []string{"owner", "repo", "pullNumbers"}, + } + + return NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "link_stack", + Description: t("TOOL_LINK_STACK_DESCRIPTION", "Create or link a pull request stack from an ordered sequence of pull request numbers."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_LINK_STACK_TITLE", "Link pull request stack"), + ReadOnlyHint: false, + }, + InputSchema: schema, + }, + []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 + } + + pullNumbers, err := parseIntArray(args, "pullNumbers") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if len(pullNumbers) == 0 { + return utils.NewToolResultError("missing required parameter: pullNumbers"), nil, nil + } + + base, err := OptionalParam[string](args, "base") + 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 + } + + input := LinkStackInput{ + Base: base, + PullNumbers: pullNumbers, + } + + urlStr := fmt.Sprintf("repos/%s/%s/stacks", owner, repo) + req, err := client.NewRequest(http.MethodPost, urlStr, input) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil + } + + var stack Stack + resp, err := client.Do(ctx, req, &stack) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to link pull request stack", resp, err), nil, nil + } + + r, err := json.Marshal(stack) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// UpdateStack creates a tool to update an existing pull request stack. +func UpdateStack(t translations.TranslationHelperFunc) inventory.ServerTool { + schema := &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "stackNumber": { + Type: "number", + Description: "Stack number to update", + }, + "pullNumbers": { + Type: "array", + Description: "Updated ordered list of pull request numbers", + Items: &jsonschema.Schema{ + Type: "number", + }, + }, + "base": { + Type: "string", + Description: "Updated base/trunk branch name", + }, + }, + Required: []string{"owner", "repo", "stackNumber"}, + } + + return NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "update_stack", + Description: t("TOOL_UPDATE_STACK_DESCRIPTION", "Update an existing pull request stack's layers or base branch."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_UPDATE_STACK_TITLE", "Update pull request stack"), + ReadOnlyHint: false, + }, + InputSchema: schema, + }, + []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 + } + stackNumber, err := RequiredInt(args, "stackNumber") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + pullNumbers, err := parseIntArray(args, "pullNumbers") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + base, err := OptionalParam[string](args, "base") + 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 + } + + input := UpdateStackInput{ + Base: base, + PullNumbers: pullNumbers, + } + + urlStr := fmt.Sprintf("repos/%s/%s/stacks/%d", owner, repo, stackNumber) + req, err := client.NewRequest(http.MethodPatch, urlStr, input) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil + } + + var stack Stack + resp, err := client.Do(ctx, req, &stack) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update pull request stack", resp, err), nil, nil + } + + r, err := json.Marshal(stack) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + } + return utils.NewToolResultText(string(r)), nil, nil + }, + ) +} + +// DissolveStack creates a tool to dissolve a pull request stack. +func DissolveStack(t translations.TranslationHelperFunc) inventory.ServerTool { + schema := &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "stackNumber": { + Type: "number", + Description: "Stack number to dissolve", + }, + }, + Required: []string{"owner", "repo", "stackNumber"}, + } + + return NewTool( + ToolsetMetadataPullRequests, + mcp.Tool{ + Name: "dissolve_stack", + Description: t("TOOL_DISSOLVE_STACK_DESCRIPTION", "Dissolve a pull request stack object without deleting the underlying pull requests."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_DISSOLVE_STACK_TITLE", "Dissolve pull request stack"), + ReadOnlyHint: false, + }, + InputSchema: schema, + }, + []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 + } + stackNumber, err := RequiredInt(args, "stackNumber") + 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 + } + + urlStr := fmt.Sprintf("repos/%s/%s/stacks/%d", owner, repo, stackNumber) + req, err := client.NewRequest(http.MethodDelete, urlStr, nil) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil + } + + resp, err := client.Do(ctx, req, nil) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to dissolve pull request stack", resp, err), nil, nil + } + + return utils.NewToolResultText(fmt.Sprintf("Successfully dissolved stack %d in %s/%s", stackNumber, owner, repo)), nil, nil + }, + ) +} diff --git a/pkg/github/pullrequests_stacks_test.go b/pkg/github/pullrequests_stacks_test.go new file mode 100644 index 0000000000..cc88b56e88 --- /dev/null +++ b/pkg/github/pullrequests_stacks_test.go @@ -0,0 +1,213 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v89/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_GetStack_ToolDefinition(t *testing.T) { + serverTool := GetStack(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "get_stack", tool.Name) + assert.NotEmpty(t, tool.Description) + schema := tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "owner") + assert.Contains(t, schema.Properties, "repo") + assert.Contains(t, schema.Properties, "stackNumber") + assert.Contains(t, schema.Properties, "pullNumber") + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"}) +} + +func Test_ListStacks_ToolDefinition(t *testing.T) { + serverTool := ListStacks(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "list_stacks", tool.Name) + assert.NotEmpty(t, tool.Description) + schema := tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "owner") + assert.Contains(t, schema.Properties, "repo") + assert.Contains(t, schema.Properties, "page") + assert.Contains(t, schema.Properties, "perPage") + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"}) +} + +func Test_LinkStack_ToolDefinition(t *testing.T) { + serverTool := LinkStack(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "link_stack", tool.Name) + assert.NotEmpty(t, tool.Description) + schema := tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "owner") + assert.Contains(t, schema.Properties, "repo") + assert.Contains(t, schema.Properties, "pullNumbers") + assert.Contains(t, schema.Properties, "base") + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "pullNumbers"}) +} + +func Test_UpdateStack_ToolDefinition(t *testing.T) { + serverTool := UpdateStack(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "update_stack", tool.Name) + assert.NotEmpty(t, tool.Description) + schema := tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "owner") + assert.Contains(t, schema.Properties, "repo") + assert.Contains(t, schema.Properties, "stackNumber") + assert.Contains(t, schema.Properties, "pullNumbers") + assert.Contains(t, schema.Properties, "base") + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "stackNumber"}) +} + +func Test_DissolveStack_ToolDefinition(t *testing.T) { + serverTool := DissolveStack(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "dissolve_stack", tool.Name) + assert.NotEmpty(t, tool.Description) + schema := tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "owner") + assert.Contains(t, schema.Properties, "repo") + assert.Contains(t, schema.Properties, "stackNumber") + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "stackNumber"}) +} + +func Test_GetStack_Execution(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/owner/repo/stacks/10", func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "GET", r.Method) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(Stack{ + ID: 100, + StackNumber: 10, + Title: "Test Stack", + Base: "main", + PullRequests: []StackLayer{ + {PullNumber: 101, Head: "feature-1", Base: "main"}, + {PullNumber: 102, Head: "feature-2", Base: "feature-1"}, + }, + }) + }) + + ts := httptest.NewServer(mux) + defer ts.Close() + + client := github.NewClient(ts.Client()) + url, _ := url.Parse(ts.URL + "/") + client.BaseURL = url + + deps := ToolDependencies{ + GetClient: func(ctx context.Context) (*github.Client, error) { + return client, nil + }, + } + + serverTool := GetStack(translations.NullTranslationHelper) + handler := serverTool.HandlerFunc(deps) + + res, err := handler(context.Background(), &mcp.CallToolRequest{}, map[string]any{ + "owner": "owner", + "repo": "repo", + "stackNumber": 10, + }) + require.NoError(t, err) + assert.False(t, res.IsError) + assert.Contains(t, res.Content[0].(mcp.TextContent).Text, `"stack_number":10`) +} + +func Test_LinkStack_Execution(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/owner/repo/stacks", func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "POST", r.Method) + var body LinkStackInput + _ = json.NewDecoder(r.Body).Decode(&body) + assert.Equal(t, "main", body.Base) + assert.Equal(t, []int{101, 102}, body.PullNumbers) + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(Stack{ + ID: 200, + StackNumber: 15, + Base: body.Base, + }) + }) + + ts := httptest.NewServer(mux) + defer ts.Close() + + client := github.NewClient(ts.Client()) + url, _ := url.Parse(ts.URL + "/") + client.BaseURL = url + + deps := ToolDependencies{ + GetClient: func(ctx context.Context) (*github.Client, error) { + return client, nil + }, + } + + serverTool := LinkStack(translations.NullTranslationHelper) + handler := serverTool.HandlerFunc(deps) + + res, err := handler(context.Background(), &mcp.CallToolRequest{}, map[string]any{ + "owner": "owner", + "repo": "repo", + "base": "main", + "pullNumbers": []any{101, 102}, + }) + require.NoError(t, err) + assert.False(t, res.IsError) + assert.Contains(t, res.Content[0].(mcp.TextContent).Text, `"stack_number":15`) +} + +func Test_DissolveStack_Execution(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/owner/repo/stacks/10", func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "DELETE", r.Method) + w.WriteHeader(http.StatusNoContent) + }) + + ts := httptest.NewServer(mux) + defer ts.Close() + + client := github.NewClient(ts.Client()) + url, _ := url.Parse(ts.URL + "/") + client.BaseURL = url + + deps := ToolDependencies{ + GetClient: func(ctx context.Context) (*github.Client, error) { + return client, nil + }, + } + + serverTool := DissolveStack(translations.NullTranslationHelper) + handler := serverTool.HandlerFunc(deps) + + res, err := handler(context.Background(), &mcp.CallToolRequest{}, map[string]any{ + "owner": "owner", + "repo": "repo", + "stackNumber": 10, + }) + require.NoError(t, err) + assert.False(t, res.IsError) + assert.Contains(t, res.Content[0].(mcp.TextContent).Text, "Successfully dissolved stack 10") +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 6764edfc26..9f3ea62af0 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -288,6 +288,11 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent PullRequestReviewWriteWithResolutionReason(t, opts...), AddCommentToPendingReview(t), AddReplyToPullRequestComment(t), + GetStack(t), + ListStacks(t), + LinkStack(t), + UpdateStack(t), + DissolveStack(t), // Copilot tools AssignCopilotToIssue(t), diff --git a/pkg/github/toolset_instructions.go b/pkg/github/toolset_instructions.go index 3b3a54eadd..697c70cb5d 100644 --- a/pkg/github/toolset_instructions.go +++ b/pkg/github/toolset_instructions.go @@ -18,7 +18,9 @@ Check 'list_issue_types' first for organizations to use proper issue types. Use func generatePullRequestsToolsetInstructions(inv *inventory.Inventory) string { instructions := `## Pull Requests -PR review workflow: Always use 'pull_request_review_write' with method 'create' to create a pending review, then 'add_comment_to_pending_review' to add comments, and finally 'pull_request_review_write' with method 'submit_pending' to submit the review for complex reviews with line-specific comments.` +PR review workflow: Always use 'pull_request_review_write' with method 'create' to create a pending review, then 'add_comment_to_pending_review' to add comments, and finally 'pull_request_review_write' with method 'submit_pending' to submit the review for complex reviews with line-specific comments. + +Stacked PRs workflow: Use 'link_stack' to group dependent pull requests into a native GitHub stack, 'get_stack' or 'list_stacks' to inspect stack layers, 'update_stack' to update ordering or base branches, and 'dissolve_stack' to un-group stack layers.` if inv.HasToolset("repos") { instructions += ` From 0957ac62fbc4eff659b374d3588d6adf9b0b62e0 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 3 Sep 2026 15:51:55 +0200 Subject: [PATCH 2/3] fix(pull_requests): align stack tools with current API Use the 2026-03-10 native stack endpoints, consolidate the tool surface, and cover pagination, validation, scopes, and unstack outcomes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 18 + .../pull_request_stack_read.snap | 56 ++ .../pull_request_stack_write.snap | 53 ++ pkg/github/pullrequests_stacks.go | 724 ++++++++---------- pkg/github/pullrequests_stacks_test.go | 421 ++++++---- pkg/github/tools.go | 7 +- pkg/github/toolset_instructions.go | 2 +- 7 files changed, 740 insertions(+), 541 deletions(-) create mode 100644 pkg/github/__toolsnaps__/pull_request_stack_read.snap create mode 100644 pkg/github/__toolsnaps__/pull_request_stack_write.snap diff --git a/README.md b/README.md index 5b90f64a58..84a61a9efc 100644 --- a/README.md +++ b/README.md @@ -1296,6 +1296,24 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - `threadId`: The node ID of the review thread (e.g., PRRT_kwDOxxx). Required for resolve_thread and unresolve_thread methods. Get thread IDs from pull_request_read with method get_review_comments. (string, optional) +- **pull_request_stack_read** - Read pull request stacks + - **OAuth Challenge Scopes**: `repo` + - `method`: The read operation: `get` retrieves one stack by stackNumber; `list` lists repository stacks and can filter by pullNumber. (string, required) + - `owner`: Repository owner (string, required) + - `page`: Page number for pagination (min 1) (number, optional) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `pullNumber`: Filter listed stacks to the stack containing this repository pull request number. Used only when method is `list`. (number, optional) + - `repo`: Repository name (string, required) + - `stackNumber`: Stack number. Required when method is `get`. (number, optional) + +- **pull_request_stack_write** - Manage pull request stack + - **OAuth Challenge Scopes**: `repo` + - `method`: The write operation: `create`, `add`, or `unstack`. (string, required) + - `owner`: Repository owner (string, required) + - `pullNumbers`: Repository pull request numbers in bottom-to-top order. Required for `create` and `add`. (number[], optional) + - `repo`: Repository name (string, required) + - `stackNumber`: Stack number. Required for `add` and `unstack`. (number, optional) + - **search_pull_requests** - Search pull requests - **OAuth Challenge Scopes**: `repo` - `fields`: Subset of fields to return for each pull request result. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields; omitting 'body', 'reactions', and 'labels' in particular drops the largest per-result data. (string[], optional) diff --git a/pkg/github/__toolsnaps__/pull_request_stack_read.snap b/pkg/github/__toolsnaps__/pull_request_stack_read.snap new file mode 100644 index 0000000000..54d7f3e69e --- /dev/null +++ b/pkg/github/__toolsnaps__/pull_request_stack_read.snap @@ -0,0 +1,56 @@ +{ + "annotations": { + "idempotentHint": false, + "readOnlyHint": true, + "title": "Read pull request stacks" + }, + "description": "Read native GitHub pull request stacks. Use `get` for a stack number or `list` to enumerate stacks and optionally resolve the stack containing a pull request.", + "inputSchema": { + "properties": { + "method": { + "description": "The read operation: `get` retrieves one stack by stackNumber; `list` lists repository stacks and can filter by pullNumber.", + "enum": [ + "get", + "list" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "pullNumber": { + "description": "Filter listed stacks to the stack containing this repository pull request number. Used only when method is `list`.", + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "stackNumber": { + "description": "Stack number. Required when method is `get`.", + "minimum": 1, + "type": "number" + } + }, + "required": [ + "method", + "owner", + "repo" + ], + "type": "object" + }, + "name": "pull_request_stack_read" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/pull_request_stack_write.snap b/pkg/github/__toolsnaps__/pull_request_stack_write.snap new file mode 100644 index 0000000000..a45bc3d7f5 --- /dev/null +++ b/pkg/github/__toolsnaps__/pull_request_stack_write.snap @@ -0,0 +1,53 @@ +{ + "annotations": { + "destructiveHint": true, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false, + "title": "Manage pull request stack" + }, + "description": "Create, extend, or unstack a native GitHub pull request stack. `create` accepts 2-100 pullNumbers ordered bottom-to-top. `add` accepts 1-100 pullNumbers to append above the current top. `unstack` removes every removable unmerged pull request and may leave locked or queued pull requests in the stack. All pull requests must belong to the target repository, use branches in that repository, and form a linear base/head chain. These operations manage stack metadata only; they do not create pull requests, retarget bases, rebase commits, push branches, or merge.", + "inputSchema": { + "properties": { + "method": { + "description": "The write operation: `create`, `add`, or `unstack`.", + "enum": [ + "create", + "add", + "unstack" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "pullNumbers": { + "description": "Repository pull request numbers in bottom-to-top order. Required for `create` and `add`.", + "items": { + "minimum": 1, + "type": "number" + }, + "maxItems": 100, + "minItems": 1, + "type": "array" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "stackNumber": { + "description": "Stack number. Required for `add` and `unstack`.", + "minimum": 1, + "type": "number" + } + }, + "required": [ + "method", + "owner", + "repo" + ], + "type": "object" + }, + "name": "pull_request_stack_write" +} \ No newline at end of file diff --git a/pkg/github/pullrequests_stacks.go b/pkg/github/pullrequests_stacks.go index cdb53ddeae..e98e21315e 100644 --- a/pkg/github/pullrequests_stacks.go +++ b/pkg/github/pullrequests_stacks.go @@ -2,88 +2,91 @@ package github import ( "context" - "encoding/json" "fmt" "net/http" - - "github.com/google/jsonschema-go/jsonschema" - "github.com/modelcontextprotocol/go-sdk/mcp" + "net/url" + "strconv" ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/ifc" "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/v89/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" ) -// StackLayer represents a pull request layer inside a stack. -type StackLayer struct { - PullNumber int `json:"pull_number,omitempty"` - Head string `json:"head,omitempty"` - Base string `json:"base,omitempty"` - Title string `json:"title,omitempty"` - State string `json:"state,omitempty"` - Mergeable *bool `json:"mergeable,omitempty"` - ReviewDecision string `json:"review_decision,omitempty"` +const pullRequestStacksAPIVersion = "2026-03-10" + +// PullRequestStackRepository identifies a repository referenced by a stack layer. +type PullRequestStackRepository struct { + ID int64 `json:"id"` + Name string `json:"name"` + URL string `json:"url"` } -// Stack represents a GitHub native pull request stack. -type Stack struct { - ID int64 `json:"id,omitempty"` - StackNumber int `json:"stack_number,omitempty"` - Title string `json:"title,omitempty"` - Base string `json:"base,omitempty"` - PullRequests []StackLayer `json:"pull_requests,omitempty"` - CreatedAt string `json:"created_at,omitempty"` - UpdatedAt string `json:"updated_at,omitempty"` +// PullRequestStackRef identifies a branch and commit referenced by a stack. +type PullRequestStackRef struct { + Ref string `json:"ref"` + SHA string `json:"sha,omitempty"` + Repo *PullRequestStackRepository `json:"repo,omitempty"` } -// LinkStackInput represents the JSON payload to create/link a stack. -type LinkStackInput struct { - Base string `json:"base,omitempty"` - PullNumbers []int `json:"pull_numbers"` +// PullRequestStackPullRequest is the compact pull request representation returned +// by the stack tools. +type PullRequestStackPullRequest struct { + ID int64 `json:"id,omitempty"` + NodeID string `json:"node_id,omitempty"` + Number int `json:"number"` + URL string `json:"url,omitempty"` + HTMLURL string `json:"html_url,omitempty"` + State string `json:"state"` + MergedAt *string `json:"merged_at"` + Draft bool `json:"draft"` + Head PullRequestStackRef `json:"head"` + Base *PullRequestStackRef `json:"base,omitempty"` } -// UpdateStackInput represents the JSON payload to update a stack. -type UpdateStackInput struct { - Base string `json:"base,omitempty"` - PullNumbers []int `json:"pull_numbers,omitempty"` +// PullRequestStack is a compact representation of a native GitHub pull request +// stack. PullRequests are ordered from the bottom of the stack to the top. +type PullRequestStack struct { + ID int64 `json:"id"` + Number int `json:"number"` + NodeID string `json:"node_id"` + URL string `json:"url"` + Base PullRequestStackRef `json:"base"` + Open bool `json:"open"` + CreatedAt string `json:"created_at"` + PullRequests []PullRequestStackPullRequest `json:"pull_requests"` } -func parseIntArray(args map[string]any, p string) ([]int, error) { - val, ok := args[p] - if !ok { - return nil, nil - } - switch v := val.(type) { - case []any: - res := make([]int, len(v)) - for i, item := range v { - num, err := toInt(item) - if err != nil { - return nil, fmt.Errorf("item at index %d in %s is invalid: %w", i, p, err) - } - res[i] = num - } - return res, nil - case []int: - return v, nil - case []float64: - res := make([]int, len(v)) - for i, num := range v { - res[i] = int(num) - } - return res, nil - default: - return nil, fmt.Errorf("parameter %s is not an array", p) - } +type pullRequestStackInput struct { + PullRequests []int `json:"pull_requests"` +} + +type pullRequestStackListResult struct { + Stacks []PullRequestStack `json:"stacks"` + PageInfo map[string]any `json:"pageInfo"` +} + +type pullRequestStackUnstackResult struct { + Dissolved bool `json:"dissolved"` + StackNumber int `json:"stack_number"` + Stack *PullRequestStack `json:"stack,omitempty"` } -// GetStack creates a tool to fetch details for a pull request stack. -func GetStack(t translations.TranslationHelperFunc) inventory.ServerTool { - schema := &jsonschema.Schema{ +// PullRequestStackRead creates a tool for reading native pull request stacks. +func PullRequestStackRead(t translations.TranslationHelperFunc) inventory.ServerTool { + schema := WithPagination(&jsonschema.Schema{ Type: "object", Properties: map[string]*jsonschema.Schema{ + "method": { + Type: "string", + Description: "The read operation: `get` retrieves one stack by stackNumber; `list` lists repository stacks and can filter by pullNumber.", + Enum: []any{"get", "list"}, + }, "owner": { Type: "string", Description: "Repository owner", @@ -94,426 +97,357 @@ func GetStack(t translations.TranslationHelperFunc) inventory.ServerTool { }, "stackNumber": { Type: "number", - Description: "Stack number", + Description: "Stack number. Required when method is `get`.", + Minimum: jsonschema.Ptr(1.0), }, "pullNumber": { Type: "number", - Description: "Pull request number contained within the target stack", + Description: "Filter listed stacks to the stack containing this repository pull request number. Used only when method is `list`.", + Minimum: jsonschema.Ptr(1.0), }, }, - Required: []string{"owner", "repo"}, - } + Required: []string{"method", "owner", "repo"}, + }) return NewTool( ToolsetMetadataPullRequests, mcp.Tool{ - Name: "get_stack", - Description: t("TOOL_GET_STACK_DESCRIPTION", "Get details of a specific pull request stack in a GitHub repository."), + Name: "pull_request_stack_read", + Description: t("TOOL_PULL_REQUEST_STACK_READ_DESCRIPTION", "Read native GitHub pull request stacks. Use `get` for a stack number or `list` to enumerate stacks and optionally resolve the stack containing a pull request."), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_GET_STACK_TITLE", "Get pull request stack details"), + Title: t("TOOL_PULL_REQUEST_STACK_READ_USER_TITLE", "Read pull request stacks"), ReadOnlyHint: true, }, InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + scopes.PublicRead(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") + method, err := RequiredParam[string](args, "method") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - - stackNumber, err := OptionalIntParam(args, "stackNumber") + owner, err := RequiredParam[string](args, "owner") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - - pullNumber, err := OptionalIntParam(args, "pullNumber") + repo, err := RequiredParam[string](args, "repo") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - - if stackNumber == 0 && pullNumber == 0 { - return utils.NewToolResultError("must provide either stackNumber or pullNumber"), nil, nil - } - client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - var urlStr string - if stackNumber != 0 { - urlStr = fmt.Sprintf("repos/%s/%s/stacks/%d", owner, repo, stackNumber) - } else { - urlStr = fmt.Sprintf("repos/%s/%s/stacks?pull_request=%d", owner, repo, pullNumber) - } - - req, err := client.NewRequest(http.MethodGet, urlStr, nil) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil - } - - if stackNumber != 0 { - var stack Stack - resp, err := client.Do(ctx, req, &stack) + var result *mcp.CallToolResult + switch method { + case "get": + stackNumber, err := requiredPullRequestStackNumber(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + stack, resp, err := GetPullRequestStack(ctx, client, owner, repo, stackNumber) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get pull request stack", resp, err), nil, nil } - - r, err := json.Marshal(stack) + result = MarshalledTextResult(stack) + case "list": + pullNumber, err := OptionalIntParam(args, "pullNumber") if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } - return utils.NewToolResultText(string(r)), nil, nil - } - - var stacks []Stack - resp, err := client.Do(ctx, req, &stacks) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get pull request stack", resp, err), nil, nil - } - - r, err := json.Marshal(stacks) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil + if _, provided := args["pullNumber"]; provided && pullNumber < 1 { + return utils.NewToolResultError("parameter pullNumber must be greater than zero"), nil, nil + } + pagination, err := OptionalPaginationParams(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + stacks, resp, err := ListPullRequestStacks(ctx, client, owner, repo, pullNumber, pagination) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list pull request stacks", resp, err), nil, nil + } + result = MarshalledTextResult(pullRequestStackListResult{ + Stacks: stacks, + PageInfo: map[string]any{ + "hasNextPage": resp.NextPage != 0, + "nextPage": resp.NextPage, + }, + }) + default: + return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil + } + + result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata) + return result, nil, nil }, ) } -// ListStacks creates a tool to list pull request stacks in a repository. -func ListStacks(t translations.TranslationHelperFunc) inventory.ServerTool { - schema := &jsonschema.Schema{ - Type: "object", - Properties: map[string]*jsonschema.Schema{ - "owner": { - Type: "string", - Description: "Repository owner", - }, - "repo": { - Type: "string", - Description: "Repository name", - }, - }, - Required: []string{"owner", "repo"}, - } - WithPagination(schema) - +// PullRequestStackWrite creates a tool for creating, extending, or unstacking +// native pull request stacks. +func PullRequestStackWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return NewTool( ToolsetMetadataPullRequests, mcp.Tool{ - Name: "list_stacks", - Description: t("TOOL_LIST_STACKS_DESCRIPTION", "List pull request stacks in a GitHub repository."), + Name: "pull_request_stack_write", + Description: t("TOOL_PULL_REQUEST_STACK_WRITE_DESCRIPTION", + "Create, extend, or unstack a native GitHub pull request stack. "+ + "`create` accepts 2-100 pullNumbers ordered bottom-to-top. "+ + "`add` accepts 1-100 pullNumbers to append above the current top. "+ + "`unstack` removes every removable unmerged pull request and may leave locked or queued pull requests in the stack. "+ + "All pull requests must belong to the target repository, use branches in that repository, and form a linear base/head chain. "+ + "These operations manage stack metadata only; they do not create pull requests, retarget bases, rebase commits, push branches, or merge."), Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_LIST_STACKS_TITLE", "List pull request stacks"), - ReadOnlyHint: true, + Title: t("TOOL_PULL_REQUEST_STACK_WRITE_USER_TITLE", "Manage pull request stack"), + ReadOnlyHint: false, + DestructiveHint: jsonschema.Ptr(true), + OpenWorldHint: jsonschema.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "method": { + Type: "string", + Description: "The write operation: `create`, `add`, or `unstack`.", + Enum: []any{"create", "add", "unstack"}, + }, + "owner": { + Type: "string", + Description: "Repository owner", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "stackNumber": { + Type: "number", + Description: "Stack number. Required for `add` and `unstack`.", + Minimum: jsonschema.Ptr(1.0), + }, + "pullNumbers": { + Type: "array", + Description: "Repository pull request numbers in bottom-to-top order. Required for `create` and `add`.", + Items: &jsonschema.Schema{ + Type: "number", + Minimum: jsonschema.Ptr(1.0), + }, + MinItems: jsonschema.Ptr(1), + MaxItems: jsonschema.Ptr(100), + }, + }, + Required: []string{"method", "owner", "repo"}, }, - InputSchema: schema, }, - []scopes.Scope{scopes.Repo}, + publicRepositoryWriteScopeAccess(), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - owner, err := RequiredParam[string](args, "owner") + method, err := RequiredParam[string](args, "method") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - repo, err := RequiredParam[string](args, "repo") + owner, err := RequiredParam[string](args, "owner") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - - pagination, err := OptionalPaginationParams(args) + repo, err := RequiredParam[string](args, "repo") 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 } - urlStr := fmt.Sprintf("repos/%s/%s/stacks?page=%d&per_page=%d", owner, repo, pagination.Page, pagination.PerPage) - req, err := client.NewRequest(http.MethodGet, urlStr, nil) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil - } - - var stacks []Stack - resp, err := client.Do(ctx, req, &stacks) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list pull request stacks", resp, err), nil, nil + var result *mcp.CallToolResult + switch method { + case "create": + pullNumbers, err := parsePullRequestStackNumbers(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if len(pullNumbers) < 2 { + return utils.NewToolResultError("method create requires at least two pullNumbers"), nil, nil + } + stack, resp, err := CreatePullRequestStack(ctx, client, owner, repo, pullNumbers) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create pull request stack", resp, err), nil, nil + } + result = MarshalledTextResult(stack) + case "add": + stackNumber, err := requiredPullRequestStackNumber(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + pullNumbers, err := parsePullRequestStackNumbers(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if len(pullNumbers) == 0 { + return utils.NewToolResultError("method add requires at least one pullNumber"), nil, nil + } + stack, resp, err := AddPullRequestsToStack(ctx, client, owner, repo, stackNumber, pullNumbers) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add pull requests to stack", resp, err), nil, nil + } + result = MarshalledTextResult(stack) + case "unstack": + stackNumber, err := requiredPullRequestStackNumber(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + stack, resp, err := UnstackPullRequests(ctx, client, owner, repo, stackNumber) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to unstack pull requests", resp, err), nil, nil + } + result = MarshalledTextResult(pullRequestStackUnstackResult{ + Dissolved: stack == nil, + StackNumber: stackNumber, + Stack: stack, + }) + default: + return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } - r, err := json.Marshal(stacks) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil + result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata) + return result, nil, nil }, ) } -// LinkStack creates a tool to link PRs into a new stack. -func LinkStack(t translations.TranslationHelperFunc) inventory.ServerTool { - schema := &jsonschema.Schema{ - Type: "object", - Properties: map[string]*jsonschema.Schema{ - "owner": { - Type: "string", - Description: "Repository owner", - }, - "repo": { - Type: "string", - Description: "Repository name", - }, - "pullNumbers": { - Type: "array", - Description: "Ordered list of pull request numbers (bottom to top)", - Items: &jsonschema.Schema{ - Type: "number", - }, - }, - "base": { - Type: "string", - Description: "Base/trunk branch name", - }, - }, - Required: []string{"owner", "repo", "pullNumbers"}, +// GetPullRequestStack gets a native pull request stack by number. +func GetPullRequestStack(ctx context.Context, client *github.Client, owner, repo string, stackNumber int) (*PullRequestStack, *github.Response, error) { + apiURL := fmt.Sprintf("repos/%s/%s/stacks/%d", owner, repo, stackNumber) + req, err := newPullRequestStackRequest(ctx, client, http.MethodGet, apiURL, nil) + if err != nil { + return nil, nil, err } - return NewTool( - ToolsetMetadataPullRequests, - mcp.Tool{ - Name: "link_stack", - Description: t("TOOL_LINK_STACK_DESCRIPTION", "Create or link a pull request stack from an ordered sequence of pull request numbers."), - Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_LINK_STACK_TITLE", "Link pull request stack"), - ReadOnlyHint: false, - }, - InputSchema: schema, - }, - []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 - } - - pullNumbers, err := parseIntArray(args, "pullNumbers") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - if len(pullNumbers) == 0 { - return utils.NewToolResultError("missing required parameter: pullNumbers"), nil, nil - } - - base, err := OptionalParam[string](args, "base") - 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 - } - - input := LinkStackInput{ - Base: base, - PullNumbers: pullNumbers, - } - - urlStr := fmt.Sprintf("repos/%s/%s/stacks", owner, repo) - req, err := client.NewRequest(http.MethodPost, urlStr, input) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil - } - - var stack Stack - resp, err := client.Do(ctx, req, &stack) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to link pull request stack", resp, err), nil, nil - } - - r, err := json.Marshal(stack) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil - }, - ) + var stack PullRequestStack + resp, err := client.Do(req, &stack) + return &stack, resp, err } -// UpdateStack creates a tool to update an existing pull request stack. -func UpdateStack(t translations.TranslationHelperFunc) inventory.ServerTool { - schema := &jsonschema.Schema{ - Type: "object", - Properties: map[string]*jsonschema.Schema{ - "owner": { - Type: "string", - Description: "Repository owner", - }, - "repo": { - Type: "string", - Description: "Repository name", - }, - "stackNumber": { - Type: "number", - Description: "Stack number to update", - }, - "pullNumbers": { - Type: "array", - Description: "Updated ordered list of pull request numbers", - Items: &jsonschema.Schema{ - Type: "number", - }, - }, - "base": { - Type: "string", - Description: "Updated base/trunk branch name", - }, - }, - Required: []string{"owner", "repo", "stackNumber"}, +// ListPullRequestStacks lists native pull request stacks, optionally filtering +// to the stack containing pullNumber. +func ListPullRequestStacks(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, pagination PaginationParams) ([]PullRequestStack, *github.Response, error) { + query := url.Values{ + "page": {strconv.Itoa(pagination.Page)}, + "per_page": {strconv.Itoa(pagination.PerPage)}, + } + if pullNumber > 0 { + query.Set("pull_request", strconv.Itoa(pullNumber)) + } + apiURL := fmt.Sprintf("repos/%s/%s/stacks?%s", owner, repo, query.Encode()) + req, err := newPullRequestStackRequest(ctx, client, http.MethodGet, apiURL, nil) + if err != nil { + return nil, nil, err } - return NewTool( - ToolsetMetadataPullRequests, - mcp.Tool{ - Name: "update_stack", - Description: t("TOOL_UPDATE_STACK_DESCRIPTION", "Update an existing pull request stack's layers or base branch."), - Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_UPDATE_STACK_TITLE", "Update pull request stack"), - ReadOnlyHint: false, - }, - InputSchema: schema, - }, - []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 - } - stackNumber, err := RequiredInt(args, "stackNumber") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } + var stacks []PullRequestStack + resp, err := client.Do(req, &stacks) + return stacks, resp, err +} - pullNumbers, err := parseIntArray(args, "pullNumbers") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } +// CreatePullRequestStack creates a native stack from pull request numbers +// ordered from bottom to top. +func CreatePullRequestStack(ctx context.Context, client *github.Client, owner, repo string, pullNumbers []int) (*PullRequestStack, *github.Response, error) { + return mutatePullRequestStack(ctx, client, owner, repo, "", pullNumbers) +} - base, err := OptionalParam[string](args, "base") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } +// AddPullRequestsToStack appends pull request numbers above the current stack top. +func AddPullRequestsToStack(ctx context.Context, client *github.Client, owner, repo string, stackNumber int, pullNumbers []int) (*PullRequestStack, *github.Response, error) { + return mutatePullRequestStack(ctx, client, owner, repo, fmt.Sprintf("%d/add", stackNumber), pullNumbers) +} - client, err := deps.GetClient(ctx) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil - } +// UnstackPullRequests removes every removable unmerged pull request from a +// native stack. A nil stack means the stack was dissolved. +func UnstackPullRequests(ctx context.Context, client *github.Client, owner, repo string, stackNumber int) (*PullRequestStack, *github.Response, error) { + apiURL := fmt.Sprintf("repos/%s/%s/stacks/%d/unstack", owner, repo, stackNumber) + req, err := newPullRequestStackRequest(ctx, client, http.MethodPost, apiURL, nil) + if err != nil { + return nil, nil, err + } - input := UpdateStackInput{ - Base: base, - PullNumbers: pullNumbers, - } + var stack PullRequestStack + resp, err := client.Do(req, &stack) + if err != nil { + return nil, resp, err + } + if resp.StatusCode == http.StatusNoContent { + return nil, resp, nil + } + return &stack, resp, nil +} - urlStr := fmt.Sprintf("repos/%s/%s/stacks/%d", owner, repo, stackNumber) - req, err := client.NewRequest(http.MethodPatch, urlStr, input) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil - } +func mutatePullRequestStack(ctx context.Context, client *github.Client, owner, repo, suffix string, pullNumbers []int) (*PullRequestStack, *github.Response, error) { + apiURL := fmt.Sprintf("repos/%s/%s/stacks", owner, repo) + if suffix != "" { + apiURL += "/" + suffix + } + req, err := newPullRequestStackRequest(ctx, client, http.MethodPost, apiURL, pullRequestStackInput{PullRequests: pullNumbers}) + if err != nil { + return nil, nil, err + } - var stack Stack - resp, err := client.Do(ctx, req, &stack) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update pull request stack", resp, err), nil, nil - } + var stack PullRequestStack + resp, err := client.Do(req, &stack) + return &stack, resp, err +} - r, err := json.Marshal(stack) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil - } - return utils.NewToolResultText(string(r)), nil, nil - }, - ) +func newPullRequestStackRequest(ctx context.Context, client *github.Client, method, apiURL string, body any) (*http.Request, error) { + return client.NewRequest(ctx, method, apiURL, body, github.WithVersion(pullRequestStacksAPIVersion)) } -// DissolveStack creates a tool to dissolve a pull request stack. -func DissolveStack(t translations.TranslationHelperFunc) inventory.ServerTool { - schema := &jsonschema.Schema{ - Type: "object", - Properties: map[string]*jsonschema.Schema{ - "owner": { - Type: "string", - Description: "Repository owner", - }, - "repo": { - Type: "string", - Description: "Repository name", - }, - "stackNumber": { - Type: "number", - Description: "Stack number to dissolve", - }, - }, - Required: []string{"owner", "repo", "stackNumber"}, +func requiredPullRequestStackNumber(args map[string]any) (int, error) { + stackNumber, err := RequiredInt(args, "stackNumber") + if err != nil { + return 0, err } + if stackNumber < 1 { + return 0, fmt.Errorf("parameter stackNumber must be greater than zero") + } + return stackNumber, nil +} - return NewTool( - ToolsetMetadataPullRequests, - mcp.Tool{ - Name: "dissolve_stack", - Description: t("TOOL_DISSOLVE_STACK_DESCRIPTION", "Dissolve a pull request stack object without deleting the underlying pull requests."), - Annotations: &mcp.ToolAnnotations{ - Title: t("TOOL_DISSOLVE_STACK_TITLE", "Dissolve pull request stack"), - ReadOnlyHint: false, - }, - InputSchema: schema, - }, - []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 - } - stackNumber, err := RequiredInt(args, "stackNumber") - 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 - } +func parsePullRequestStackNumbers(args map[string]any) ([]int, error) { + value, ok := args["pullNumbers"] + if !ok { + return nil, nil + } - urlStr := fmt.Sprintf("repos/%s/%s/stacks/%d", owner, repo, stackNumber) - req, err := client.NewRequest(http.MethodDelete, urlStr, nil) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to create request", err), nil, nil + var pullNumbers []int + switch values := value.(type) { + case []int: + pullNumbers = append([]int(nil), values...) + case []any: + pullNumbers = make([]int, len(values)) + for i, value := range values { + if number, ok := value.(int); ok { + pullNumbers[i] = number + continue } - - resp, err := client.Do(ctx, req, nil) + number, err := toInt(value) if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to dissolve pull request stack", resp, err), nil, nil + return nil, fmt.Errorf("pullNumbers[%d] is invalid: %w", i, err) } + pullNumbers[i] = number + } + default: + return nil, fmt.Errorf("parameter pullNumbers is not an array, is %T", value) + } - return utils.NewToolResultText(fmt.Sprintf("Successfully dissolved stack %d in %s/%s", stackNumber, owner, repo)), nil, nil - }, - ) + if len(pullNumbers) > 100 { + return nil, fmt.Errorf("pullNumbers must contain at most 100 items") + } + seen := make(map[int]struct{}, len(pullNumbers)) + for i, number := range pullNumbers { + if number < 1 { + return nil, fmt.Errorf("pullNumbers[%d] must be greater than zero", i) + } + if _, exists := seen[number]; exists { + return nil, fmt.Errorf("pullNumbers[%d] duplicates pull request %d", i, number) + } + seen[number] = struct{}{} + } + return pullNumbers, nil } diff --git a/pkg/github/pullrequests_stacks_test.go b/pkg/github/pullrequests_stacks_test.go index cc88b56e88..85814c1319 100644 --- a/pkg/github/pullrequests_stacks_test.go +++ b/pkg/github/pullrequests_stacks_test.go @@ -5,10 +5,10 @@ import ( "encoding/json" "net/http" "net/http/httptest" - "net/url" "testing" "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v89/github" "github.com/google/jsonschema-go/jsonschema" @@ -17,197 +17,338 @@ import ( "github.com/stretchr/testify/require" ) -func Test_GetStack_ToolDefinition(t *testing.T) { - serverTool := GetStack(translations.NullTranslationHelper) +func Test_PullRequestStackRead_ToolDefinition(t *testing.T) { + serverTool := PullRequestStackRead(translations.NullTranslationHelper) tool := serverTool.Tool require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "get_stack", tool.Name) - assert.NotEmpty(t, tool.Description) + assert.Equal(t, "pull_request_stack_read", tool.Name) + assert.True(t, tool.Annotations.ReadOnlyHint) schema := tool.InputSchema.(*jsonschema.Schema) - assert.Contains(t, schema.Properties, "owner") - assert.Contains(t, schema.Properties, "repo") + assert.ElementsMatch(t, []string{"method", "owner", "repo"}, schema.Required) + assert.ElementsMatch(t, []any{"get", "list"}, schema.Properties["method"].Enum) assert.Contains(t, schema.Properties, "stackNumber") assert.Contains(t, schema.Properties, "pullNumber") - assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"}) -} - -func Test_ListStacks_ToolDefinition(t *testing.T) { - serverTool := ListStacks(translations.NullTranslationHelper) - tool := serverTool.Tool - require.NoError(t, toolsnaps.Test(tool.Name, tool)) - - assert.Equal(t, "list_stacks", tool.Name) - assert.NotEmpty(t, tool.Description) - schema := tool.InputSchema.(*jsonschema.Schema) - assert.Contains(t, schema.Properties, "owner") - assert.Contains(t, schema.Properties, "repo") assert.Contains(t, schema.Properties, "page") assert.Contains(t, schema.Properties, "perPage") - assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"}) + assert.True(t, serverTool.ScopeAccess.Visible(nil)) } -func Test_LinkStack_ToolDefinition(t *testing.T) { - serverTool := LinkStack(translations.NullTranslationHelper) +func Test_PullRequestStackWrite_ToolDefinition(t *testing.T) { + serverTool := PullRequestStackWrite(translations.NullTranslationHelper) tool := serverTool.Tool require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "link_stack", tool.Name) - assert.NotEmpty(t, tool.Description) + assert.Equal(t, "pull_request_stack_write", tool.Name) + assert.False(t, tool.Annotations.ReadOnlyHint) + require.NotNil(t, tool.Annotations.DestructiveHint) + assert.True(t, *tool.Annotations.DestructiveHint) schema := tool.InputSchema.(*jsonschema.Schema) - assert.Contains(t, schema.Properties, "owner") - assert.Contains(t, schema.Properties, "repo") - assert.Contains(t, schema.Properties, "pullNumbers") - assert.Contains(t, schema.Properties, "base") - assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "pullNumbers"}) -} - -func Test_UpdateStack_ToolDefinition(t *testing.T) { - serverTool := UpdateStack(translations.NullTranslationHelper) - tool := serverTool.Tool - require.NoError(t, toolsnaps.Test(tool.Name, tool)) - - assert.Equal(t, "update_stack", tool.Name) - assert.NotEmpty(t, tool.Description) - schema := tool.InputSchema.(*jsonschema.Schema) - assert.Contains(t, schema.Properties, "owner") - assert.Contains(t, schema.Properties, "repo") + assert.ElementsMatch(t, []string{"method", "owner", "repo"}, schema.Required) + assert.ElementsMatch(t, []any{"create", "add", "unstack"}, schema.Properties["method"].Enum) assert.Contains(t, schema.Properties, "stackNumber") assert.Contains(t, schema.Properties, "pullNumbers") - assert.Contains(t, schema.Properties, "base") - assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "stackNumber"}) + assert.Equal(t, 1, *schema.Properties["pullNumbers"].MinItems) + assert.Equal(t, 100, *schema.Properties["pullNumbers"].MaxItems) + assert.True(t, serverTool.ScopeAccess.Visible([]string{"public_repo"})) + assert.False(t, serverTool.ScopeAccess.Visible(nil)) } -func Test_DissolveStack_ToolDefinition(t *testing.T) { - serverTool := DissolveStack(translations.NullTranslationHelper) - tool := serverTool.Tool - require.NoError(t, toolsnaps.Test(tool.Name, tool)) +func Test_PullRequestStackRead_Get(t *testing.T) { + client := newPullRequestStackTestClient(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/repos/owner/repo/stacks/42", r.URL.Path) + assert.Equal(t, pullRequestStacksAPIVersion, r.Header.Get("X-GitHub-Api-Version")) + writePullRequestStack(t, w, http.StatusOK) + }) - assert.Equal(t, "dissolve_stack", tool.Name) - assert.NotEmpty(t, tool.Description) - schema := tool.InputSchema.(*jsonschema.Schema) - assert.Contains(t, schema.Properties, "owner") - assert.Contains(t, schema.Properties, "repo") - assert.Contains(t, schema.Properties, "stackNumber") - assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "stackNumber"}) + result := callPullRequestStackTool(t, PullRequestStackRead, client, map[string]any{ + "method": "get", + "owner": "owner", + "repo": "repo", + "stackNumber": float64(42), + }) + + assert.False(t, result.IsError) + text := getTextResult(t, result).Text + assert.Contains(t, text, `"number":42`) + assert.Contains(t, text, `"base":{"ref":"main"}`) + assert.Contains(t, text, `"head":{"ref":"feature","sha":"abc123"`) + assert.NotContains(t, text, `"title"`) + assert.NotContains(t, text, `"user"`) } -func Test_GetStack_Execution(t *testing.T) { - mux := http.NewServeMux() - mux.HandleFunc("/repos/owner/repo/stacks/10", func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "GET", r.Method) +func Test_PullRequestStackRead_List(t *testing.T) { + client := newPullRequestStackTestClient(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/repos/owner/repo/stacks", r.URL.Path) + assert.Equal(t, "17", r.URL.Query().Get("pull_request")) + assert.Equal(t, "2", r.URL.Query().Get("page")) + assert.Equal(t, "25", r.URL.Query().Get("per_page")) + assert.Equal(t, pullRequestStacksAPIVersion, r.Header.Get("X-GitHub-Api-Version")) + w.Header().Set("Link", `; rel="next"`) w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(Stack{ - ID: 100, - StackNumber: 10, - Title: "Test Stack", - Base: "main", - PullRequests: []StackLayer{ - {PullNumber: 101, Head: "feature-1", Base: "main"}, - {PullNumber: 102, Head: "feature-2", Base: "feature-1"}, - }, - }) + w.WriteHeader(http.StatusOK) + require.NoError(t, json.NewEncoder(w).Encode([]PullRequestStack{testPullRequestStack()})) }) - ts := httptest.NewServer(mux) - defer ts.Close() + result := callPullRequestStackTool(t, PullRequestStackRead, client, map[string]any{ + "method": "list", + "owner": "owner", + "repo": "repo", + "pullNumber": float64(17), + "page": float64(2), + "perPage": float64(25), + }) - client := github.NewClient(ts.Client()) - url, _ := url.Parse(ts.URL + "/") - client.BaseURL = url + assert.False(t, result.IsError) + text := getTextResult(t, result).Text + assert.Contains(t, text, `"stacks":[{"id":9876543`) + assert.Contains(t, text, `"pageInfo":{"hasNextPage":true,"nextPage":3}`) +} - deps := ToolDependencies{ - GetClient: func(ctx context.Context) (*github.Client, error) { - return client, nil - }, - } +func Test_PullRequestStackWrite_Create(t *testing.T) { + client := newPullRequestStackTestClient(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/repos/owner/repo/stacks", r.URL.Path) + assert.Equal(t, pullRequestStacksAPIVersion, r.Header.Get("X-GitHub-Api-Version")) + var input pullRequestStackInput + require.NoError(t, json.NewDecoder(r.Body).Decode(&input)) + assert.Equal(t, []int{101, 102}, input.PullRequests) + writePullRequestStack(t, w, http.StatusCreated) + }) - serverTool := GetStack(translations.NullTranslationHelper) - handler := serverTool.HandlerFunc(deps) + result := callPullRequestStackTool(t, PullRequestStackWrite, client, map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "pullNumbers": []any{float64(101), "102"}, + }) - res, err := handler(context.Background(), &mcp.CallToolRequest{}, map[string]any{ + assert.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, `"number":42`) +} + +func Test_PullRequestStackWrite_Add(t *testing.T) { + client := newPullRequestStackTestClient(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/repos/owner/repo/stacks/42/add", r.URL.Path) + var input pullRequestStackInput + require.NoError(t, json.NewDecoder(r.Body).Decode(&input)) + assert.Equal(t, []int{103}, input.PullRequests) + writePullRequestStack(t, w, http.StatusOK) + }) + + result := callPullRequestStackTool(t, PullRequestStackWrite, client, map[string]any{ + "method": "add", "owner": "owner", "repo": "repo", - "stackNumber": 10, + "stackNumber": float64(42), + "pullNumbers": []int{103}, }) - require.NoError(t, err) - assert.False(t, res.IsError) - assert.Contains(t, res.Content[0].(mcp.TextContent).Text, `"stack_number":10`) + + assert.False(t, result.IsError) } -func Test_LinkStack_Execution(t *testing.T) { - mux := http.NewServeMux() - mux.HandleFunc("/repos/owner/repo/stacks", func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "POST", r.Method) - var body LinkStackInput - _ = json.NewDecoder(r.Body).Decode(&body) - assert.Equal(t, "main", body.Base) - assert.Equal(t, []int{101, 102}, body.PullNumbers) +func Test_PullRequestStackWrite_Unstack(t *testing.T) { + t.Run("remaining locked pull requests", func(t *testing.T) { + client := newPullRequestStackTestClient(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/repos/owner/repo/stacks/42/unstack", r.URL.Path) + writePullRequestStack(t, w, http.StatusOK) + }) - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(Stack{ - ID: 200, - StackNumber: 15, - Base: body.Base, + result := callPullRequestStackTool(t, PullRequestStackWrite, client, map[string]any{ + "method": "unstack", + "owner": "owner", + "repo": "repo", + "stackNumber": float64(42), }) + + assert.False(t, result.IsError) + text := getTextResult(t, result).Text + assert.Contains(t, text, `"dissolved":false`) + assert.Contains(t, text, `"stack":{"id":9876543`) }) - ts := httptest.NewServer(mux) - defer ts.Close() + t.Run("dissolved stack", func(t *testing.T) { + client := newPullRequestStackTestClient(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/repos/owner/repo/stacks/42/unstack", r.URL.Path) + w.WriteHeader(http.StatusNoContent) + }) + + result := callPullRequestStackTool(t, PullRequestStackWrite, client, map[string]any{ + "method": "unstack", + "owner": "owner", + "repo": "repo", + "stackNumber": float64(42), + }) - client := github.NewClient(ts.Client()) - url, _ := url.Parse(ts.URL + "/") - client.BaseURL = url + assert.False(t, result.IsError) + assert.JSONEq(t, `{"dissolved":true,"stack_number":42}`, getTextResult(t, result).Text) + }) +} - deps := ToolDependencies{ - GetClient: func(ctx context.Context) (*github.Client, error) { - return client, nil +func Test_PullRequestStackTool_Validation(t *testing.T) { + client := newPullRequestStackTestClient(t, func(http.ResponseWriter, *http.Request) { + t.Fatal("validation should fail before making a request") + }) + + tests := []struct { + name string + tool func(translations.TranslationHelperFunc) inventory.ServerTool + args map[string]any + want string + }{ + { + name: "get requires stack number", + tool: PullRequestStackRead, + args: map[string]any{"method": "get", "owner": "owner", "repo": "repo"}, + want: "missing required parameter: stackNumber", + }, + { + name: "get rejects negative stack number", + tool: PullRequestStackRead, + args: map[string]any{"method": "get", "owner": "owner", "repo": "repo", "stackNumber": float64(-1)}, + want: "parameter stackNumber must be greater than zero", + }, + { + name: "list rejects zero pull number", + tool: PullRequestStackRead, + args: map[string]any{"method": "list", "owner": "owner", "repo": "repo", "pullNumber": float64(0)}, + want: "parameter pullNumber must be greater than zero", + }, + { + name: "create requires two pull requests", + tool: PullRequestStackWrite, + args: map[string]any{"method": "create", "owner": "owner", "repo": "repo", "pullNumbers": []any{float64(1)}}, + want: "method create requires at least two pullNumbers", + }, + { + name: "add requires pull requests", + tool: PullRequestStackWrite, + args: map[string]any{"method": "add", "owner": "owner", "repo": "repo", "stackNumber": float64(1)}, + want: "method add requires at least one pullNumber", + }, + { + name: "duplicate pull request", + tool: PullRequestStackWrite, + args: map[string]any{"method": "create", "owner": "owner", "repo": "repo", "pullNumbers": []any{float64(1), float64(1)}}, + want: "duplicates pull request 1", + }, + { + name: "invalid pull request", + tool: PullRequestStackWrite, + args: map[string]any{"method": "create", "owner": "owner", "repo": "repo", "pullNumbers": []any{float64(1), float64(-2)}}, + want: "must be greater than zero", }, } - serverTool := LinkStack(translations.NullTranslationHelper) - handler := serverTool.HandlerFunc(deps) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := callPullRequestStackTool(t, tt.tool, client, tt.args) + assert.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, tt.want) + }) + } +} - res, err := handler(context.Background(), &mcp.CallToolRequest{}, map[string]any{ +func Test_PullRequestStackTool_APIError(t *testing.T) { + client := newPullRequestStackTestClient(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message":"Pull requests must form a stack"}`)) + }) + + result := callPullRequestStackTool(t, PullRequestStackWrite, client, map[string]any{ + "method": "create", "owner": "owner", "repo": "repo", - "base": "main", - "pullNumbers": []any{101, 102}, + "pullNumbers": []any{float64(101), float64(102)}, }) + + assert.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "failed to create pull request stack") + assert.Contains(t, getTextResult(t, result).Text, "Pull requests must form a stack") +} + +func callPullRequestStackTool( + t *testing.T, + tool func(translations.TranslationHelperFunc) inventory.ServerTool, + client *github.Client, + args map[string]any, +) *mcp.CallToolResult { + t.Helper() + deps := BaseDeps{Client: client} + request := createMCPRequest(args) + serverTool := tool(translations.NullTranslationHelper) + result, err := serverTool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) - assert.False(t, res.IsError) - assert.Contains(t, res.Content[0].(mcp.TextContent).Text, `"stack_number":15`) + require.NotNil(t, result) + return result } -func Test_DissolveStack_Execution(t *testing.T) { - mux := http.NewServeMux() - mux.HandleFunc("/repos/owner/repo/stacks/10", func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "DELETE", r.Method) - w.WriteHeader(http.StatusNoContent) - }) +func newPullRequestStackTestClient(t *testing.T, handler http.HandlerFunc) *github.Client { + t.Helper() + server := httptest.NewServer(handler) + t.Cleanup(server.Close) - ts := httptest.NewServer(mux) - defer ts.Close() + baseURL := server.URL + "/" + client, err := github.NewClient( + github.WithHTTPClient(server.Client()), + github.WithURLs(&baseURL, nil), + ) + require.NoError(t, err) + return client +} - client := github.NewClient(ts.Client()) - url, _ := url.Parse(ts.URL + "/") - client.BaseURL = url +func writePullRequestStack(t *testing.T, w http.ResponseWriter, status int) { + t.Helper() + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + require.NoError(t, json.NewEncoder(w).Encode(testPullRequestStack())) +} - deps := ToolDependencies{ - GetClient: func(ctx context.Context) (*github.Client, error) { - return client, nil +func testPullRequestStack() PullRequestStack { + mergedAt := "2026-08-01T12:00:00Z" + return PullRequestStack{ + ID: 9876543, + Number: 42, + NodeID: "S_kwDOABCDEF4AAAAA", + URL: "https://api.github.test/repos/owner/repo/stacks/42", + Base: PullRequestStackRef{Ref: "main"}, + Open: true, + CreatedAt: "2026-08-01T10:00:00Z", + PullRequests: []PullRequestStackPullRequest{ + { + ID: 100001, + NodeID: "PR_kwDOABCDEF4AAAAA", + Number: 101, + URL: "https://api.github.test/repos/owner/repo/pulls/101", + HTMLURL: "https://github.test/owner/repo/pull/101", + State: "closed", + MergedAt: &mergedAt, + Draft: false, + Head: PullRequestStackRef{ + Ref: "feature", + SHA: "abc123", + Repo: &PullRequestStackRepository{ + ID: 1, + Name: "repo", + URL: "https://api.github.test/repos/owner/repo", + }, + }, + Base: &PullRequestStackRef{ + Ref: "main", + SHA: "def456", + Repo: &PullRequestStackRepository{ + ID: 1, + Name: "repo", + URL: "https://api.github.test/repos/owner/repo", + }, + }, + }, }, } - - serverTool := DissolveStack(translations.NullTranslationHelper) - handler := serverTool.HandlerFunc(deps) - - res, err := handler(context.Background(), &mcp.CallToolRequest{}, map[string]any{ - "owner": "owner", - "repo": "repo", - "stackNumber": 10, - }) - require.NoError(t, err) - assert.False(t, res.IsError) - assert.Contains(t, res.Content[0].(mcp.TextContent).Text, "Successfully dissolved stack 10") } diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 9f3ea62af0..36c4632977 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -288,11 +288,8 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent PullRequestReviewWriteWithResolutionReason(t, opts...), AddCommentToPendingReview(t), AddReplyToPullRequestComment(t), - GetStack(t), - ListStacks(t), - LinkStack(t), - UpdateStack(t), - DissolveStack(t), + PullRequestStackRead(t), + PullRequestStackWrite(t), // Copilot tools AssignCopilotToIssue(t), diff --git a/pkg/github/toolset_instructions.go b/pkg/github/toolset_instructions.go index 697c70cb5d..2ab95a29c5 100644 --- a/pkg/github/toolset_instructions.go +++ b/pkg/github/toolset_instructions.go @@ -20,7 +20,7 @@ func generatePullRequestsToolsetInstructions(inv *inventory.Inventory) string { PR review workflow: Always use 'pull_request_review_write' with method 'create' to create a pending review, then 'add_comment_to_pending_review' to add comments, and finally 'pull_request_review_write' with method 'submit_pending' to submit the review for complex reviews with line-specific comments. -Stacked PRs workflow: Use 'link_stack' to group dependent pull requests into a native GitHub stack, 'get_stack' or 'list_stacks' to inspect stack layers, 'update_stack' to update ordering or base branches, and 'dissolve_stack' to un-group stack layers.` +Stacked PRs workflow: Use 'pull_request_stack_read' to inspect native stack metadata. Use 'pull_request_stack_write' with method 'create' for 2-100 repository pull requests ordered bottom-to-top, 'add' only to append new pull requests above the current top, and 'unstack' to remove every removable unmerged pull request. Native stack operations require same-repository branches in an existing linear base/head chain; they do not create pull requests, retarget bases, rebase commits, push branches, or merge. After 'unstack', inspect the result because locked or queued pull requests can remain.` if inv.HasToolset("repos") { instructions += ` From 7e496d51a3e60743c53a3ca6ae3633a8d48f3b43 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 3 Sep 2026 16:26:16 +0200 Subject: [PATCH 3/3] fix(pull_requests): gate stack tools on supported hosts Hide native stack tools and instructions on GHES, and conservatively label stack responses as repository user content. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f37b1980-3ec0-4b4e-9795-c9d8b448555e --- pkg/github/pullrequests_stacks.go | 22 +++-- pkg/github/pullrequests_stacks_test.go | 114 ++++++++++++++++++++++++- pkg/github/tools.go | 4 +- pkg/github/toolset_instructions.go | 21 ++++- 4 files changed, 149 insertions(+), 12 deletions(-) diff --git a/pkg/github/pullrequests_stacks.go b/pkg/github/pullrequests_stacks.go index e98e21315e..93670c06c8 100644 --- a/pkg/github/pullrequests_stacks.go +++ b/pkg/github/pullrequests_stacks.go @@ -78,7 +78,8 @@ type pullRequestStackUnstackResult struct { } // PullRequestStackRead creates a tool for reading native pull request stacks. -func PullRequestStackRead(t translations.TranslationHelperFunc) inventory.ServerTool { +func PullRequestStackRead(t translations.TranslationHelperFunc, opts ...ToolOption) inventory.ServerTool { + cfg := newToolConfig(opts) schema := WithPagination(&jsonschema.Schema{ Type: "object", Properties: map[string]*jsonschema.Schema{ @@ -109,7 +110,7 @@ func PullRequestStackRead(t translations.TranslationHelperFunc) inventory.Server Required: []string{"method", "owner", "repo"}, }) - return NewTool( + st := NewTool( ToolsetMetadataPullRequests, mcp.Tool{ Name: "pull_request_stack_read", @@ -178,16 +179,21 @@ func PullRequestStackRead(t translations.TranslationHelperFunc) inventory.Server return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata) + result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoUserContent) return result, nil, nil }, ) + if cfg.hostType == utils.HostTypeGHES { + st.Enabled = func(context.Context) (bool, error) { return false, nil } + } + return st } // PullRequestStackWrite creates a tool for creating, extending, or unstacking // native pull request stacks. -func PullRequestStackWrite(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( +func PullRequestStackWrite(t translations.TranslationHelperFunc, opts ...ToolOption) inventory.ServerTool { + cfg := newToolConfig(opts) + st := NewTool( ToolsetMetadataPullRequests, mcp.Tool{ Name: "pull_request_stack_write", @@ -308,10 +314,14 @@ func PullRequestStackWrite(t translations.TranslationHelperFunc) inventory.Serve return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata) + result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoUserContent) return result, nil, nil }, ) + if cfg.hostType == utils.HostTypeGHES { + st.Enabled = func(context.Context) (bool, error) { return false, nil } + } + return st } // GetPullRequestStack gets a native pull request stack by number. diff --git a/pkg/github/pullrequests_stacks_test.go b/pkg/github/pullrequests_stacks_test.go index 85814c1319..94ee86bfe9 100644 --- a/pkg/github/pullrequests_stacks_test.go +++ b/pkg/github/pullrequests_stacks_test.go @@ -10,6 +10,7 @@ import ( "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/pkg/utils" "github.com/google/go-github/v89/github" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -54,6 +55,42 @@ func Test_PullRequestStackWrite_ToolDefinition(t *testing.T) { assert.False(t, serverTool.ScopeAccess.Visible(nil)) } +func Test_PullRequestStackTools_HostAvailability(t *testing.T) { + tests := []struct { + name string + host utils.HostType + wantStack bool + }{ + {name: "dotcom", host: utils.HostTypeDotcom, wantStack: true}, + {name: "GHES", host: utils.HostTypeGHES, wantStack: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inv, err := NewInventory(translations.NullTranslationHelper, WithHost(tt.host)). + WithToolsets([]string{"pull_requests"}). + WithFeatureChecker(featureCheckerFor()). + WithServerInstructions(). + Build() + require.NoError(t, err) + + available := make(map[string]bool) + for _, tool := range inv.ToolsForRegistration(context.Background()) { + available[tool.Tool.Name] = true + } + assert.Equal(t, tt.wantStack, available["pull_request_stack_read"]) + assert.Equal(t, tt.wantStack, available["pull_request_stack_write"]) + if tt.wantStack { + assert.Contains(t, inv.Instructions(), "pull_request_stack_read") + assert.Contains(t, inv.Instructions(), "pull_request_stack_write") + } else { + assert.NotContains(t, inv.Instructions(), "pull_request_stack_read") + assert.NotContains(t, inv.Instructions(), "pull_request_stack_write") + } + }) + } +} + func Test_PullRequestStackRead_Get(t *testing.T) { client := newPullRequestStackTestClient(t, func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodGet, r.Method) @@ -197,7 +234,7 @@ func Test_PullRequestStackTool_Validation(t *testing.T) { tests := []struct { name string - tool func(translations.TranslationHelperFunc) inventory.ServerTool + tool pullRequestStackToolConstructor args map[string]any want string }{ @@ -273,14 +310,87 @@ func Test_PullRequestStackTool_APIError(t *testing.T) { assert.Contains(t, getTextResult(t, result).Text, "Pull requests must form a stack") } +func Test_PullRequestStackTools_IFCLabels(t *testing.T) { + tests := []struct { + name string + tool pullRequestStackToolConstructor + args map[string]any + status int + }{ + { + name: "read response", + tool: PullRequestStackRead, + args: map[string]any{ + "method": "get", + "owner": "owner", + "repo": "repo", + "stackNumber": float64(42), + }, + status: http.StatusOK, + }, + { + name: "write response", + tool: PullRequestStackWrite, + args: map[string]any{ + "method": "create", + "owner": "owner", + "repo": "repo", + "pullNumbers": []any{float64(101), float64(102)}, + }, + status: http.StatusCreated, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := newPullRequestStackTestClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/repos/owner/repo", "/repositories/owner/repo": + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(map[string]any{ + "name": "repo", + "private": false, + })) + default: + writePullRequestStack(t, w, tt.status) + } + }) + deps := BaseDeps{ + Client: client, + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), + } + + result := callPullRequestStackToolWithDeps(t, tt.tool, deps, tt.args) + + require.False(t, result.IsError) + require.NotNil(t, result.Meta) + ifcMap := unmarshalIFC(t, result.Meta["ifc"]) + assert.Equal(t, "untrusted", ifcMap["integrity"]) + assert.Equal(t, "public", ifcMap["confidentiality"]) + }) + } +} + +type pullRequestStackToolConstructor func(translations.TranslationHelperFunc, ...ToolOption) inventory.ServerTool + func callPullRequestStackTool( t *testing.T, - tool func(translations.TranslationHelperFunc) inventory.ServerTool, + tool pullRequestStackToolConstructor, client *github.Client, args map[string]any, ) *mcp.CallToolResult { t.Helper() deps := BaseDeps{Client: client} + return callPullRequestStackToolWithDeps(t, tool, deps, args) +} + +func callPullRequestStackToolWithDeps( + t *testing.T, + tool pullRequestStackToolConstructor, + deps BaseDeps, + args map[string]any, +) *mcp.CallToolResult { + t.Helper() request := createMCPRequest(args) serverTool := tool(translations.NullTranslationHelper) result, err := serverTool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 36c4632977..ddc92b50c5 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -288,8 +288,8 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent PullRequestReviewWriteWithResolutionReason(t, opts...), AddCommentToPendingReview(t), AddReplyToPullRequestComment(t), - PullRequestStackRead(t), - PullRequestStackWrite(t), + PullRequestStackRead(t, opts...), + PullRequestStackWrite(t, opts...), // Copilot tools AssignCopilotToIssue(t), diff --git a/pkg/github/toolset_instructions.go b/pkg/github/toolset_instructions.go index 2ab95a29c5..37f458929c 100644 --- a/pkg/github/toolset_instructions.go +++ b/pkg/github/toolset_instructions.go @@ -1,6 +1,10 @@ package github -import "github.com/github/github-mcp-server/pkg/inventory" +import ( + "context" + + "github.com/github/github-mcp-server/pkg/inventory" +) // Toolset instruction functions - these generate context-aware instructions for each toolset. // They are called during inventory build to generate server instructions. @@ -18,9 +22,13 @@ Check 'list_issue_types' first for organizations to use proper issue types. Use func generatePullRequestsToolsetInstructions(inv *inventory.Inventory) string { instructions := `## Pull Requests -PR review workflow: Always use 'pull_request_review_write' with method 'create' to create a pending review, then 'add_comment_to_pending_review' to add comments, and finally 'pull_request_review_write' with method 'submit_pending' to submit the review for complex reviews with line-specific comments. +PR review workflow: Always use 'pull_request_review_write' with method 'create' to create a pending review, then 'add_comment_to_pending_review' to add comments, and finally 'pull_request_review_write' with method 'submit_pending' to submit the review for complex reviews with line-specific comments.` + + if inventoryHasAvailableTool(inv, "pull_request_stack_read") { + instructions += ` Stacked PRs workflow: Use 'pull_request_stack_read' to inspect native stack metadata. Use 'pull_request_stack_write' with method 'create' for 2-100 repository pull requests ordered bottom-to-top, 'add' only to append new pull requests above the current top, and 'unstack' to remove every removable unmerged pull request. Native stack operations require same-repository branches in an existing linear base/head chain; they do not create pull requests, retarget bases, rebase commits, push branches, or merge. After 'unstack', inspect the result because locked or queued pull requests can remain.` + } if inv.HasToolset("repos") { instructions += ` @@ -30,6 +38,15 @@ Before creating a pull request, search for pull request templates in the reposit return instructions } +func inventoryHasAvailableTool(inv *inventory.Inventory, name string) bool { + for _, tool := range inv.AvailableTools(context.Background()) { + if tool.Tool.Name == name { + return true + } + } + return false +} + func generateDiscussionsToolsetInstructions(_ *inventory.Inventory) string { return `## Discussions