f929c58c6b
* Add CSV output for list tools under insiders mode * fix: resolve rebase feature flag conflicts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Simplify feature-flag handling: collapse CSV dual-variant + skip filtering when no checker (#2516) * refactor: generic toolset+name sort, clarify feature flag intent Address review feedback on #2450: - Collapse the three near-identical sort helpers in pkg/inventory/filters.go into a generic sortByToolsetThenName so adding new inventory item types doesn't require copying the comparator. - Expand the doc comments on the three *WithoutFeatureFiltering helpers to spell out why they exist: HTTP mode builds a static (process-wide) inventory as an upper bound, but per-request feature flags from headers (X-MCP-Features, X-MCP-Insiders) are evaluated later, so feature-flagged variants must be preserved here. - Strengthen the doc comment on ResolveFeatureFlags to make the contract explicit: user-supplied flags are validated against AllowedFeatureFlags, but insiders expansion deliberately is not — InsidersFeatureFlags may include server-controlled flags that are not user-toggleable. CORS comments are intentionally left for the PR author. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(feature-flags): clarify allowed and insiders sets are independent Also add tests covering: - a user-toggleable flag (FeatureFlagIssuesGranular) that insiders does not turn on automatically - insiders mode not turning on user-only allowed flags Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(inventory): collapse three *WithoutFeatureFiltering helpers into StaticUpperBound The three parallel methods (AvailableToolsWithoutFeatureFiltering, AvailableResourceTemplatesWithoutFeatureFiltering, AvailablePromptsWithoutFeatureFiltering) were always called as a triple in exactly two places: HTTP buildStaticInventory and its test mirror. They exist because the dual-variant pattern (sibling tools with mirrored FeatureFlagEnable / FeatureFlagDisable on the same name, e.g. CSV output) makes feature filtering at static-build time impossible — both variants must be kept and resolved per-request. Replace the three with one method, Inventory.StaticUpperBound(ctx), that returns (tools, resources, prompts) and carries the rationale in its doc comment. Reduces API surface, eliminates the triplication, and makes the single "skip feature filtering" concept obvious to readers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: simplify feature-flag handling Two related simplifications, both about treating insiders as a meta flag that expands once at startup and then stops mattering: - Collapse CSV's dual-variant pattern into a single tool whose handler performs a runtime feature-flag check via deps.IsFeatureEnabled. CSV is a pure response-format toggle, not a schema change, so it does not need the dual-name pattern that genuine schema variants (granular issues/PRs) still use. - When no feature checker is installed, skip feature-flag filtering and return the full upper bound. The static HTTP inventory now uses plain AvailableTools/Resources/Prompts; the per-request inventory always installs a checker, so MCP registration (which serves a tool name once) always sees a deduplicated set. The bespoke StaticUpperBound helper and the isToolEnabledWithFeatureFlags split go away. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci(mcp-diff): add insiders + per-feature configs The mcp-diff matrix now includes: - --insiders (and --insiders --read-only) - one config per github.AllowedFeatureFlags entry, generated by script/print-mcp-diff-configs so new user-controllable flags get diffed automatically without editing the workflow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(insiders): explain feature-flag resolution for contributors Adds a 'How feature flags are resolved' section covering: - Insiders is a meta flag, like 'all'/'default' for toolsets - User input -> allowlist filter -> insiders expansion -> server-side fallback (remote only) - AllowedFeatureFlags vs InsidersFeatureFlags are independent - How to add a new feature flag, including the TestGitHubPackageDoesNotReadInsidersMode guard Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(inventory): make feature-flag gating a regular ToolFilter Move tool feature-flag evaluation out of isToolEnabled and into a ToolFilter installed at the head of the pipeline by Build() when WithFeatureChecker received a non-nil checker. The 'no checker = no filtering' contract is now expressed structurally (the filter isn't installed) instead of by a runtime nil check inside the helper. Resources and prompts have no filter pipeline, so they call the now-pure featureFlagAllowed helper behind an explicit r.featureChecker != nil guard at the iteration site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * perf(inventory): cache extracted toolset IDs in sort comparator Avoid evaluating the extractor closures up to three times per comparison. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: correct MCP features header in cors * docs: regenerate README for CSV output toolset Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove duplicate MCPFeaturesHeader from CORS headers * ci(mcp-diff): add streamable-http job with header-based configs Adds a sibling mcp-diff-http job that exercises the streamable-http transport against a shared HTTP server, with per-config settings supplied via X-MCP-* request headers — mirroring how the remote server is invoked in production (server-side defaults + per-user header overrides). The config generator gains a -transport flag: - stdio (default, unchanged behaviour) - http-headers (emits headers-only configs targeting a shared server) Two new combined entries layer multiple headers together as a smoke test for header-merging regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: regenerate after merging main Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Sam Morrow <info@sam-morrow.com> Co-authored-by: sammorrowdrums <sammorrowdrums@github.com>
3888 lines
118 KiB
Go
3888 lines
118 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 (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")
|
|
})
|
|
}
|
|
|
|
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)
|
|
})
|
|
}
|
|
}
|