Files
github--github-mcp-server/pkg/github/pullrequests_test.go
Matt Holloway 04c8dcbc8f Skip MCP App form when issue/PR write carries non-form params (#2589)
* Skip MCP App form when issue/PR write carries non-form params

When MCP Apps are enabled and the client supports UI, issue_write and
create_pull_request route the call to an interactive form. The form only
collects a subset of fields and rebuilds the submit payload from scratch,
so any parameter it cannot represent was silently dropped — e.g. labels,
assignees, milestone, type, state and issue_fields (priority) for
issue_write.

Skip the form and execute directly whenever the call carries a parameter
outside the set the form collects and re-sends. This generalizes the
previous state-only guard and is robust to future parameter additions
(an unrecognized param now bypasses the form rather than being lost).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Forward original tool params on MCP App form submit

The issue-write and pr-write forms rebuilt their submit payload from
scratch, so any parameter the form does not render was dropped on submit.
Spread the original toolInput first and override only the edited fields,
so unsupported params (e.g. issue_fields, labels, state) are preserved
when the user submits the form.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-06-01 17:13:19 +01:00

3949 lines
120 KiB
Go

package github
import (
"context"
"encoding/json"
"net/http"
"testing"
"time"
"github.com/github/github-mcp-server/internal/githubv4mock"
"github.com/github/github-mcp-server/internal/toolsnaps"
"github.com/github/github-mcp-server/pkg/translations"
"github.com/google/go-github/v87/github"
"github.com/google/jsonschema-go/jsonschema"
"github.com/shurcooL/githubv4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func Test_GetPullRequest(t *testing.T) {
// Verify tool definition once
serverTool := PullRequestRead(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_read", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
// Setup mock PR for success case
mockPR := &github.PullRequest{
Number: github.Ptr(42),
Title: github.Ptr("Test PR"),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
Head: &github.PullRequestBranch{
SHA: github.Ptr("abcd1234"),
Ref: github.Ptr("feature-branch"),
},
Base: &github.PullRequestBranch{
Ref: github.Ptr("main"),
},
Body: github.Ptr("This is a test PR"),
User: &github.User{
Login: github.Ptr("testuser"),
},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedPR *github.PullRequest
expectedErrMsg string
}{
{
name: "successful PR fetch",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
}),
requestArgs: map[string]any{
"method": "get",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: false,
expectedPR: mockPR,
},
{
name: "PR fetch fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
},
}),
requestArgs: map[string]any{
"method": "get",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(999),
},
expectError: true,
expectedErrMsg: "failed to get pull request",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient())
deps := BaseDeps{
Client: client,
GQLClient: gqlClient,
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Unmarshal and verify the minimal result
var returnedPR MinimalPullRequest
err = json.Unmarshal([]byte(textContent.Text), &returnedPR)
require.NoError(t, err)
assert.Equal(t, tc.expectedPR.GetNumber(), returnedPR.Number)
assert.Equal(t, tc.expectedPR.GetTitle(), returnedPR.Title)
assert.Equal(t, tc.expectedPR.GetState(), returnedPR.State)
assert.Equal(t, tc.expectedPR.GetHTMLURL(), returnedPR.HTMLURL)
})
}
}
func Test_UpdatePullRequest(t *testing.T) {
// Verify tool definition once
serverTool := UpdatePullRequest(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "update_pull_request", 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, "pullNumber")
assert.Contains(t, schema.Properties, "draft")
assert.Contains(t, schema.Properties, "title")
assert.Contains(t, schema.Properties, "body")
assert.Contains(t, schema.Properties, "state")
assert.Contains(t, schema.Properties, "base")
assert.Contains(t, schema.Properties, "maintainer_can_modify")
assert.Contains(t, schema.Properties, "reviewers")
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "pullNumber"})
// Setup mock PR for success case
mockUpdatedPR := &github.PullRequest{
Number: github.Ptr(42),
Title: github.Ptr("Updated Test PR Title"),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
Body: github.Ptr("Updated test PR body."),
MaintainerCanModify: github.Ptr(false),
Draft: github.Ptr(false),
Base: &github.PullRequestBranch{
Ref: github.Ptr("develop"),
},
}
mockClosedPR := &github.PullRequest{
Number: github.Ptr(42),
Title: github.Ptr("Test PR"),
State: github.Ptr("closed"), // State updated
}
// Mock PR for when there are no updates but we still need a response
mockPRWithReviewers := &github.PullRequest{
Number: github.Ptr(42),
Title: github.Ptr("Test PR"),
State: github.Ptr("open"),
RequestedReviewers: []*github.User{
{Login: github.Ptr("reviewer1")},
{Login: github.Ptr("reviewer2")},
},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedPR *github.PullRequest
expectedErrMsg string
}{
{
name: "successful PR update (title, body, base, maintainer_can_modify)",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PatchReposPullsByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{
"title": "Updated Test PR Title",
"body": "Updated test PR body.",
"base": "develop",
"maintainer_can_modify": false,
}).andThen(
mockResponse(t, http.StatusOK, mockUpdatedPR),
),
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockUpdatedPR),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"title": "Updated Test PR Title",
"body": "Updated test PR body.",
"base": "develop",
"maintainer_can_modify": false,
},
expectError: false,
expectedPR: mockUpdatedPR,
},
{
name: "successful PR update (state)",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PatchReposPullsByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{
"state": "closed",
}).andThen(
mockResponse(t, http.StatusOK, mockClosedPR),
),
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockClosedPR),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"state": "closed",
},
expectError: false,
expectedPR: mockClosedPR,
},
{
name: "successful PR update with reviewers",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPRWithReviewers),
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPRWithReviewers),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"reviewers": []any{"reviewer1", "reviewer2"},
},
expectError: false,
expectedPR: mockPRWithReviewers,
},
{
name: "successful PR update with user and team reviewers",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{
"reviewers": []any{"reviewer1"},
"team_reviewers": []any{"platform"},
}).andThen(mockResponse(t, http.StatusOK, mockPRWithReviewers)),
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPRWithReviewers),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"reviewers": []any{"reviewer1", "owner/platform"},
},
expectError: false,
expectedPR: mockPRWithReviewers,
},
{
name: "successful PR update (title only)",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PatchReposPullsByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{
"title": "Updated Test PR Title",
}).andThen(
mockResponse(t, http.StatusOK, mockUpdatedPR),
),
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockUpdatedPR),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"title": "Updated Test PR Title",
},
expectError: false,
expectedPR: mockUpdatedPR,
},
{
name: "no update parameters provided",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), // No API call expected
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
// No update fields
},
expectError: false, // Error is returned in the result, not as Go error
expectedErrMsg: "No update parameters provided",
},
{
name: "PR update fails (API error)",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PatchReposPullsByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusUnprocessableEntity)
_, _ = w.Write([]byte(`{"message": "Validation Failed"}`))
},
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"title": "Invalid Title Causing Error",
},
expectError: true,
expectedErrMsg: "failed to update pull request",
},
{
name: "request reviewers fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusUnprocessableEntity)
_, _ = w.Write([]byte(`{"message": "Invalid reviewers"}`))
},
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"reviewers": []any{"invalid-user"},
},
expectError: true,
expectedErrMsg: "failed to request reviewers",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
gqlClient := githubv4.NewClient(nil)
deps := BaseDeps{
Client: client,
GQLClient: gqlClient,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError || tc.expectedErrMsg != "" {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
if tc.expectedErrMsg != "" {
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
}
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content
textContent := getTextResult(t, result)
// Unmarshal and verify the minimal result
var updateResp MinimalResponse
err = json.Unmarshal([]byte(textContent.Text), &updateResp)
require.NoError(t, err)
assert.Equal(t, tc.expectedPR.GetHTMLURL(), updateResp.URL)
})
}
}
func Test_UpdatePullRequest_Draft(t *testing.T) {
// Setup mock PR for success case
mockUpdatedPR := &github.PullRequest{
Number: github.Ptr(42),
Title: github.Ptr("Test PR Title"),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
Body: github.Ptr("Test PR body."),
MaintainerCanModify: github.Ptr(false),
Draft: github.Ptr(false), // Updated to ready for review
Base: &github.PullRequestBranch{
Ref: github.Ptr("main"),
},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedPR *github.PullRequest
expectedErrMsg string
}{
{
name: "successful draft update to ready for review",
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
ID githubv4.ID
IsDraft githubv4.Boolean
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
},
githubv4mock.DataResponse(map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"id": "PR_kwDOA0xdyM50BPaO",
"isDraft": true, // Current state is draft
},
},
}),
),
githubv4mock.NewMutationMatcher(
struct {
MarkPullRequestReadyForReview struct {
PullRequest struct {
ID githubv4.ID
IsDraft githubv4.Boolean
}
} `graphql:"markPullRequestReadyForReview(input: $input)"`
}{},
githubv4.MarkPullRequestReadyForReviewInput{
PullRequestID: "PR_kwDOA0xdyM50BPaO",
},
nil,
githubv4mock.DataResponse(map[string]any{
"markPullRequestReadyForReview": map[string]any{
"pullRequest": map[string]any{
"id": "PR_kwDOA0xdyM50BPaO",
"isDraft": false,
},
},
}),
),
),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"draft": false,
},
expectError: false,
expectedPR: mockUpdatedPR,
},
{
name: "successful convert pull request to draft",
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
ID githubv4.ID
IsDraft githubv4.Boolean
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
},
githubv4mock.DataResponse(map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"id": "PR_kwDOA0xdyM50BPaO",
"isDraft": false, // Current state is draft
},
},
}),
),
githubv4mock.NewMutationMatcher(
struct {
ConvertPullRequestToDraft struct {
PullRequest struct {
ID githubv4.ID
IsDraft githubv4.Boolean
}
} `graphql:"convertPullRequestToDraft(input: $input)"`
}{},
githubv4.ConvertPullRequestToDraftInput{
PullRequestID: "PR_kwDOA0xdyM50BPaO",
},
nil,
githubv4mock.DataResponse(map[string]any{
"convertPullRequestToDraft": map[string]any{
"pullRequest": map[string]any{
"id": "PR_kwDOA0xdyM50BPaO",
"isDraft": true,
},
},
}),
),
),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"draft": true,
},
expectError: false,
expectedPR: mockUpdatedPR,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// For draft-only tests, we need to mock both GraphQL and the final REST GET call
restClient := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockUpdatedPR),
}))
gqlClient := githubv4.NewClient(tc.mockedClient)
serverTool := UpdatePullRequest(translations.NullTranslationHelper)
deps := BaseDeps{
Client: restClient,
GQLClient: gqlClient,
}
handler := serverTool.Handler(deps)
request := createMCPRequest(tc.requestArgs)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
if tc.expectError || tc.expectedErrMsg != "" {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
if tc.expectedErrMsg != "" {
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
}
return
}
require.NoError(t, err)
require.False(t, result.IsError)
textContent := getTextResult(t, result)
// Unmarshal and verify the minimal result
var updateResp MinimalResponse
err = json.Unmarshal([]byte(textContent.Text), &updateResp)
require.NoError(t, err)
assert.Equal(t, tc.expectedPR.GetHTMLURL(), updateResp.URL)
})
}
}
func Test_ListPullRequests(t *testing.T) {
// Verify tool definition once
serverTool := ListPullRequests(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "list_pull_requests", 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, "state")
assert.Contains(t, schema.Properties, "head")
assert.Contains(t, schema.Properties, "base")
assert.Contains(t, schema.Properties, "sort")
assert.Contains(t, schema.Properties, "direction")
assert.Contains(t, schema.Properties, "perPage")
assert.Contains(t, schema.Properties, "page")
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"})
// Setup mock PRs for success case
mockPRs := []*github.PullRequest{
{
Number: github.Ptr(42),
Title: github.Ptr("First PR"),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
},
{
Number: github.Ptr(43),
Title: github.Ptr("Second PR"),
State: github.Ptr("closed"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/43"),
},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedPRs []*github.PullRequest
expectedErrMsg string
}{
{
name: "successful PRs listing",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepo: expectQueryParams(t, map[string]string{
"state": "all",
"sort": "created",
"direction": "desc",
"per_page": "30",
"page": "1",
}).andThen(
mockResponse(t, http.StatusOK, mockPRs),
),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"state": "all",
"sort": "created",
"direction": "desc",
"perPage": float64(30),
"page": float64(1),
},
expectError: false,
expectedPRs: mockPRs,
},
{
name: "PRs listing fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepo: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"message": "Invalid request"}`))
},
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"state": "invalid",
},
expectError: true,
expectedErrMsg: "failed to list pull requests",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := ListPullRequests(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Unmarshal and verify the result
var returnedPRs []MinimalPullRequest
err = json.Unmarshal([]byte(textContent.Text), &returnedPRs)
require.NoError(t, err)
assert.Len(t, returnedPRs, 2)
assert.Equal(t, *tc.expectedPRs[0].Number, returnedPRs[0].Number)
assert.Equal(t, *tc.expectedPRs[0].Title, returnedPRs[0].Title)
assert.Equal(t, *tc.expectedPRs[0].State, returnedPRs[0].State)
assert.Equal(t, *tc.expectedPRs[1].Number, returnedPRs[1].Number)
assert.Equal(t, *tc.expectedPRs[1].Title, returnedPRs[1].Title)
assert.Equal(t, *tc.expectedPRs[1].State, returnedPRs[1].State)
})
}
}
func Test_MergePullRequest(t *testing.T) {
// Verify tool definition once
serverTool := MergePullRequest(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "merge_pull_request", 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, "pullNumber")
assert.Contains(t, schema.Properties, "commit_title")
assert.Contains(t, schema.Properties, "commit_message")
assert.Contains(t, schema.Properties, "merge_method")
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "pullNumber"})
// Setup mock merge result for success case
mockMergeResult := &github.PullRequestMergeResult{
Merged: github.Ptr(true),
Message: github.Ptr("Pull Request successfully merged"),
SHA: github.Ptr("abcd1234efgh5678"),
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedMergeResult *github.PullRequestMergeResult
expectedErrMsg string
}{
{
name: "successful merge",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PutReposPullsMergeByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{
"commit_title": "Merge PR #42",
"commit_message": "Merging awesome feature",
"merge_method": "squash",
}).andThen(
mockResponse(t, http.StatusOK, mockMergeResult),
),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"commit_title": "Merge PR #42",
"commit_message": "Merging awesome feature",
"merge_method": "squash",
},
expectError: false,
expectedMergeResult: mockMergeResult,
},
{
name: "merge fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PutReposPullsMergeByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusMethodNotAllowed)
_, _ = w.Write([]byte(`{"message": "Pull request cannot be merged"}`))
},
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: true,
expectedErrMsg: "failed to merge pull request",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := MergePullRequest(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Unmarshal and verify the result
var returnedResult github.PullRequestMergeResult
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
require.NoError(t, err)
assert.Equal(t, *tc.expectedMergeResult.Merged, *returnedResult.Merged)
assert.Equal(t, *tc.expectedMergeResult.Message, *returnedResult.Message)
assert.Equal(t, *tc.expectedMergeResult.SHA, *returnedResult.SHA)
})
}
}
func Test_SearchPullRequests(t *testing.T) {
serverTool := SearchPullRequests(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "search_pull_requests", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "query")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "sort")
assert.Contains(t, schema.Properties, "order")
assert.Contains(t, schema.Properties, "perPage")
assert.Contains(t, schema.Properties, "page")
assert.ElementsMatch(t, schema.Required, []string{"query"})
mockSearchResult := &github.IssuesSearchResult{
Total: github.Ptr(2),
IncompleteResults: github.Ptr(false),
Issues: []*github.Issue{
{
Number: github.Ptr(42),
Title: github.Ptr("Test PR 1"),
Body: github.Ptr("Updated tests."),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/1"),
Comments: github.Ptr(5),
User: &github.User{
Login: github.Ptr("user1"),
},
},
{
Number: github.Ptr(43),
Title: github.Ptr("Test PR 2"),
Body: github.Ptr("Updated build scripts."),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/2"),
Comments: github.Ptr(3),
User: &github.User{
Login: github.Ptr("user2"),
},
},
},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedResult *github.IssuesSearchResult
expectedErrMsg string
}{
{
name: "successful pull request search with all parameters",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:pr repo:owner/repo is:open",
"sort": "created",
"order": "desc",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
),
}),
requestArgs: map[string]any{
"query": "repo:owner/repo is:open",
"sort": "created",
"order": "desc",
"page": float64(1),
"perPage": float64(30),
},
expectError: false,
expectedResult: mockSearchResult,
},
{
name: "pull request search with owner and repo parameters",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "repo:test-owner/test-repo is:pr draft:false",
"sort": "updated",
"order": "asc",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
),
}),
requestArgs: map[string]any{
"query": "draft:false",
"owner": "test-owner",
"repo": "test-repo",
"sort": "updated",
"order": "asc",
},
expectError: false,
expectedResult: mockSearchResult,
},
{
name: "pull request search with only owner parameter (should ignore it)",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:pr feature",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
),
}),
requestArgs: map[string]any{
"query": "feature",
"owner": "test-owner",
},
expectError: false,
expectedResult: mockSearchResult,
},
{
name: "pull request search with only repo parameter (should ignore it)",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:pr review-required",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
),
}),
requestArgs: map[string]any{
"query": "review-required",
"repo": "test-repo",
},
expectError: false,
expectedResult: mockSearchResult,
},
{
name: "pull request search with minimal parameters",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: mockResponse(t, http.StatusOK, mockSearchResult),
}),
requestArgs: map[string]any{
"query": "is:pr repo:owner/repo is:open",
},
expectError: false,
expectedResult: mockSearchResult,
},
{
name: "query with existing is:pr filter - no duplication",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:pr repo:github/github-mcp-server is:open draft:false",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
),
}),
requestArgs: map[string]any{
"query": "is:pr repo:github/github-mcp-server is:open draft:false",
},
expectError: false,
expectedResult: mockSearchResult,
},
{
name: "query with existing repo: filter and conflicting owner/repo params - uses query filter",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:pr repo:github/github-mcp-server author:octocat",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
),
}),
requestArgs: map[string]any{
"query": "repo:github/github-mcp-server author:octocat",
"owner": "different-owner",
"repo": "different-repo",
},
expectError: false,
expectedResult: mockSearchResult,
},
{
name: "complex query with existing is:pr filter and OR operators",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: expectQueryParams(
t,
map[string]string{
"q": "is:pr repo:github/github-mcp-server (label:bug OR label:enhancement OR label:feature)",
"page": "1",
"per_page": "30",
},
).andThen(
mockResponse(t, http.StatusOK, mockSearchResult),
),
}),
requestArgs: map[string]any{
"query": "is:pr repo:github/github-mcp-server (label:bug OR label:enhancement OR label:feature)",
},
expectError: false,
expectedResult: mockSearchResult,
},
{
name: "search pull requests fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchIssues: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"message": "Validation Failed"}`))
},
}),
requestArgs: map[string]any{
"query": "invalid:query",
},
expectError: true,
expectedErrMsg: "failed to search pull requests",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := SearchPullRequests(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.NotNil(t, result)
require.True(t, result.IsError)
textContent := getErrorResult(t, result)
assert.Contains(t, textContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Unmarshal and verify the result
var returnedResult github.IssuesSearchResult
err = json.Unmarshal([]byte(textContent.Text), &returnedResult)
require.NoError(t, err)
assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total)
assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults)
assert.Len(t, returnedResult.Issues, len(tc.expectedResult.Issues))
for i, issue := range returnedResult.Issues {
assert.Equal(t, *tc.expectedResult.Issues[i].Number, *issue.Number)
assert.Equal(t, *tc.expectedResult.Issues[i].Title, *issue.Title)
assert.Equal(t, *tc.expectedResult.Issues[i].State, *issue.State)
assert.Equal(t, *tc.expectedResult.Issues[i].HTMLURL, *issue.HTMLURL)
assert.Equal(t, *tc.expectedResult.Issues[i].User.Login, *issue.User.Login)
}
})
}
}
func Test_GetPullRequestFiles(t *testing.T) {
// Verify tool definition once
serverTool := PullRequestRead(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_read", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.Contains(t, schema.Properties, "page")
assert.Contains(t, schema.Properties, "perPage")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
// Setup mock PR files for success case
mockFiles := []*github.CommitFile{
{
Filename: github.Ptr("file1.go"),
Status: github.Ptr("modified"),
Additions: github.Ptr(10),
Deletions: github.Ptr(5),
Changes: github.Ptr(15),
Patch: github.Ptr("@@ -1,5 +1,10 @@"),
},
{
Filename: github.Ptr("file2.go"),
Status: github.Ptr("added"),
Additions: github.Ptr(20),
Deletions: github.Ptr(0),
Changes: github.Ptr(20),
Patch: github.Ptr("@@ -0,0 +1,20 @@"),
},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedFiles []*github.CommitFile
expectedErrMsg string
}{
{
name: "successful files fetch",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsFilesByOwnerByRepoByPullNumber: expectQueryParams(t, map[string]string{
"page": "1",
"per_page": "30",
}).andThen(
mockResponse(t, http.StatusOK, mockFiles),
),
}),
requestArgs: map[string]any{
"method": "get_files",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: false,
expectedFiles: mockFiles,
},
{
name: "successful files fetch with pagination",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsFilesByOwnerByRepoByPullNumber: expectQueryParams(t, map[string]string{
"page": "2",
"per_page": "10",
}).andThen(
mockResponse(t, http.StatusOK, mockFiles),
),
}),
requestArgs: map[string]any{
"method": "get_files",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"page": float64(2),
"perPage": float64(10),
},
expectError: false,
expectedFiles: mockFiles,
},
{
name: "files fetch fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsFilesByOwnerByRepoByPullNumber: expectQueryParams(t, map[string]string{
"page": "1",
"per_page": "30",
}).andThen(
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
}),
),
}),
requestArgs: map[string]any{
"method": "get_files",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(999),
},
expectError: true,
expectedErrMsg: "failed to get pull request files",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := PullRequestRead(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Unmarshal and verify the result
var returnedFiles []MinimalPRFile
err = json.Unmarshal([]byte(textContent.Text), &returnedFiles)
require.NoError(t, err)
assert.Len(t, returnedFiles, len(tc.expectedFiles))
for i, file := range returnedFiles {
assert.Equal(t, tc.expectedFiles[i].GetFilename(), file.Filename)
assert.Equal(t, tc.expectedFiles[i].GetStatus(), file.Status)
assert.Equal(t, tc.expectedFiles[i].GetAdditions(), file.Additions)
assert.Equal(t, tc.expectedFiles[i].GetDeletions(), file.Deletions)
}
})
}
}
func Test_GetPullRequestStatus(t *testing.T) {
// Verify tool definition once
serverTool := PullRequestRead(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_read", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
// Setup mock PR for successful PR fetch
mockPR := &github.PullRequest{
Number: github.Ptr(42),
Title: github.Ptr("Test PR"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
Head: &github.PullRequestBranch{
SHA: github.Ptr("abcd1234"),
Ref: github.Ptr("feature-branch"),
},
}
// Setup mock status for success case
mockStatus := &github.CombinedStatus{
State: github.Ptr("success"),
TotalCount: github.Ptr(3),
Statuses: []*github.RepoStatus{
{
State: github.Ptr("success"),
Context: github.Ptr("continuous-integration/travis-ci"),
Description: github.Ptr("Build succeeded"),
TargetURL: github.Ptr("https://travis-ci.org/owner/repo/builds/123"),
},
{
State: github.Ptr("success"),
Context: github.Ptr("codecov/patch"),
Description: github.Ptr("Coverage increased"),
TargetURL: github.Ptr("https://codecov.io/gh/owner/repo/pull/42"),
},
{
State: github.Ptr("success"),
Context: github.Ptr("lint/golangci-lint"),
Description: github.Ptr("No issues found"),
TargetURL: github.Ptr("https://golangci.com/r/owner/repo/pull/42"),
},
},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedStatus *github.CombinedStatus
expectedErrMsg string
}{
{
name: "successful status fetch",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
GetReposCommitsStatusByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockStatus),
}),
requestArgs: map[string]any{
"method": "get_status",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: false,
expectedStatus: mockStatus,
},
{
name: "PR fetch fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
}),
}),
requestArgs: map[string]any{
"method": "get_status",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(999),
},
expectError: true,
expectedErrMsg: "failed to get pull request",
},
{
name: "status fetch fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
GetReposCommitsStatusesByOwnerByRepoByRef: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
}),
}),
requestArgs: map[string]any{
"method": "get_status",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: true,
expectedErrMsg: "failed to get combined status",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := PullRequestRead(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Unmarshal and verify the result
var returnedStatus github.CombinedStatus
err = json.Unmarshal([]byte(textContent.Text), &returnedStatus)
require.NoError(t, err)
assert.Equal(t, *tc.expectedStatus.State, *returnedStatus.State)
assert.Equal(t, *tc.expectedStatus.TotalCount, *returnedStatus.TotalCount)
assert.Len(t, returnedStatus.Statuses, len(tc.expectedStatus.Statuses))
for i, status := range returnedStatus.Statuses {
assert.Equal(t, *tc.expectedStatus.Statuses[i].State, *status.State)
assert.Equal(t, *tc.expectedStatus.Statuses[i].Context, *status.Context)
assert.Equal(t, *tc.expectedStatus.Statuses[i].Description, *status.Description)
}
})
}
}
func Test_GetPullRequestCheckRuns(t *testing.T) {
// Verify tool definition once
serverTool := PullRequestRead(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_read", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
// Setup mock PR for successful PR fetch
mockPR := &github.PullRequest{
Number: github.Ptr(42),
Title: github.Ptr("Test PR"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
Head: &github.PullRequestBranch{
SHA: github.Ptr("abcd1234"),
Ref: github.Ptr("feature-branch"),
},
}
// Setup mock check runs for success case
mockCheckRuns := &github.ListCheckRunsResults{
Total: github.Ptr(2),
CheckRuns: []*github.CheckRun{
{
ID: github.Ptr(int64(1)),
Name: github.Ptr("build"),
Status: github.Ptr("completed"),
Conclusion: github.Ptr("success"),
HTMLURL: github.Ptr("https://github.com/owner/repo/runs/1"),
},
{
ID: github.Ptr(int64(2)),
Name: github.Ptr("test"),
Status: github.Ptr("completed"),
Conclusion: github.Ptr("success"),
HTMLURL: github.Ptr("https://github.com/owner/repo/runs/2"),
},
},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedCheckRuns *github.ListCheckRunsResults
expectedErrMsg string
}{
{
name: "successful check runs fetch",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
GetReposCommitsCheckRunsByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockCheckRuns),
}),
requestArgs: map[string]any{
"method": "get_check_runs",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: false,
expectedCheckRuns: mockCheckRuns,
},
{
name: "PR fetch fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
}),
}),
requestArgs: map[string]any{
"method": "get_check_runs",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(999),
},
expectError: true,
expectedErrMsg: "failed to get pull request",
},
{
name: "check runs fetch fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
GetReposCommitsCheckRunsByOwnerByRepoByRef: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
}),
}),
requestArgs: map[string]any{
"method": "get_check_runs",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: true,
expectedErrMsg: "failed to get check runs",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := PullRequestRead(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Unmarshal and verify the result (using minimal type)
var returnedCheckRuns MinimalCheckRunsResult
err = json.Unmarshal([]byte(textContent.Text), &returnedCheckRuns)
require.NoError(t, err)
assert.Equal(t, *tc.expectedCheckRuns.Total, returnedCheckRuns.TotalCount)
assert.Len(t, returnedCheckRuns.CheckRuns, len(tc.expectedCheckRuns.CheckRuns))
for i, checkRun := range returnedCheckRuns.CheckRuns {
assert.Equal(t, *tc.expectedCheckRuns.CheckRuns[i].Name, checkRun.Name)
assert.Equal(t, *tc.expectedCheckRuns.CheckRuns[i].Status, checkRun.Status)
assert.Equal(t, *tc.expectedCheckRuns.CheckRuns[i].Conclusion, checkRun.Conclusion)
}
})
}
}
func Test_UpdatePullRequestBranch(t *testing.T) {
// Verify tool definition once
serverTool := UpdatePullRequestBranch(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "update_pull_request_branch", 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, "pullNumber")
assert.Contains(t, schema.Properties, "expectedHeadSha")
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "pullNumber"})
// Setup mock update result for success case
mockUpdateResult := &github.PullRequestBranchUpdateResponse{
Message: github.Ptr("Branch was updated successfully"),
URL: github.Ptr("https://api.github.com/repos/owner/repo/pulls/42"),
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedUpdateResult *github.PullRequestBranchUpdateResponse
expectedErrMsg string
}{
{
name: "successful branch update",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PutReposPullsUpdateBranchByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{
"expected_head_sha": "abcd1234",
}).andThen(
mockResponse(t, http.StatusAccepted, mockUpdateResult),
),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"expectedHeadSha": "abcd1234",
},
expectError: false,
expectedUpdateResult: mockUpdateResult,
},
{
name: "branch update without expected SHA",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PutReposPullsUpdateBranchByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{}).andThen(
mockResponse(t, http.StatusAccepted, mockUpdateResult),
),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: false,
expectedUpdateResult: mockUpdateResult,
},
{
name: "branch update fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PutReposPullsUpdateBranchByOwnerByRepoByPullNumber: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusConflict)
_, _ = w.Write([]byte(`{"message": "Merge conflict"}`))
}),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: true,
expectedErrMsg: "failed to update pull request branch",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := UpdatePullRequestBranch(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, "is in progress")
})
}
}
func Test_GetPullRequestComments(t *testing.T) {
// Verify tool definition once
serverTool := PullRequestRead(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_read", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
// `after` is required for cursor-based pagination on get_review_comments
// to be reachable from MCP clients; without it in the schema, callers
// cannot advance past the first page (issue #2122).
assert.Contains(t, schema.Properties, "after")
assert.Equal(t, "string", schema.Properties["after"].Type)
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
tests := []struct {
name string
gqlHTTPClient *http.Client
requestArgs map[string]any
expectError bool
expectedErrMsg string
lockdownEnabled bool
validateResult func(t *testing.T, textContent string)
}{
{
name: "successful review threads fetch",
gqlHTTPClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
reviewThreadsQuery{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
"first": githubv4.Int(30),
"commentsPerThread": githubv4.Int(100),
"after": (*githubv4.String)(nil),
},
githubv4mock.DataResponse(map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"reviewThreads": map[string]any{
"nodes": []map[string]any{
{
"id": "RT_kwDOA0xdyM4AX1Yz",
"isResolved": false,
"isOutdated": false,
"isCollapsed": false,
"comments": map[string]any{
"totalCount": 2,
"nodes": []map[string]any{
{
"id": "PRRC_kwDOA0xdyM4AX1Y0",
"body": "This looks good",
"path": "file1.go",
"line": 5,
"author": map[string]any{
"login": "reviewer1",
},
"createdAt": "2024-01-01T12:00:00Z",
"updatedAt": "2024-01-01T12:00:00Z",
"url": "https://github.com/owner/repo/pull/42#discussion_r101",
},
{
"id": "PRRC_kwDOA0xdyM4AX1Y1",
"body": "Please fix this",
"path": "file1.go",
"line": 10,
"author": map[string]any{
"login": "reviewer2",
},
"createdAt": "2024-01-01T13:00:00Z",
"updatedAt": "2024-01-01T13:00:00Z",
"url": "https://github.com/owner/repo/pull/42#discussion_r102",
},
},
},
},
},
"pageInfo": map[string]any{
"hasNextPage": false,
"hasPreviousPage": false,
"startCursor": "cursor1",
"endCursor": "cursor2",
},
"totalCount": 1,
},
},
},
}),
),
),
requestArgs: map[string]any{
"method": "get_review_comments",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: false,
validateResult: func(t *testing.T, textContent string) {
var result MinimalReviewThreadsResponse
err := json.Unmarshal([]byte(textContent), &result)
require.NoError(t, err)
// Validate review threads
assert.Len(t, result.ReviewThreads, 1)
thread := result.ReviewThreads[0]
assert.Equal(t, false, thread.IsResolved)
assert.Equal(t, false, thread.IsOutdated)
assert.Equal(t, false, thread.IsCollapsed)
// Validate comments within thread
assert.Len(t, thread.Comments, 2)
// Validate first comment
comment1 := thread.Comments[0]
assert.Equal(t, "This looks good", comment1.Body)
assert.Equal(t, "file1.go", comment1.Path)
assert.Equal(t, "reviewer1", comment1.Author)
// Validate pagination info
assert.Equal(t, false, result.PageInfo.HasNextPage)
assert.Equal(t, false, result.PageInfo.HasPreviousPage)
assert.Equal(t, "cursor1", result.PageInfo.StartCursor)
assert.Equal(t, "cursor2", result.PageInfo.EndCursor)
// Validate total count
assert.Equal(t, 1, result.TotalCount)
},
},
{
name: "after cursor is forwarded to GraphQL query",
gqlHTTPClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
reviewThreadsQuery{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
"first": githubv4.Int(30),
"commentsPerThread": githubv4.Int(100),
"after": githubv4.String("cursor-page-2"),
},
githubv4mock.DataResponse(map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"reviewThreads": map[string]any{
"nodes": []map[string]any{},
"pageInfo": map[string]any{
"hasNextPage": false,
"hasPreviousPage": true,
"startCursor": "cursor3",
"endCursor": "cursor4",
},
"totalCount": 5,
},
},
},
}),
),
),
requestArgs: map[string]any{
"method": "get_review_comments",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"after": "cursor-page-2",
},
expectError: false,
validateResult: func(t *testing.T, textContent string) {
var result MinimalReviewThreadsResponse
err := json.Unmarshal([]byte(textContent), &result)
require.NoError(t, err)
assert.Len(t, result.ReviewThreads, 0)
assert.Equal(t, true, result.PageInfo.HasPreviousPage)
assert.Equal(t, "cursor4", result.PageInfo.EndCursor)
},
},
{
name: "review threads fetch fails",
gqlHTTPClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
reviewThreadsQuery{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(999),
"first": githubv4.Int(30),
"commentsPerThread": githubv4.Int(100),
"after": (*githubv4.String)(nil),
},
githubv4mock.ErrorResponse("Could not resolve to a PullRequest with the number of 999."),
),
),
requestArgs: map[string]any{
"method": "get_review_comments",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(999),
},
expectError: true,
expectedErrMsg: "failed to get pull request review threads",
},
{
name: "lockdown enabled filters review comments without push access",
gqlHTTPClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
reviewThreadsQuery{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
"first": githubv4.Int(30),
"commentsPerThread": githubv4.Int(100),
"after": (*githubv4.String)(nil),
},
githubv4mock.DataResponse(map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"reviewThreads": map[string]any{
"nodes": []map[string]any{
{
"id": "RT_kwDOA0xdyM4AX1Yz",
"isResolved": false,
"isOutdated": false,
"isCollapsed": false,
"comments": map[string]any{
"totalCount": 2,
"nodes": []map[string]any{
{
"id": "PRRC_kwDOA0xdyM4AX1Y0",
"body": "Maintainer review comment",
"path": "file1.go",
"line": 5,
"author": map[string]any{
"login": "maintainer",
},
"createdAt": "2024-01-01T12:00:00Z",
"updatedAt": "2024-01-01T12:00:00Z",
"url": "https://github.com/owner/repo/pull/42#discussion_r2010",
},
{
"id": "PRRC_kwDOA0xdyM4AX1Y1",
"body": "External review comment",
"path": "file1.go",
"line": 10,
"author": map[string]any{
"login": "testuser",
},
"createdAt": "2024-01-01T13:00:00Z",
"updatedAt": "2024-01-01T13:00:00Z",
"url": "https://github.com/owner/repo/pull/42#discussion_r2011",
},
},
},
},
},
"pageInfo": map[string]any{
"hasNextPage": false,
"hasPreviousPage": false,
"startCursor": "cursor1",
"endCursor": "cursor2",
},
"totalCount": 1,
},
},
},
}),
),
),
requestArgs: map[string]any{
"method": "get_review_comments",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: false,
lockdownEnabled: true,
validateResult: func(t *testing.T, textContent string) {
var result MinimalReviewThreadsResponse
err := json.Unmarshal([]byte(textContent), &result)
require.NoError(t, err)
// Validate that only maintainer comment is returned
assert.Len(t, result.ReviewThreads, 1)
thread := result.ReviewThreads[0]
// Should only have 1 comment (maintainer) after filtering
assert.Equal(t, 1, thread.TotalCount)
assert.Len(t, thread.Comments, 1)
comment := thread.Comments[0]
assert.Equal(t, "maintainer", comment.Author)
assert.Equal(t, "Maintainer review comment", comment.Body)
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup GraphQL client with mock
var gqlClient *githubv4.Client
if tc.gqlHTTPClient != nil {
gqlClient = githubv4.NewClient(tc.gqlHTTPClient)
} else {
gqlClient = githubv4.NewClient(nil)
}
// Setup cache for lockdown mode
var restClient *github.Client
if tc.lockdownEnabled {
restClient = mockRESTPermissionServer(t, "read", map[string]string{
"maintainer": "write",
"external-user": "read",
"testuser": "read",
})
}
cache := stubRepoAccessCache(restClient, 5*time.Minute)
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdownEnabled})
serverTool := PullRequestRead(translations.NullTranslationHelper)
deps := BaseDeps{
Client: mustNewGHClient(t, nil),
GQLClient: gqlClient,
RepoAccessCache: cache,
Flags: flags,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Use custom validation if provided
if tc.validateResult != nil {
tc.validateResult(t, textContent.Text)
}
})
}
}
func Test_GetPullRequestReviews(t *testing.T) {
// Verify tool definition once
serverTool := PullRequestRead(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_read", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
// Setup mock PR reviews for success case
mockReviews := []*github.PullRequestReview{
{
ID: github.Ptr(int64(201)),
State: github.Ptr("APPROVED"),
Body: github.Ptr("LGTM"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42#pullrequestreview-201"),
User: &github.User{
Login: github.Ptr("approver"),
},
CommitID: github.Ptr("abcdef123456"),
SubmittedAt: &github.Timestamp{Time: time.Now().Add(-24 * time.Hour)},
},
{
ID: github.Ptr(int64(202)),
State: github.Ptr("CHANGES_REQUESTED"),
Body: github.Ptr("Please address the following issues"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42#pullrequestreview-202"),
User: &github.User{
Login: github.Ptr("reviewer"),
},
CommitID: github.Ptr("abcdef123456"),
SubmittedAt: &github.Timestamp{Time: time.Now().Add(-12 * time.Hour)},
},
}
tests := []struct {
name string
mockedClient *http.Client
gqlHTTPClient *http.Client
requestArgs map[string]any
expectError bool
expectedReviews []*github.PullRequestReview
expectedErrMsg string
lockdownEnabled bool
}{
{
name: "successful reviews fetch",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsReviewsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockReviews),
}),
requestArgs: map[string]any{
"method": "get_reviews",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: false,
expectedReviews: mockReviews,
},
{
name: "successful reviews fetch with pagination",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsReviewsByOwnerByRepoByPullNumber: expectQueryParams(t, map[string]string{
"page": "2",
"per_page": "10",
}).andThen(
mockResponse(t, http.StatusOK, mockReviews),
),
}),
requestArgs: map[string]any{
"method": "get_reviews",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"page": float64(2),
"perPage": float64(10),
},
expectError: false,
expectedReviews: mockReviews,
},
{
name: "reviews fetch fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsReviewsByOwnerByRepoByPullNumber: expectQueryParams(t, map[string]string{
"page": "1",
"per_page": "30",
}).andThen(
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
}),
),
}),
requestArgs: map[string]any{
"method": "get_reviews",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(999),
},
expectError: true,
expectedErrMsg: "failed to get pull request reviews",
},
{
name: "lockdown enabled filters reviews without push access",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsReviewsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, []*github.PullRequestReview{
{
ID: github.Ptr(int64(2030)),
State: github.Ptr("APPROVED"),
Body: github.Ptr("Maintainer review"),
User: &github.User{Login: github.Ptr("maintainer")},
},
{
ID: github.Ptr(int64(2031)),
State: github.Ptr("COMMENTED"),
Body: github.Ptr("External reviewer"),
User: &github.User{Login: github.Ptr("testuser")},
},
}),
}),
requestArgs: map[string]any{
"method": "get_reviews",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
expectError: false,
expectedReviews: []*github.PullRequestReview{
{
ID: github.Ptr(int64(2030)),
State: github.Ptr("APPROVED"),
Body: github.Ptr("Maintainer review"),
User: &github.User{Login: github.Ptr("maintainer")},
},
},
lockdownEnabled: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
var restClient *github.Client
if tc.lockdownEnabled {
restClient = mockRESTPermissionServer(t, "read", map[string]string{
"maintainer": "write",
"testuser": "read",
})
}
cache := stubRepoAccessCache(restClient, 5*time.Minute)
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdownEnabled})
serverTool := PullRequestRead(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
RepoAccessCache: cache,
Flags: flags,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
require.NoError(t, err)
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
require.False(t, result.IsError)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Unmarshal and verify the result
var returnedReviews []MinimalPullRequestReview
err = json.Unmarshal([]byte(textContent.Text), &returnedReviews)
require.NoError(t, err)
assert.Len(t, returnedReviews, len(tc.expectedReviews))
for i, review := range returnedReviews {
assert.Equal(t, tc.expectedReviews[i].GetID(), review.ID)
assert.Equal(t, tc.expectedReviews[i].GetState(), review.State)
assert.Equal(t, tc.expectedReviews[i].GetBody(), review.Body)
require.NotNil(t, tc.expectedReviews[i].User)
require.NotNil(t, review.User)
assert.Equal(t, tc.expectedReviews[i].GetUser().GetLogin(), review.User.Login)
assert.Equal(t, tc.expectedReviews[i].GetHTMLURL(), review.HTMLURL)
}
})
}
}
func Test_CreatePullRequest(t *testing.T) {
// Verify tool definition once
serverTool := CreatePullRequest(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "create_pull_request", 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, "title")
assert.Contains(t, schema.Properties, "body")
assert.Contains(t, schema.Properties, "head")
assert.Contains(t, schema.Properties, "base")
assert.Contains(t, schema.Properties, "draft")
assert.Contains(t, schema.Properties, "maintainer_can_modify")
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "title", "head", "base"})
// Setup mock PR for success case
mockPR := &github.PullRequest{
Number: github.Ptr(42),
Title: github.Ptr("Test PR"),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
Head: &github.PullRequestBranch{
SHA: github.Ptr("abcd1234"),
Ref: github.Ptr("feature-branch"),
},
Base: &github.PullRequestBranch{
SHA: github.Ptr("efgh5678"),
Ref: github.Ptr("main"),
},
Body: github.Ptr("This is a test PR"),
Draft: github.Ptr(false),
MaintainerCanModify: github.Ptr(true),
User: &github.User{
Login: github.Ptr("testuser"),
},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectError bool
expectedPR *github.PullRequest
expectedErrMsg string
}{
{
name: "successful PR creation",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposPullsByOwnerByRepo: expectRequestBody(t, map[string]any{
"title": "Test PR",
"body": "This is a test PR",
"head": "feature-branch",
"base": "main",
"draft": false,
"maintainer_can_modify": true,
}).andThen(
mockResponse(t, http.StatusCreated, mockPR),
),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"title": "Test PR",
"body": "This is a test PR",
"head": "feature-branch",
"base": "main",
"draft": false,
"maintainer_can_modify": true,
},
expectError: false,
expectedPR: mockPR,
},
{
name: "missing required parameter",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
// missing title, head, base
},
expectError: true,
expectedErrMsg: "missing required parameter: title",
},
{
name: "PR creation fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposPullsByOwnerByRepo: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusUnprocessableEntity)
_, _ = w.Write([]byte(`{"message":"Validation failed","errors":[{"resource":"PullRequest","code":"invalid"}]}`))
}),
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"title": "Test PR",
"head": "feature-branch",
"base": "main",
},
expectError: true,
expectedErrMsg: "failed to create pull request",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := CreatePullRequest(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
// Verify results
if tc.expectError {
if err != nil {
assert.Contains(t, err.Error(), tc.expectedErrMsg)
return
}
// If no error returned but in the result
textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, tc.expectedErrMsg)
return
}
require.NoError(t, err)
// Parse the result and get the text content if no error
textContent := getTextResult(t, result)
// Unmarshal and verify the minimal result
var returnedPR MinimalResponse
err = json.Unmarshal([]byte(textContent.Text), &returnedPR)
require.NoError(t, err)
assert.Equal(t, tc.expectedPR.GetHTMLURL(), returnedPR.URL)
})
}
}
// Test_CreatePullRequest_MCPAppsFeature_UIGate verifies the MCP Apps feature UI gate
// behavior: UI clients get a form message, non-UI clients execute directly.
func Test_CreatePullRequest_MCPAppsFeature_UIGate(t *testing.T) {
t.Parallel()
mockPR := &github.PullRequest{
Number: github.Ptr(42),
Title: github.Ptr("Test PR"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"),
Head: &github.PullRequestBranch{SHA: github.Ptr("abc"), Ref: github.Ptr("feature")},
Base: &github.PullRequestBranch{SHA: github.Ptr("def"), Ref: github.Ptr("main")},
User: &github.User{Login: github.Ptr("testuser")},
}
serverTool := CreatePullRequest(translations.NullTranslationHelper)
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposPullsByOwnerByRepo: mockResponse(t, http.StatusCreated, mockPR),
}))
deps := BaseDeps{
Client: client,
GQLClient: githubv4.NewClient(nil),
featureChecker: featureCheckerFor(MCPAppsFeatureFlag),
}
handler := serverTool.Handler(deps)
t.Run("UI client without _ui_submitted returns form message", func(t *testing.T) {
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
"owner": "owner",
"repo": "repo",
"title": "Test PR",
"head": "feature",
"base": "main",
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, "Ready to create a pull request")
})
t.Run("UI client with _ui_submitted executes directly", func(t *testing.T) {
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
"owner": "owner",
"repo": "repo",
"title": "Test PR",
"head": "feature",
"base": "main",
"_ui_submitted": true,
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
"tool should return the created PR URL")
})
t.Run("non-UI client executes directly without _ui_submitted", func(t *testing.T) {
request := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"title": "Test PR",
"head": "feature",
"base": "main",
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
"non-UI client should execute directly")
})
t.Run("UI client with non-form param skips form and executes directly", func(t *testing.T) {
// A parameter the form does not collect must bypass the form rather than
// be silently dropped.
request := createMCPRequestWithSession(t, ClientNameVSCodeInsiders, true, map[string]any{
"owner": "owner",
"repo": "repo",
"title": "Test PR",
"head": "feature",
"base": "main",
"reviewers": []any{"octocat"},
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
assert.NotContains(t, textContent.Text, "Ready to create a pull request",
"non-form param should skip UI form")
assert.Contains(t, textContent.Text, "https://github.com/owner/repo/pull/42",
"non-form param call should execute directly and return PR URL")
})
}
func Test_pullRequestWriteHasNonFormParams(t *testing.T) {
t.Parallel()
tests := []struct {
name string
args map[string]any
want bool
}{
{name: "no params", args: map[string]any{}, want: false},
{name: "only form params", args: map[string]any{"owner": "o", "repo": "r", "title": "t", "body": "b", "head": "h", "base": "b", "draft": true, "maintainer_can_modify": false, "_ui_submitted": true}, want: false},
{name: "unknown param present", args: map[string]any{"title": "t", "reviewers": []any{"octocat"}}, want: true},
{name: "nil value is ignored", args: map[string]any{"reviewers": nil}, want: false},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tc.want, pullRequestWriteHasNonFormParams(tc.args))
})
}
}
func TestCreateAndSubmitPullRequestReview(t *testing.T) {
t.Parallel()
// Verify tool definition once
serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_review_write", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.Contains(t, schema.Properties, "body")
assert.Contains(t, schema.Properties, "event")
assert.Contains(t, schema.Properties, "commitID")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectToolError bool
expectedToolErrMsg string
}{
{
name: "successful review creation",
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
ID githubv4.ID
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
},
githubv4mock.DataResponse(
map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"id": "PR_kwDODKw3uc6WYN1T",
},
},
},
),
),
githubv4mock.NewMutationMatcher(
struct {
AddPullRequestReview struct {
PullRequestReview struct {
ID githubv4.ID
}
} `graphql:"addPullRequestReview(input: $input)"`
}{},
githubv4.AddPullRequestReviewInput{
PullRequestID: githubv4.ID("PR_kwDODKw3uc6WYN1T"),
Body: githubv4.NewString("This is a test review"),
Event: githubv4mock.Ptr(githubv4.PullRequestReviewEventComment),
CommitOID: githubv4.NewGitObjectID("abcd1234"),
},
nil,
githubv4mock.DataResponse(map[string]any{}),
),
),
requestArgs: map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"body": "This is a test review",
"event": "COMMENT",
"commitID": "abcd1234",
},
expectToolError: false,
},
{
name: "successful review creation with string pullNumber",
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
ID githubv4.ID
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
},
githubv4mock.DataResponse(
map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"id": "PR_kwDODKw3uc6WYN1T",
},
},
},
),
),
githubv4mock.NewMutationMatcher(
struct {
AddPullRequestReview struct {
PullRequestReview struct {
ID githubv4.ID
}
} `graphql:"addPullRequestReview(input: $input)"`
}{},
githubv4.AddPullRequestReviewInput{
PullRequestID: githubv4.ID("PR_kwDODKw3uc6WYN1T"),
Body: githubv4.NewString("This is a test review"),
Event: githubv4mock.Ptr(githubv4.PullRequestReviewEventComment),
CommitOID: githubv4.NewGitObjectID("abcd1234"),
},
nil,
githubv4mock.DataResponse(map[string]any{}),
),
),
requestArgs: map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"pullNumber": "42", // Some MCP clients send numeric values as strings
"body": "This is a test review",
"event": "COMMENT",
"commitID": "abcd1234",
},
expectToolError: false,
},
{
name: "failure to get pull request",
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
ID githubv4.ID
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
},
githubv4mock.ErrorResponse("expected test failure"),
),
),
requestArgs: map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"body": "This is a test review",
"event": "COMMENT",
"commitID": "abcd1234",
},
expectToolError: true,
expectedToolErrMsg: "expected test failure",
},
{
name: "failure to submit review",
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
ID githubv4.ID
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
},
githubv4mock.DataResponse(
map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"id": "PR_kwDODKw3uc6WYN1T",
},
},
},
),
),
githubv4mock.NewMutationMatcher(
struct {
AddPullRequestReview struct {
PullRequestReview struct {
ID githubv4.ID
}
} `graphql:"addPullRequestReview(input: $input)"`
}{},
githubv4.AddPullRequestReviewInput{
PullRequestID: githubv4.ID("PR_kwDODKw3uc6WYN1T"),
Body: githubv4.NewString("This is a test review"),
Event: githubv4mock.Ptr(githubv4.PullRequestReviewEventComment),
CommitOID: githubv4.NewGitObjectID("abcd1234"),
},
nil,
githubv4mock.ErrorResponse("expected test failure"),
),
),
requestArgs: map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"body": "This is a test review",
"event": "COMMENT",
"commitID": "abcd1234",
},
expectToolError: true,
expectedToolErrMsg: "expected test failure",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
// Setup client with mock
client := githubv4.NewClient(tc.mockedClient)
serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
deps := BaseDeps{
GQLClient: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
if tc.expectToolError {
require.True(t, result.IsError)
assert.Contains(t, textContent.Text, tc.expectedToolErrMsg)
return
}
// Parse the result and get the text content if no error
require.Equal(t, textContent.Text, "pull request review submitted successfully")
})
}
}
func TestCreatePendingPullRequestReview(t *testing.T) {
t.Parallel()
// Verify tool definition once
serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_review_write", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.Contains(t, schema.Properties, "commitID")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectToolError bool
expectedToolErrMsg string
}{
{
name: "successful review creation",
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
ID githubv4.ID
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
},
githubv4mock.DataResponse(
map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"id": "PR_kwDODKw3uc6WYN1T",
},
},
},
),
),
githubv4mock.NewMutationMatcher(
struct {
AddPullRequestReview struct {
PullRequestReview struct {
ID githubv4.ID
}
} `graphql:"addPullRequestReview(input: $input)"`
}{},
githubv4.AddPullRequestReviewInput{
PullRequestID: githubv4.ID("PR_kwDODKw3uc6WYN1T"),
CommitOID: githubv4.NewGitObjectID("abcd1234"),
},
nil,
githubv4mock.DataResponse(map[string]any{}),
),
),
requestArgs: map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"commitID": "abcd1234",
},
expectToolError: false,
},
{
name: "failure to get pull request",
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
ID githubv4.ID
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
},
githubv4mock.ErrorResponse("expected test failure"),
),
),
requestArgs: map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"commitID": "abcd1234",
},
expectToolError: true,
expectedToolErrMsg: "expected test failure",
},
{
name: "failure to create pending review",
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
ID githubv4.ID
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"prNum": githubv4.Int(42),
},
githubv4mock.DataResponse(
map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"id": "PR_kwDODKw3uc6WYN1T",
},
},
},
),
),
githubv4mock.NewMutationMatcher(
struct {
AddPullRequestReview struct {
PullRequestReview struct {
ID githubv4.ID
}
} `graphql:"addPullRequestReview(input: $input)"`
}{},
githubv4.AddPullRequestReviewInput{
PullRequestID: githubv4.ID("PR_kwDODKw3uc6WYN1T"),
CommitOID: githubv4.NewGitObjectID("abcd1234"),
},
nil,
githubv4mock.ErrorResponse("expected test failure"),
),
),
requestArgs: map[string]any{
"method": "create",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"commitID": "abcd1234",
},
expectToolError: true,
expectedToolErrMsg: "expected test failure",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
// Setup client with mock
client := githubv4.NewClient(tc.mockedClient)
serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
deps := BaseDeps{
GQLClient: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
if tc.expectToolError {
require.True(t, result.IsError)
assert.Contains(t, textContent.Text, tc.expectedToolErrMsg)
return
}
// Parse the result and get the text content if no error
require.Equal(t, "pending pull request created", textContent.Text)
})
}
}
func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) {
t.Parallel()
// Verify tool definition once
serverTool := AddCommentToPendingReview(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "add_comment_to_pending_review", 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, "pullNumber")
assert.Contains(t, schema.Properties, "path")
assert.Contains(t, schema.Properties, "body")
assert.Contains(t, schema.Properties, "subjectType")
assert.Contains(t, schema.Properties, "line")
assert.Contains(t, schema.Properties, "side")
assert.Contains(t, schema.Properties, "startLine")
assert.Contains(t, schema.Properties, "startSide")
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "pullNumber", "path", "body", "subjectType"})
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectToolError bool
expectedToolErrMsg string
}{
{
name: "successful line comment addition",
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"path": "file.go",
"body": "This is a test comment",
"subjectType": "LINE",
"line": float64(10),
"side": "RIGHT",
"startLine": float64(5),
"startSide": "RIGHT",
},
mockedClient: githubv4mock.NewMockedHTTPClient(
viewerQuery("williammartin"),
getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{
author: "williammartin",
owner: "owner",
repo: "repo",
prNum: 42,
reviews: []getLatestPendingReviewQueryReview{
{
id: "PR_kwDODKw3uc6WYN1T",
state: "PENDING",
url: "https://github.com/owner/repo/pull/42",
},
},
}),
githubv4mock.NewMutationMatcher(
struct {
AddPullRequestReviewThread struct {
Thread struct {
ID githubv4.String // We don't need this, but a selector is required or GQL complains.
}
} `graphql:"addPullRequestReviewThread(input: $input)"`
}{},
githubv4.AddPullRequestReviewThreadInput{
Path: githubv4.String("file.go"),
Body: githubv4.String("This is a test comment"),
SubjectType: githubv4mock.Ptr(githubv4.PullRequestReviewThreadSubjectTypeLine),
Line: githubv4.NewInt(10),
Side: githubv4mock.Ptr(githubv4.DiffSideRight),
StartLine: githubv4.NewInt(5),
StartSide: githubv4mock.Ptr(githubv4.DiffSideRight),
PullRequestReviewID: githubv4.NewID("PR_kwDODKw3uc6WYN1T"),
},
nil,
githubv4mock.DataResponse(map[string]any{
"addPullRequestReviewThread": map[string]any{
"thread": map[string]any{
"id": "MDEyOlB1bGxSZXF1ZXN0UmV2aWV3VGhyZWFkMTIzNDU2",
},
},
}),
),
),
},
{
name: "successful line comment with string pullNumber and line",
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": "42", // Some MCP clients send numeric values as strings
"path": "file.go",
"body": "This is a test comment",
"subjectType": "LINE",
"line": "10", // string line number
"side": "RIGHT",
"startLine": "5", // string startLine
"startSide": "RIGHT",
},
mockedClient: githubv4mock.NewMockedHTTPClient(
viewerQuery("williammartin"),
getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{
author: "williammartin",
owner: "owner",
repo: "repo",
prNum: 42,
reviews: []getLatestPendingReviewQueryReview{
{
id: "PR_kwDODKw3uc6WYN1T",
state: "PENDING",
url: "https://github.com/owner/repo/pull/42",
},
},
}),
githubv4mock.NewMutationMatcher(
struct {
AddPullRequestReviewThread struct {
Thread struct {
ID githubv4.String
}
} `graphql:"addPullRequestReviewThread(input: $input)"`
}{},
githubv4.AddPullRequestReviewThreadInput{
Path: githubv4.String("file.go"),
Body: githubv4.String("This is a test comment"),
SubjectType: githubv4mock.Ptr(githubv4.PullRequestReviewThreadSubjectTypeLine),
Line: githubv4.NewInt(10),
Side: githubv4mock.Ptr(githubv4.DiffSideRight),
StartLine: githubv4.NewInt(5),
StartSide: githubv4mock.Ptr(githubv4.DiffSideRight),
PullRequestReviewID: githubv4.NewID("PR_kwDODKw3uc6WYN1T"),
},
nil,
githubv4mock.DataResponse(map[string]any{
"addPullRequestReviewThread": map[string]any{
"thread": map[string]any{
"id": "MDEyOlB1bGxSZXF1ZXN0UmV2aWV3VGhyZWFkMTIzNDU2",
},
},
}),
),
),
},
{
name: "thread ID is nil - invalid line number",
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"path": "file.go",
"body": "Comment on non-existent line",
"subjectType": "LINE",
"line": float64(999),
"side": "RIGHT",
},
mockedClient: githubv4mock.NewMockedHTTPClient(
viewerQuery("williammartin"),
getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{
author: "williammartin",
owner: "owner",
repo: "repo",
prNum: 42,
reviews: []getLatestPendingReviewQueryReview{
{
id: "PR_kwDODKw3uc6WYN1T",
state: "PENDING",
url: "https://github.com/owner/repo/pull/42",
},
},
}),
githubv4mock.NewMutationMatcher(
struct {
AddPullRequestReviewThread struct {
Thread struct {
ID githubv4.ID
}
} `graphql:"addPullRequestReviewThread(input: $input)"`
}{},
githubv4.AddPullRequestReviewThreadInput{
Path: githubv4.String("file.go"),
Body: githubv4.String("Comment on non-existent line"),
SubjectType: githubv4mock.Ptr(githubv4.PullRequestReviewThreadSubjectTypeLine),
Line: githubv4.NewInt(999),
Side: githubv4mock.Ptr(githubv4.DiffSideRight),
StartLine: nil,
StartSide: nil,
PullRequestReviewID: githubv4.NewID("PR_kwDODKw3uc6WYN1T"),
},
nil,
githubv4mock.DataResponse(map[string]any{
"addPullRequestReviewThread": map[string]any{
"thread": map[string]any{
"id": nil,
},
},
}),
),
),
expectToolError: true,
expectedToolErrMsg: "Failed to add comment to pending review",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
// Setup client with mock
client := githubv4.NewClient(tc.mockedClient)
serverTool := AddCommentToPendingReview(translations.NullTranslationHelper)
deps := BaseDeps{
GQLClient: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
if tc.expectToolError {
require.True(t, result.IsError)
assert.Contains(t, textContent.Text, tc.expectedToolErrMsg)
return
}
// Parse the result and get the text content if no error
require.Equal(t, textContent.Text, "pull request review comment successfully added to pending review")
})
}
}
func TestSubmitPendingPullRequestReview(t *testing.T) {
t.Parallel()
// Verify tool definition once
serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_review_write", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.Contains(t, schema.Properties, "event")
assert.Contains(t, schema.Properties, "body")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectToolError bool
expectedToolErrMsg string
}{
{
name: "successful review submission",
requestArgs: map[string]any{
"method": "submit_pending",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"event": "COMMENT",
"body": "This is a test review",
},
mockedClient: githubv4mock.NewMockedHTTPClient(
viewerQuery("williammartin"),
getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{
author: "williammartin",
owner: "owner",
repo: "repo",
prNum: 42,
reviews: []getLatestPendingReviewQueryReview{
{
id: "PR_kwDODKw3uc6WYN1T",
state: "PENDING",
url: "https://github.com/owner/repo/pull/42",
},
},
}),
githubv4mock.NewMutationMatcher(
struct {
SubmitPullRequestReview struct {
PullRequestReview struct {
ID githubv4.ID
}
} `graphql:"submitPullRequestReview(input: $input)"`
}{},
githubv4.SubmitPullRequestReviewInput{
PullRequestReviewID: githubv4.NewID("PR_kwDODKw3uc6WYN1T"),
Event: githubv4.PullRequestReviewEventComment,
Body: githubv4.NewString("This is a test review"),
},
nil,
githubv4mock.DataResponse(map[string]any{}),
),
),
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
// Setup client with mock
client := githubv4.NewClient(tc.mockedClient)
serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
deps := BaseDeps{
GQLClient: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
if tc.expectToolError {
require.True(t, result.IsError)
assert.Contains(t, textContent.Text, tc.expectedToolErrMsg)
return
}
// Parse the result and get the text content if no error
require.Equal(t, "pending pull request review successfully submitted", textContent.Text)
})
}
}
func TestDeletePendingPullRequestReview(t *testing.T) {
t.Parallel()
// Verify tool definition once
serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_review_write", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
tests := []struct {
name string
requestArgs map[string]any
mockedClient *http.Client
expectToolError bool
expectedToolErrMsg string
}{
{
name: "successful review deletion",
requestArgs: map[string]any{
"method": "delete_pending",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
mockedClient: githubv4mock.NewMockedHTTPClient(
viewerQuery("williammartin"),
getLatestPendingReviewQuery(getLatestPendingReviewQueryParams{
author: "williammartin",
owner: "owner",
repo: "repo",
prNum: 42,
reviews: []getLatestPendingReviewQueryReview{
{
id: "PR_kwDODKw3uc6WYN1T",
state: "PENDING",
url: "https://github.com/owner/repo/pull/42",
},
},
}),
githubv4mock.NewMutationMatcher(
struct {
DeletePullRequestReview struct {
PullRequestReview struct {
ID githubv4.ID
}
} `graphql:"deletePullRequestReview(input: $input)"`
}{},
githubv4.DeletePullRequestReviewInput{
PullRequestReviewID: githubv4.NewID("PR_kwDODKw3uc6WYN1T"),
},
nil,
githubv4mock.DataResponse(map[string]any{}),
),
),
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
// Setup client with mock
client := githubv4.NewClient(tc.mockedClient)
serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
deps := BaseDeps{
GQLClient: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
if tc.expectToolError {
require.True(t, result.IsError)
assert.Contains(t, textContent.Text, tc.expectedToolErrMsg)
return
}
// Parse the result and get the text content if no error
require.Equal(t, "pending pull request review successfully deleted", textContent.Text)
})
}
}
func TestGetPullRequestDiff(t *testing.T) {
t.Parallel()
// Verify tool definition once
serverTool := PullRequestRead(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "pull_request_read", tool.Name)
assert.NotEmpty(t, tool.Description)
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "method")
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "pullNumber")
assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
stubbedDiff := `diff --git a/README.md b/README.md
index 5d6e7b2..8a4f5c3 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,6 @@
# Hello-World
Hello World project for GitHub
+## New Section
+
+This is a new section added in the pull request.`
tests := []struct {
name string
requestArgs map[string]any
mockedClient *http.Client
expectToolError bool
expectedToolErrMsg string
}{
{
name: "successful diff retrieval",
requestArgs: map[string]any{
"method": "get_diff",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: expectPath(t, "/repos/owner/repo/pulls/42").andThen(
mockResponse(t, http.StatusOK, stubbedDiff),
),
}),
expectToolError: false,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := PullRequestRead(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
if tc.expectToolError {
require.True(t, result.IsError)
assert.Contains(t, textContent.Text, tc.expectedToolErrMsg)
return
}
// Parse the result and get the text content if no error
require.Equal(t, stubbedDiff, textContent.Text)
})
}
}
func viewerQuery(login string) githubv4mock.Matcher {
return githubv4mock.NewQueryMatcher(
struct {
Viewer struct {
Login githubv4.String
} `graphql:"viewer"`
}{},
map[string]any{},
githubv4mock.DataResponse(map[string]any{
"viewer": map[string]any{
"login": login,
},
}),
)
}
type getLatestPendingReviewQueryReview struct {
id string
state string
url string
}
type getLatestPendingReviewQueryParams struct {
author string
owner string
repo string
prNum int32
reviews []getLatestPendingReviewQueryReview
}
func getLatestPendingReviewQuery(p getLatestPendingReviewQueryParams) githubv4mock.Matcher {
return githubv4mock.NewQueryMatcher(
struct {
Repository struct {
PullRequest struct {
Reviews struct {
Nodes []struct {
ID githubv4.ID
State githubv4.PullRequestReviewState
URL githubv4.URI
}
} `graphql:"reviews(first: 1, author: $author)"`
} `graphql:"pullRequest(number: $prNum)"`
} `graphql:"repository(owner: $owner, name: $name)"`
}{},
map[string]any{
"author": githubv4.String(p.author),
"owner": githubv4.String(p.owner),
"name": githubv4.String(p.repo),
"prNum": githubv4.Int(p.prNum),
},
githubv4mock.DataResponse(
map[string]any{
"repository": map[string]any{
"pullRequest": map[string]any{
"reviews": map[string]any{
"nodes": []any{
map[string]any{
"id": p.reviews[0].id,
"state": p.reviews[0].state,
"url": p.reviews[0].url,
},
},
},
},
},
},
),
)
}
func TestAddReplyToPullRequestComment(t *testing.T) {
t.Parallel()
// Verify tool definition once
serverTool := AddReplyToPullRequestComment(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name, tool))
assert.Equal(t, "add_reply_to_pull_request_comment", 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, "pullNumber")
assert.Contains(t, schema.Properties, "commentId")
assert.Contains(t, schema.Properties, "body")
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "pullNumber", "commentId", "body"})
// Setup mock reply comment for success case
mockReplyComment := &github.PullRequestComment{
ID: github.Ptr(int64(456)),
Body: github.Ptr("This is a reply to the comment"),
InReplyTo: github.Ptr(int64(123)),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42#discussion_r456"),
User: &github.User{
Login: github.Ptr("responder"),
},
CreatedAt: &github.Timestamp{Time: time.Now()},
UpdatedAt: &github.Timestamp{Time: time.Now()},
}
tests := []struct {
name string
mockedClient *http.Client
requestArgs map[string]any
expectToolError bool
expectedToolErrMsg string
}{
{
name: "successful reply to pull request comment",
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"commentId": float64(123),
"body": "This is a reply to the comment",
},
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposPullsCommentsByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusCreated)
responseData, _ := json.Marshal(mockReplyComment)
_, _ = w.Write(responseData)
},
}),
},
{
name: "missing required parameter owner",
requestArgs: map[string]any{
"repo": "repo",
"pullNumber": float64(42),
"commentId": float64(123),
"body": "This is a reply to the comment",
},
expectToolError: true,
expectedToolErrMsg: "missing required parameter: owner",
},
{
name: "missing required parameter repo",
requestArgs: map[string]any{
"owner": "owner",
"pullNumber": float64(42),
"commentId": float64(123),
"body": "This is a reply to the comment",
},
expectToolError: true,
expectedToolErrMsg: "missing required parameter: repo",
},
{
name: "missing required parameter pullNumber",
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"commentId": float64(123),
"body": "This is a reply to the comment",
},
expectToolError: true,
expectedToolErrMsg: "missing required parameter: pullNumber",
},
{
name: "missing required parameter commentId",
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"body": "This is a reply to the comment",
},
expectToolError: true,
expectedToolErrMsg: "missing required parameter: commentId",
},
{
name: "missing required parameter body",
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"commentId": float64(123),
},
expectToolError: true,
expectedToolErrMsg: "missing required parameter: body",
},
{
name: "API error when adding reply",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposPullsCommentsByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
},
}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"commentId": float64(123),
"body": "This is a reply to the comment",
},
expectToolError: true,
expectedToolErrMsg: "failed to add reply to pull request comment",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
// Setup client with mock
client := mustNewGHClient(t, tc.mockedClient)
serverTool := AddReplyToPullRequestComment(translations.NullTranslationHelper)
deps := BaseDeps{
Client: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
if tc.expectToolError {
require.True(t, result.IsError)
errorContent := getErrorResult(t, result)
assert.Contains(t, errorContent.Text, tc.expectedToolErrMsg)
return
}
// Parse the result and verify it's not an error
require.False(t, result.IsError)
textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, "This is a reply to the comment")
})
}
}
func TestResolveReviewThread(t *testing.T) {
t.Parallel()
tests := []struct {
name string
requestArgs map[string]any
mockedClient *http.Client
expectToolError bool
expectedToolErrMsg string
expectedResult string
}{
{
name: "successful resolve thread",
requestArgs: map[string]any{
"method": "resolve_thread",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"threadId": "PRRT_kwDOTest123",
},
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewMutationMatcher(
struct {
ResolveReviewThread struct {
Thread struct {
ID githubv4.ID
IsResolved githubv4.Boolean
}
} `graphql:"resolveReviewThread(input: $input)"`
}{},
githubv4.ResolveReviewThreadInput{
ThreadID: githubv4.ID("PRRT_kwDOTest123"),
},
nil,
githubv4mock.DataResponse(map[string]any{
"resolveReviewThread": map[string]any{
"thread": map[string]any{
"id": "PRRT_kwDOTest123",
"isResolved": true,
},
},
}),
),
),
expectedResult: "review thread resolved successfully",
},
{
name: "successful unresolve thread",
requestArgs: map[string]any{
"method": "unresolve_thread",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"threadId": "PRRT_kwDOTest123",
},
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewMutationMatcher(
struct {
UnresolveReviewThread struct {
Thread struct {
ID githubv4.ID
IsResolved githubv4.Boolean
}
} `graphql:"unresolveReviewThread(input: $input)"`
}{},
githubv4.UnresolveReviewThreadInput{
ThreadID: githubv4.ID("PRRT_kwDOTest123"),
},
nil,
githubv4mock.DataResponse(map[string]any{
"unresolveReviewThread": map[string]any{
"thread": map[string]any{
"id": "PRRT_kwDOTest123",
"isResolved": false,
},
},
}),
),
),
expectedResult: "review thread unresolved successfully",
},
{
name: "empty threadId for resolve",
requestArgs: map[string]any{
"method": "resolve_thread",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"threadId": "",
},
mockedClient: githubv4mock.NewMockedHTTPClient(),
expectToolError: true,
expectedToolErrMsg: "threadId is required",
},
{
name: "empty threadId for unresolve",
requestArgs: map[string]any{
"method": "unresolve_thread",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"threadId": "",
},
mockedClient: githubv4mock.NewMockedHTTPClient(),
expectToolError: true,
expectedToolErrMsg: "threadId is required",
},
{
name: "omitted threadId for resolve",
requestArgs: map[string]any{
"method": "resolve_thread",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
mockedClient: githubv4mock.NewMockedHTTPClient(),
expectToolError: true,
expectedToolErrMsg: "threadId is required",
},
{
name: "omitted threadId for unresolve",
requestArgs: map[string]any{
"method": "unresolve_thread",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
},
mockedClient: githubv4mock.NewMockedHTTPClient(),
expectToolError: true,
expectedToolErrMsg: "threadId is required",
},
{
name: "thread not found",
requestArgs: map[string]any{
"method": "resolve_thread",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
"threadId": "PRRT_invalid",
},
mockedClient: githubv4mock.NewMockedHTTPClient(
githubv4mock.NewMutationMatcher(
struct {
ResolveReviewThread struct {
Thread struct {
ID githubv4.ID
IsResolved githubv4.Boolean
}
} `graphql:"resolveReviewThread(input: $input)"`
}{},
githubv4.ResolveReviewThreadInput{
ThreadID: githubv4.ID("PRRT_invalid"),
},
nil,
githubv4mock.ErrorResponse("Could not resolve to a PullRequestReviewThread with the id of 'PRRT_invalid'"),
),
),
expectToolError: true,
expectedToolErrMsg: "Could not resolve to a PullRequestReviewThread",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
// Setup client with mock
client := githubv4.NewClient(tc.mockedClient)
serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
deps := BaseDeps{
GQLClient: client,
}
handler := serverTool.Handler(deps)
// Create call request
request := createMCPRequest(tc.requestArgs)
// Call handler
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
textContent := getTextResult(t, result)
if tc.expectToolError {
require.True(t, result.IsError)
assert.Contains(t, textContent.Text, tc.expectedToolErrMsg)
return
}
require.False(t, result.IsError)
assert.Equal(t, tc.expectedResult, textContent.Text)
})
}
}