Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/feature-flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -349,4 +349,18 @@ runtime behavior (such as output formatting) won't appear here.
- `perPage`: Results per page for pagination (min 1, max 100) (number, optional)
- `repo`: The name of the repository (string, required)

### `thread_resolution_reason`

- **pull_request_review_write** - Write operations (create, submit, delete) on pull request reviews
- **Required OAuth Scopes**: `repo`
- `body`: Review comment text (string, optional)
- `commitID`: SHA of commit to review (string, optional)
- `event`: Review action to perform. (string, optional)
- `method`: The write operation to perform on pull request review. (string, required)
- `owner`: Repository owner (string, required)
- `pullNumber`: Pull request number (number, required)
- `repo`: Repository name (string, required)
- `resolutionReason`: Optional reason for resolving a Copilot code review thread: addressed, wont-fix, or invalid. (string, optional)
- `threadId`: The node ID of the review thread (e.g., PRRT_kwDOxxx). Required for resolve_thread and unresolve_thread methods. Get thread IDs from pull_request_read with method get_review_comments. (string, optional)

<!-- END AUTOMATED FEATURE FLAG TOOLS -->
4 changes: 4 additions & 0 deletions pkg/github/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ const FeatureFlagIssueDependencies = "issue_dependencies"
// opt-in.
const FeatureFlagDuplicateDetection = "duplicate_detection"

// FeatureFlagThreadResolutionReason exposes resolution reasons for Copilot review threads.
const FeatureFlagThreadResolutionReason = "thread_resolution_reason"

// AllowedFeatureFlags is the allowlist of feature flags that can be enabled
// by users via --features CLI flag or X-MCP-Features HTTP header.
// Only flags in this list are accepted; unknown flags are silently ignored.
Expand All @@ -48,6 +51,7 @@ var AllowedFeatureFlags = []string{
FeatureFlagFileBlame,
FeatureFlagIssueDependencies,
FeatureFlagDuplicateDetection,
FeatureFlagThreadResolutionReason,
}

// InsidersFeatureFlags is the list of feature flags that insiders mode enables.
Expand Down
71 changes: 71 additions & 0 deletions pkg/github/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"github.com/github/github-mcp-server/pkg/translations"
"github.com/google/jsonschema-go/jsonschema"
"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -200,6 +201,11 @@ func TestResolveFeatureFlags(t *testing.T) {
insidersMode: false,
expectedFlags: []string{FeatureFlagIssuesGranular},
},
{
name: "thread resolution reason can be directly enabled",
enabledFeatures: []string{FeatureFlagThreadResolutionReason},
expectedFlags: []string{FeatureFlagThreadResolutionReason},
},
{
name: "insiders does not enable user-only allowed flags",
enabledFeatures: nil,
Expand Down Expand Up @@ -227,3 +233,68 @@ func TestResolveFeatureFlags(t *testing.T) {
})
}
}

func TestThreadResolutionReasonToolVariants(t *testing.T) {
tests := []struct {
name string
flags []string
host utils.HostType
toolName string
hasReason bool
}{
{
name: "consolidated flag off",
toolName: "pull_request_review_write",
},
{
name: "consolidated flag on",
flags: []string{FeatureFlagThreadResolutionReason},
toolName: "pull_request_review_write",
hasReason: true,
},
{
name: "granular flag off",
flags: []string{FeatureFlagPullRequestsGranular},
toolName: "resolve_review_thread",
},
{
name: "granular flag on",
flags: []string{FeatureFlagPullRequestsGranular, FeatureFlagThreadResolutionReason},
toolName: "resolve_review_thread",
hasReason: true,
},
{
name: "consolidated flag on GHES",
flags: []string{FeatureFlagThreadResolutionReason},
host: utils.HostTypeGHES,
toolName: "pull_request_review_write",
},
{
name: "granular flag on GHES",
flags: []string{FeatureFlagPullRequestsGranular, FeatureFlagThreadResolutionReason},
host: utils.HostTypeGHES,
toolName: "resolve_review_thread",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
inv, err := NewInventory(translations.NullTranslationHelper, WithHost(tt.host)).
WithToolsets([]string{"all"}).
WithFeatureChecker(featureCheckerFor(tt.flags...)).
Build()
require.NoError(t, err)

var matches []inventory.ServerTool
for _, tool := range inv.AvailableTools(context.Background()) {
if tool.Tool.Name == tt.toolName {
matches = append(matches, tool)
}
}
require.Len(t, matches, 1)
schema := matches[0].Tool.InputSchema.(*jsonschema.Schema)
_, hasReason := schema.Properties["resolutionReason"]
assert.Equal(t, tt.hasReason, hasReason)
})
}
}
90 changes: 58 additions & 32 deletions pkg/github/granular_tools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
func granularToolsForToolset(toolsetID inventory.ToolsetID, featureFlag string) []inventory.ServerTool {
var result []inventory.ServerTool
for _, tool := range AllTools(translations.NullTranslationHelper) {
if tool.Toolset.ID == toolsetID && tool.FeatureFlagEnable == featureFlag {
if tool.Toolset.ID == toolsetID && tool.FeatureFlagEnable == featureFlag && len(tool.FeatureFlagEnableAll) == 0 {
result = append(result, tool)
}
}
Expand Down Expand Up @@ -1728,38 +1728,64 @@ func TestGranularAddPullRequestReviewComment(t *testing.T) {
}

func TestGranularResolveReviewThread(t *testing.T) {
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_123"),
},
nil,
githubv4mock.DataResponse(map[string]any{
"resolveReviewThread": map[string]any{
"thread": map[string]any{"id": "PRRT_123", "isResolved": true},
},
}),
),
)
gqlClient := githubv4.NewClient(mockedClient)
deps := BaseDeps{GQLClient: gqlClient}
serverTool := GranularResolveReviewThread(translations.NullTranslationHelper)
handler := serverTool.Handler(deps)
tests := []struct {
name string
withResolutionReason bool
resolutionReason *string
expectedReason *string
}{
{
name: "enabled variant forwards resolution reason",
withResolutionReason: true,
resolutionReason: gogithub.Ptr("addressed"),
expectedReason: gogithub.Ptr("addressed"),
},
{name: "default variant omits resolution reason", resolutionReason: gogithub.Ptr("addressed")},
{name: "without resolution reason"},
}

request := createMCPRequest(map[string]any{
"threadID": "PRRT_123",
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
assert.False(t, result.IsError)
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
mockedClient := githubv4mock.NewMockedHTTPClient(
githubv4mock.NewMutationMatcher(
struct {
ResolveReviewThread struct {
Thread struct {
ID githubv4.ID
IsResolved githubv4.Boolean
}
} `graphql:"resolveReviewThread(input: $input)"`
}{},
ResolveReviewThreadInput{
ThreadID: githubv4.ID("PRRT_123"),
ResolutionReason: newGQLStringlikePtr[githubv4.String](tc.expectedReason),
},
nil,
githubv4mock.DataResponse(map[string]any{
"resolveReviewThread": map[string]any{
"thread": map[string]any{"id": "PRRT_123", "isResolved": true},
},
}),
),
)
gqlClient := githubv4.NewClient(mockedClient)
deps := BaseDeps{GQLClient: gqlClient}
serverTool := GranularResolveReviewThread(translations.NullTranslationHelper)
if tc.withResolutionReason {
serverTool = GranularResolveReviewThreadWithResolutionReason(translations.NullTranslationHelper)
}
handler := serverTool.Handler(deps)

args := map[string]any{"threadID": "PRRT_123"}
if tc.resolutionReason != nil {
args["resolutionReason"] = *tc.resolutionReason
}
request := createMCPRequest(args)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
assert.False(t, result.IsError)
})
}
}

func TestGranularUnresolveReviewThread(t *testing.T) {
Expand Down
71 changes: 59 additions & 12 deletions pkg/github/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -1760,17 +1760,34 @@ func UpdatePullRequestBranch(t translations.TranslationHelperFunc) inventory.Ser
}

type PullRequestReviewWriteParams struct {
Method string
Owner string
Repo string
PullNumber int32
Body string
Event string
CommitID *string
ThreadID string
Method string
Owner string
Repo string
PullNumber int32
Body string
Event string
CommitID *string
ThreadID string
ResolutionReason *string
}

func PullRequestReviewWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
return pullRequestReviewWrite(t, false, toolConfig{})
}

// PullRequestReviewWriteWithResolutionReason creates the feature-gated review write variant with resolution reasons.
func PullRequestReviewWriteWithResolutionReason(t translations.TranslationHelperFunc, opts ...ToolOption) inventory.ServerTool {
cfg := newToolConfig(opts)
st := pullRequestReviewWrite(t, true, cfg)
if cfg.hostType == utils.HostTypeGHES {
st.Enabled = func(context.Context) (bool, error) { return false, nil }
}
return st
}

func pullRequestReviewWrite(t translations.TranslationHelperFunc, withResolutionReason bool, cfg toolConfig) inventory.ServerTool {
withResolutionReason = withResolutionReason && cfg.hostType != utils.HostTypeGHES

schema := &jsonschema.Schema{
Type: "object",
Properties: map[string]*jsonschema.Schema{
Expand Down Expand Up @@ -1814,6 +1831,12 @@ func PullRequestReviewWrite(t translations.TranslationHelperFunc) inventory.Serv
},
Required: []string{"method", "owner", "repo", "pullNumber"},
}
if withResolutionReason {
schema.Properties["resolutionReason"] = &jsonschema.Schema{
Type: "string",
Description: "Optional reason for resolving a Copilot code review thread: addressed, wont-fix, or invalid.",
}
}

st := NewTool(
ToolsetMetadataPullRequests,
Expand Down Expand Up @@ -1858,7 +1881,11 @@ Available methods:
result, err := DeletePendingPullRequestReview(ctx, client, params)
return result, nil, err
case "resolve_thread":
result, err := ResolveReviewThread(ctx, client, params.ThreadID, true)
if !withResolutionReason {
result, err := ResolveReviewThread(ctx, client, params.ThreadID, true)
return result, nil, err
}
result, err := ResolveReviewThreadWithReason(ctx, client, params.ThreadID, params.ResolutionReason, true)
return result, nil, err
case "unresolve_thread":
result, err := ResolveReviewThread(ctx, client, params.ThreadID, false)
Expand All @@ -1867,7 +1894,15 @@ Available methods:
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", params.Method)), nil, nil
}
})
st.FeatureFlagDisable = []string{FeatureFlagPullRequestsGranular}
if withResolutionReason {
st.FeatureFlagEnable = FeatureFlagThreadResolutionReason
st.FeatureFlagDisable = []string{FeatureFlagPullRequestsGranular}
} else {
st.FeatureFlagDisable = []string{FeatureFlagPullRequestsGranular}
if cfg.hostType != utils.HostTypeGHES {
st.FeatureFlagDisable = append(st.FeatureFlagDisable, FeatureFlagThreadResolutionReason)
}
}
return st
}

Expand Down Expand Up @@ -2094,8 +2129,19 @@ func DeletePendingPullRequestReview(ctx context.Context, client *githubv4.Client
return utils.NewToolResultText("pending pull request review successfully deleted"), nil
}

// ResolveReviewThreadInput is the GraphQL input for resolving a review thread.
type ResolveReviewThreadInput struct {
ThreadID githubv4.ID `json:"threadId"`
ResolutionReason *githubv4.String `json:"resolutionReason,omitempty"`
}

// ResolveReviewThread resolves or unresolves a PR review thread using GraphQL mutations.
func ResolveReviewThread(ctx context.Context, client *githubv4.Client, threadID string, resolve bool) (*mcp.CallToolResult, error) {
return ResolveReviewThreadWithReason(ctx, client, threadID, nil, resolve)
}

// ResolveReviewThreadWithReason resolves or unresolves a PR review thread with an optional resolution reason.
func ResolveReviewThreadWithReason(ctx context.Context, client *githubv4.Client, threadID string, resolutionReason *string, resolve bool) (*mcp.CallToolResult, error) {
if threadID == "" {
return utils.NewToolResultError("threadId is required for resolve_thread and unresolve_thread methods"), nil
}
Expand All @@ -2110,8 +2156,9 @@ func ResolveReviewThread(ctx context.Context, client *githubv4.Client, threadID
} `graphql:"resolveReviewThread(input: $input)"`
}

input := githubv4.ResolveReviewThreadInput{
ThreadID: githubv4.ID(threadID),
input := ResolveReviewThreadInput{
ThreadID: githubv4.ID(threadID),
ResolutionReason: newGQLStringlikePtr[githubv4.String](resolutionReason),
}

if err := client.Mutate(ctx, &mutation, input, nil); err != nil {
Expand Down
Loading
Loading