Skip to content
Open
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
58 changes: 58 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,55 @@ func TestResolveFeatureFlags(t *testing.T) {
})
}
}

func TestThreadResolutionReasonToolVariants(t *testing.T) {
tests := []struct {
name string
flags []string
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,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
inv, err := NewInventory(translations.NullTranslationHelper).
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
60 changes: 48 additions & 12 deletions pkg/github/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -1760,17 +1760,27 @@ 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)
}

// PullRequestReviewWriteWithResolutionReason creates the feature-gated review write variant with resolution reasons.
func PullRequestReviewWriteWithResolutionReason(t translations.TranslationHelperFunc) inventory.ServerTool {
return pullRequestReviewWrite(t, true)
}

func pullRequestReviewWrite(t translations.TranslationHelperFunc, withResolutionReason bool) inventory.ServerTool {
schema := &jsonschema.Schema{
Type: "object",
Properties: map[string]*jsonschema.Schema{
Expand Down Expand Up @@ -1814,6 +1824,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 +1874,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 +1887,12 @@ 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, FeatureFlagThreadResolutionReason}
}
return st
}

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

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 +2145,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