Repository navigation
Remove thread_resolution_reason feature flag - #3727
Open
cagesellchen wants to merge 5 commits into
Open
cagesellchen wants to merge 5 commits into
cagesellchen wants to merge 5 commits into
Conversation
The flag is globally enabled, so resolution reasons are now always available on pull_request_review_write and resolve_review_thread. GHES still omits the resolutionReason input because it does not support it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the change to enable resolution reasons by default.
No open issues 🎉
What changed in this PR
Removes the globally enabled thread_resolution_reason flag from the GitHub MCP Server. Both review-thread tools now expose resolutionReason by default, while continuing to omit it on GitHub Enterprise Server (GHES).
Changes:
- Replaces flag-gated variants with single host-aware constructors and simplifies registration.
- Updates tests to cover default resolution-reason support and GHES omission.
- Refreshes documentation and base schema snapshots, removing obsolete variant snapshots.
| File | Description |
|---|---|
| README.md | Documents the default resolutionReason parameter. |
| pkg/github/typed_granular_pull_request_outputs_test.go | Adapts granular schema and output tests to host-based behavior. |
| pkg/github/typed_consolidated_pull_request_outputs_test.go | Removes duplicate variant setup from consolidated tests. |
| pkg/github/tools.go | Registers one host-aware constructor per affected tool. |
| pkg/github/pullrequests.go | Enables consolidated resolution reasons except on GHES. |
| pkg/github/pullrequests_test.go | Tests default reason forwarding and GHES omission. |
| pkg/github/pullrequests_granular.go | Enables granular resolution reasons except on GHES. |
| pkg/github/granular_tools_test.go | Updates granular handler tests for host-based behavior. |
| pkg/github/feature_flags.go | Removes the flag constant and allowlist entry. |
| pkg/github/feature_flags_test.go | Replaces flag-on/off cases with default and GHES cases. |
| pkg/github/__toolsnaps__/resolve_review_thread.snap | Adds the default resolution-reason input. |
| pkg/github/__toolsnaps__/resolve_review_thread_resolution_reason.snap | Deletes the obsolete granular variant snapshot. |
| pkg/github/__toolsnaps__/pull_request_review_write.snap | Adds the default resolution-reason input. |
| pkg/github/__toolsnaps__/pull_request_review_write_resolution_reason.snap | Deletes the obsolete consolidated variant snapshot. |
| docs/feature-flags.md | Removes the retired flag section and updates granular parameters. |
Review effort: Balanced (auto)
obvioussean
reviewed
Nov 6, 2026
The resolveReviewThread mutation expects ADDRESSED, WONT_FIX or INVALID, but the tools forwarded the lowercase input values unchanged. Map them to enum names and treat an empty reason as none. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
obvioussean
previously approved these changes
Nov 7, 2026
scotttesler
reviewed
Nov 9, 2026
scotttesler
left a comment
scotttesler
left a comment
There was a problem hiding this comment.
The note above the review-write tool's input still says the resolution reason field belongs only to a separate flag-gated version of the tool. That version is gone, and the single tool now decides by host instead. I'd delete those two lines rather than reword them, since the host rule already lives with the tool.
The PR body says "the flag-on behavior is unchanged" and that "this only removes flag gating." The new reason mapping changes what gets sent to GitHub when a reason is given, so those two lines could use an update.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
@scotttesler thanks for the review.
|
obvioussean
previously approved these changes
Nov 10, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
obvioussean
previously approved these changes
Nov 10, 2026
Empty reasons are still accepted and treated as none. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
obvioussean
approved these changes
Nov 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: https://github.com/github/copilot-voyager/issues/1145
Closes: https://github.com/github/copilot-voyager/issues/1154
Summary
Removes the globally enabled
thread_resolution_reasonfeature flag.pull_request_review_writeandresolve_review_threadnow always acceptresolutionReason. GHES is the exception and still omits it.Why
Fixes https://github.com/github/copilot-voyager/issues/1154 (part of https://github.com/github/copilot-voyager/issues/1145)
The flag is globally enabled on github.com. Deploy ordering: this PR must deploy before the github/github PR that deletes the devportal flag. Until then, the remote server picks the tool variant based on that flag, so the resolution-reason variant would disappear once the flag is deleted.
What changed
FeatureFlagThreadResolutionReasonand its entry in theAllowedFeatureFlagsallowlist.PullRequestReviewWrite(t, opts...)andGranularResolveReviewThread(t, opts...)are now single host-aware constructors.resolutionReasonunless the host is GHES.PullRequestReviewWriteWithResolutionReasonandGranularResolveReviewThreadWithResolutionReason. They registered the duplicate flag-gated variants.*_resolution_reason.snaptoolsnaps with the base snaps, which are now byte-identical to the old reason variants.ResolveReviewThreadWithReasonnow sends resolution reasons as GraphQL enum names (addressedbecomesADDRESSED,wont-fixbecomesWONT_FIX,invalidbecomesINVALID) and treats""as no reason. Before this, the API rejected the lowercase values.MCP impact
resolutionReasonis now always present onpull_request_review_writeandresolve_review_thread. Previously it only appeared when the flag was on, which is globally the case today.Prompts tested (tool changes only)
Security / limits
Tool renaming
deprecated_tool_aliases.goNote: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.
Lint & tests
./script/lint: passed, 0 issues../script/test: passed.Docs
script/generate-docs.