Skip to content

Commit febc329

Browse files
authored
Expose Copilot review thread resolution reasons (#3123)
1 parent a00dc31 commit febc329

10 files changed

Lines changed: 365 additions & 66 deletions

docs/feature-flags.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,4 +354,18 @@ runtime behavior (such as output formatting) won't appear here.
354354
- `perPage`: Results per page for pagination (min 1, max 100) (number, optional)
355355
- `repo`: The name of the repository (string, required)
356356

357+
### `thread_resolution_reason`
358+
359+
- **pull_request_review_write** - Write operations (create, submit, delete) on pull request reviews
360+
- **Required OAuth Scopes**: `repo`
361+
- `body`: Review comment text (string, optional)
362+
- `commitID`: SHA of commit to review (string, optional)
363+
- `event`: Review action to perform. (string, optional)
364+
- `method`: The write operation to perform on pull request review. (string, required)
365+
- `owner`: Repository owner (string, required)
366+
- `pullNumber`: Pull request number (number, required)
367+
- `repo`: Repository name (string, required)
368+
- `resolutionReason`: Optional reason for resolving a Copilot code review thread: addressed, wont-fix, or invalid. (string, optional)
369+
- `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)
370+
357371
<!-- END AUTOMATED FEATURE FLAG TOOLS -->

pkg/github/feature_flags.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ const FeatureFlagIssueDependencies = "issue_dependencies"
3434
// opt-in.
3535
const FeatureFlagDuplicateDetection = "duplicate_detection"
3636

37+
// FeatureFlagThreadResolutionReason exposes resolution reasons for Copilot review threads.
38+
const FeatureFlagThreadResolutionReason = "thread_resolution_reason"
39+
3740
// AllowedFeatureFlags is the allowlist of feature flags that can be enabled
3841
// by users via --features CLI flag or X-MCP-Features HTTP header.
3942
// Only flags in this list are accepted; unknown flags are silently ignored.
@@ -48,6 +51,7 @@ var AllowedFeatureFlags = []string{
4851
FeatureFlagFileBlame,
4952
FeatureFlagIssueDependencies,
5053
FeatureFlagDuplicateDetection,
54+
FeatureFlagThreadResolutionReason,
5155
}
5256

5357
// InsidersFeatureFlags is the list of feature flags that insiders mode enables.

pkg/github/feature_flags_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"testing"
77

88
"github.com/github/github-mcp-server/pkg/translations"
9+
"github.com/google/jsonschema-go/jsonschema"
910
"github.com/modelcontextprotocol/go-sdk/mcp"
1011
"github.com/stretchr/testify/assert"
1112
"github.com/stretchr/testify/require"
@@ -200,6 +201,11 @@ func TestResolveFeatureFlags(t *testing.T) {
200201
insidersMode: false,
201202
expectedFlags: []string{FeatureFlagIssuesGranular},
202203
},
204+
{
205+
name: "thread resolution reason can be directly enabled",
206+
enabledFeatures: []string{FeatureFlagThreadResolutionReason},
207+
expectedFlags: []string{FeatureFlagThreadResolutionReason},
208+
},
203209
{
204210
name: "insiders does not enable user-only allowed flags",
205211
enabledFeatures: nil,
@@ -227,3 +233,68 @@ func TestResolveFeatureFlags(t *testing.T) {
227233
})
228234
}
229235
}
236+
237+
func TestThreadResolutionReasonToolVariants(t *testing.T) {
238+
tests := []struct {
239+
name string
240+
flags []string
241+
host utils.HostType
242+
toolName string
243+
hasReason bool
244+
}{
245+
{
246+
name: "consolidated flag off",
247+
toolName: "pull_request_review_write",
248+
},
249+
{
250+
name: "consolidated flag on",
251+
flags: []string{FeatureFlagThreadResolutionReason},
252+
toolName: "pull_request_review_write",
253+
hasReason: true,
254+
},
255+
{
256+
name: "granular flag off",
257+
flags: []string{FeatureFlagPullRequestsGranular},
258+
toolName: "resolve_review_thread",
259+
},
260+
{
261+
name: "granular flag on",
262+
flags: []string{FeatureFlagPullRequestsGranular, FeatureFlagThreadResolutionReason},
263+
toolName: "resolve_review_thread",
264+
hasReason: true,
265+
},
266+
{
267+
name: "consolidated flag on GHES",
268+
flags: []string{FeatureFlagThreadResolutionReason},
269+
host: utils.HostTypeGHES,
270+
toolName: "pull_request_review_write",
271+
},
272+
{
273+
name: "granular flag on GHES",
274+
flags: []string{FeatureFlagPullRequestsGranular, FeatureFlagThreadResolutionReason},
275+
host: utils.HostTypeGHES,
276+
toolName: "resolve_review_thread",
277+
},
278+
}
279+
280+
for _, tt := range tests {
281+
t.Run(tt.name, func(t *testing.T) {
282+
inv, err := NewInventory(translations.NullTranslationHelper, WithHost(tt.host)).
283+
WithToolsets([]string{"all"}).
284+
WithFeatureChecker(featureCheckerFor(tt.flags...)).
285+
Build()
286+
require.NoError(t, err)
287+
288+
var matches []inventory.ServerTool
289+
for _, tool := range inv.AvailableTools(context.Background()) {
290+
if tool.Tool.Name == tt.toolName {
291+
matches = append(matches, tool)
292+
}
293+
}
294+
require.Len(t, matches, 1)
295+
schema := matches[0].Tool.InputSchema.(*jsonschema.Schema)
296+
_, hasReason := schema.Properties["resolutionReason"]
297+
assert.Equal(t, tt.hasReason, hasReason)
298+
})
299+
}
300+
}

pkg/github/granular_tools_test.go

Lines changed: 58 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
func granularToolsForToolset(toolsetID inventory.ToolsetID, featureFlag string) []inventory.ServerTool {
2424
var result []inventory.ServerTool
2525
for _, tool := range AllTools(translations.NullTranslationHelper) {
26-
if tool.Toolset.ID == toolsetID && tool.FeatureFlagEnable == featureFlag {
26+
if tool.Toolset.ID == toolsetID && tool.FeatureFlagEnable == featureFlag && len(tool.FeatureFlagEnableAll) == 0 {
2727
result = append(result, tool)
2828
}
2929
}
@@ -1728,38 +1728,64 @@ func TestGranularAddPullRequestReviewComment(t *testing.T) {
17281728
}
17291729

17301730
func TestGranularResolveReviewThread(t *testing.T) {
1731-
mockedClient := githubv4mock.NewMockedHTTPClient(
1732-
githubv4mock.NewMutationMatcher(
1733-
struct {
1734-
ResolveReviewThread struct {
1735-
Thread struct {
1736-
ID githubv4.ID
1737-
IsResolved githubv4.Boolean
1738-
}
1739-
} `graphql:"resolveReviewThread(input: $input)"`
1740-
}{},
1741-
githubv4.ResolveReviewThreadInput{
1742-
ThreadID: githubv4.ID("PRRT_123"),
1743-
},
1744-
nil,
1745-
githubv4mock.DataResponse(map[string]any{
1746-
"resolveReviewThread": map[string]any{
1747-
"thread": map[string]any{"id": "PRRT_123", "isResolved": true},
1748-
},
1749-
}),
1750-
),
1751-
)
1752-
gqlClient := githubv4.NewClient(mockedClient)
1753-
deps := BaseDeps{GQLClient: gqlClient}
1754-
serverTool := GranularResolveReviewThread(translations.NullTranslationHelper)
1755-
handler := serverTool.Handler(deps)
1731+
tests := []struct {
1732+
name string
1733+
withResolutionReason bool
1734+
resolutionReason *string
1735+
expectedReason *string
1736+
}{
1737+
{
1738+
name: "enabled variant forwards resolution reason",
1739+
withResolutionReason: true,
1740+
resolutionReason: gogithub.Ptr("addressed"),
1741+
expectedReason: gogithub.Ptr("addressed"),
1742+
},
1743+
{name: "default variant omits resolution reason", resolutionReason: gogithub.Ptr("addressed")},
1744+
{name: "without resolution reason"},
1745+
}
17561746

1757-
request := createMCPRequest(map[string]any{
1758-
"threadID": "PRRT_123",
1759-
})
1760-
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1761-
require.NoError(t, err)
1762-
assert.False(t, result.IsError)
1747+
for _, tc := range tests {
1748+
t.Run(tc.name, func(t *testing.T) {
1749+
mockedClient := githubv4mock.NewMockedHTTPClient(
1750+
githubv4mock.NewMutationMatcher(
1751+
struct {
1752+
ResolveReviewThread struct {
1753+
Thread struct {
1754+
ID githubv4.ID
1755+
IsResolved githubv4.Boolean
1756+
}
1757+
} `graphql:"resolveReviewThread(input: $input)"`
1758+
}{},
1759+
ResolveReviewThreadInput{
1760+
ThreadID: githubv4.ID("PRRT_123"),
1761+
ResolutionReason: newGQLStringlikePtr[githubv4.String](tc.expectedReason),
1762+
},
1763+
nil,
1764+
githubv4mock.DataResponse(map[string]any{
1765+
"resolveReviewThread": map[string]any{
1766+
"thread": map[string]any{"id": "PRRT_123", "isResolved": true},
1767+
},
1768+
}),
1769+
),
1770+
)
1771+
gqlClient := githubv4.NewClient(mockedClient)
1772+
deps := BaseDeps{GQLClient: gqlClient}
1773+
serverTool := GranularResolveReviewThread(translations.NullTranslationHelper)
1774+
if tc.withResolutionReason {
1775+
serverTool = GranularResolveReviewThreadWithResolutionReason(translations.NullTranslationHelper)
1776+
}
1777+
handler := serverTool.Handler(deps)
1778+
1779+
args := map[string]any{"threadID": "PRRT_123"}
1780+
if tc.resolutionReason != nil {
1781+
args["resolutionReason"] = *tc.resolutionReason
1782+
}
1783+
request := createMCPRequest(args)
1784+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1785+
require.NoError(t, err)
1786+
assert.False(t, result.IsError)
1787+
})
1788+
}
17631789
}
17641790

17651791
func TestGranularUnresolveReviewThread(t *testing.T) {

pkg/github/pullrequests.go

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,17 +1760,34 @@ func UpdatePullRequestBranch(t translations.TranslationHelperFunc) inventory.Ser
17601760
}
17611761

17621762
type PullRequestReviewWriteParams struct {
1763-
Method string
1764-
Owner string
1765-
Repo string
1766-
PullNumber int32
1767-
Body string
1768-
Event string
1769-
CommitID *string
1770-
ThreadID string
1763+
Method string
1764+
Owner string
1765+
Repo string
1766+
PullNumber int32
1767+
Body string
1768+
Event string
1769+
CommitID *string
1770+
ThreadID string
1771+
ResolutionReason *string
17711772
}
17721773

17731774
func PullRequestReviewWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
1775+
return pullRequestReviewWrite(t, false, toolConfig{})
1776+
}
1777+
1778+
// PullRequestReviewWriteWithResolutionReason creates the feature-gated review write variant with resolution reasons.
1779+
func PullRequestReviewWriteWithResolutionReason(t translations.TranslationHelperFunc, opts ...ToolOption) inventory.ServerTool {
1780+
cfg := newToolConfig(opts)
1781+
st := pullRequestReviewWrite(t, true, cfg)
1782+
if cfg.hostType == utils.HostTypeGHES {
1783+
st.Enabled = func(context.Context) (bool, error) { return false, nil }
1784+
}
1785+
return st
1786+
}
1787+
1788+
func pullRequestReviewWrite(t translations.TranslationHelperFunc, withResolutionReason bool, cfg toolConfig) inventory.ServerTool {
1789+
withResolutionReason = withResolutionReason && cfg.hostType != utils.HostTypeGHES
1790+
17741791
schema := &jsonschema.Schema{
17751792
Type: "object",
17761793
Properties: map[string]*jsonschema.Schema{
@@ -1814,6 +1831,12 @@ func PullRequestReviewWrite(t translations.TranslationHelperFunc) inventory.Serv
18141831
},
18151832
Required: []string{"method", "owner", "repo", "pullNumber"},
18161833
}
1834+
if withResolutionReason {
1835+
schema.Properties["resolutionReason"] = &jsonschema.Schema{
1836+
Type: "string",
1837+
Description: "Optional reason for resolving a Copilot code review thread: addressed, wont-fix, or invalid.",
1838+
}
1839+
}
18171840

18181841
st := NewTool(
18191842
ToolsetMetadataPullRequests,
@@ -1858,7 +1881,11 @@ Available methods:
18581881
result, err := DeletePendingPullRequestReview(ctx, client, params)
18591882
return result, nil, err
18601883
case "resolve_thread":
1861-
result, err := ResolveReviewThread(ctx, client, params.ThreadID, true)
1884+
if !withResolutionReason {
1885+
result, err := ResolveReviewThread(ctx, client, params.ThreadID, true)
1886+
return result, nil, err
1887+
}
1888+
result, err := ResolveReviewThreadWithReason(ctx, client, params.ThreadID, params.ResolutionReason, true)
18621889
return result, nil, err
18631890
case "unresolve_thread":
18641891
result, err := ResolveReviewThread(ctx, client, params.ThreadID, false)
@@ -1867,7 +1894,15 @@ Available methods:
18671894
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", params.Method)), nil, nil
18681895
}
18691896
})
1870-
st.FeatureFlagDisable = []string{FeatureFlagPullRequestsGranular}
1897+
if withResolutionReason {
1898+
st.FeatureFlagEnable = FeatureFlagThreadResolutionReason
1899+
st.FeatureFlagDisable = []string{FeatureFlagPullRequestsGranular}
1900+
} else {
1901+
st.FeatureFlagDisable = []string{FeatureFlagPullRequestsGranular}
1902+
if cfg.hostType != utils.HostTypeGHES {
1903+
st.FeatureFlagDisable = append(st.FeatureFlagDisable, FeatureFlagThreadResolutionReason)
1904+
}
1905+
}
18711906
return st
18721907
}
18731908

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

2132+
// ResolveReviewThreadInput is the GraphQL input for resolving a review thread.
2133+
type ResolveReviewThreadInput struct {
2134+
ThreadID githubv4.ID `json:"threadId"`
2135+
ResolutionReason *githubv4.String `json:"resolutionReason,omitempty"`
2136+
}
2137+
20972138
// ResolveReviewThread resolves or unresolves a PR review thread using GraphQL mutations.
20982139
func ResolveReviewThread(ctx context.Context, client *githubv4.Client, threadID string, resolve bool) (*mcp.CallToolResult, error) {
2140+
return ResolveReviewThreadWithReason(ctx, client, threadID, nil, resolve)
2141+
}
2142+
2143+
// ResolveReviewThreadWithReason resolves or unresolves a PR review thread with an optional resolution reason.
2144+
func ResolveReviewThreadWithReason(ctx context.Context, client *githubv4.Client, threadID string, resolutionReason *string, resolve bool) (*mcp.CallToolResult, error) {
20992145
if threadID == "" {
21002146
return utils.NewToolResultError("threadId is required for resolve_thread and unresolve_thread methods"), nil
21012147
}
@@ -2110,8 +2156,9 @@ func ResolveReviewThread(ctx context.Context, client *githubv4.Client, threadID
21102156
} `graphql:"resolveReviewThread(input: $input)"`
21112157
}
21122158

2113-
input := githubv4.ResolveReviewThreadInput{
2114-
ThreadID: githubv4.ID(threadID),
2159+
input := ResolveReviewThreadInput{
2160+
ThreadID: githubv4.ID(threadID),
2161+
ResolutionReason: newGQLStringlikePtr[githubv4.String](resolutionReason),
21152162
}
21162163

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

0 commit comments

Comments
 (0)