Skip to content

Commit f151a6d

Browse files
feat: gate filtered write guidance behind feature flag
Hide write-oriented prompts and toolset instructions only when the new filtered_write_guidance feature flag is enabled. The flag is user-opt-in via --features / X-MCP-Features and auto-on in insiders mode, so default users see no behavior or context change. - Add FeatureFlagFilteredWriteGuidance to allowed + insiders flag lists - Gate the RequiredTools prompt filtering in AvailablePrompts behind it - Restore the original always-on issues/PR instructions when the flag is off - Expose Inventory.IsFeatureEnabled for instruction functions in pkg/github Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2bcfb23 commit f151a6d

7 files changed

Lines changed: 139 additions & 11 deletions

File tree

pkg/errors/error_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@ package errors
33
import (
44
"context"
55
"fmt"
6-
"net/http"
7-
"testing"
8-
"time"
96
"github.com/google/go-github/v87/github"
107
"github.com/modelcontextprotocol/go-sdk/mcp"
118
"github.com/stretchr/testify/assert"
129
"github.com/stretchr/testify/require"
10+
"net/http"
11+
"testing"
12+
"time"
1313
)
1414

1515
func TestGitHubErrorContext(t *testing.T) {
@@ -687,4 +687,3 @@ func TestNewGitHubAPIErrorResponse_RateLimits(t *testing.T) {
687687
assert.Contains(t, text, "validation failed")
688688
})
689689
}
690-

pkg/github/feature_flags.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ const FeatureFlagIFCLabels = "ifc_labels"
1616
// and field_values enrichment in list_issues / search_issues output.
1717
const FeatureFlagIssueFields = "remote_mcp_issue_fields"
1818

19+
// FeatureFlagFilteredWriteGuidance is the feature flag name for hiding
20+
// write-oriented prompts and toolset instructions when the underlying write
21+
// tools have been filtered out (e.g. read-only mode or excluded tools). When
22+
// disabled, write guidance is always advertised regardless of tool availability.
23+
const FeatureFlagFilteredWriteGuidance = "filtered_write_guidance"
24+
1925
// AllowedFeatureFlags is the allowlist of feature flags that can be enabled
2026
// by users via --features CLI flag or X-MCP-Features HTTP header.
2127
// Only flags in this list are accepted; unknown flags are silently ignored.
@@ -27,6 +33,7 @@ var AllowedFeatureFlags = []string{
2733
FeatureFlagIssueFields,
2834
FeatureFlagIssuesGranular,
2935
FeatureFlagPullRequestsGranular,
36+
FeatureFlagFilteredWriteGuidance,
3037
}
3138

3239
// InsidersFeatureFlags is the list of feature flags that insiders mode enables.
@@ -37,6 +44,7 @@ var InsidersFeatureFlags = []string{
3744
MCPAppsFeatureFlag,
3845
FeatureFlagCSVOutput,
3946
FeatureFlagIssueFields,
47+
FeatureFlagFilteredWriteGuidance,
4048
}
4149

4250
// FeatureFlags defines runtime feature toggles that adjust tool behavior.

pkg/github/toolset_instructions.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ func generateContextToolsetInstructions(_ *inventory.Inventory) string {
1414
}
1515

1616
func generateIssuesToolsetInstructions(inv *inventory.Inventory) string {
17+
if !inv.IsFeatureEnabled(context.Background(), FeatureFlagFilteredWriteGuidance) {
18+
return `## Issues
19+
20+
Check 'list_issue_types' first for organizations to use proper issue types. Use 'search_issues' before creating new issues to avoid duplicates. Always set 'state_reason' when closing issues.`
21+
}
22+
1723
instructions := `## Issues
1824
1925
Use 'search_issues' before creating new issues to avoid duplicates.`
@@ -26,6 +32,19 @@ Check 'list_issue_types' first for organizations to use proper issue types. Alwa
2632
}
2733

2834
func generatePullRequestsToolsetInstructions(inv *inventory.Inventory) string {
35+
if !inv.IsFeatureEnabled(context.Background(), FeatureFlagFilteredWriteGuidance) {
36+
instructions := `## Pull Requests
37+
38+
PR review workflow: Always use 'pull_request_review_write' with method 'create' to create a pending review, then 'add_comment_to_pending_review' to add comments, and finally 'pull_request_review_write' with method 'submit_pending' to submit the review for complex reviews with line-specific comments.`
39+
40+
if inv.HasToolset("repos") {
41+
instructions += `
42+
43+
Before creating a pull request, search for pull request templates in the repository. Template files are called pull_request_template.md or they're located in '.github/PULL_REQUEST_TEMPLATE' directory. Use the template content to structure the PR description and then call create_pull_request tool.`
44+
}
45+
return instructions
46+
}
47+
2948
instructions := ""
3049

3150
if inv.HasAvailableTool(context.Background(), "pull_request_review_write") {
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package github
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/github/github-mcp-server/pkg/inventory"
8+
"github.com/modelcontextprotocol/go-sdk/mcp"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
// featureCheckerFor is defined in feature_flags_test.go.
13+
14+
func buildInstructionsInventory(t *testing.T, readOnly bool, checker inventory.FeatureFlagChecker) *inventory.Inventory {
15+
t.Helper()
16+
tools := []inventory.ServerTool{
17+
{Tool: mcp.Tool{Name: "search_issues"}, Toolset: ToolsetMetadataIssues},
18+
{Tool: mcp.Tool{Name: "issue_write"}, Toolset: ToolsetMetadataIssues},
19+
{Tool: mcp.Tool{Name: "pull_request_review_write"}, Toolset: ToolsetMetadataPullRequests},
20+
{Tool: mcp.Tool{Name: "create_pull_request"}, Toolset: ToolsetMetadataRepos},
21+
}
22+
builder := inventory.NewBuilder().
23+
SetTools(tools).
24+
WithToolsets([]string{"issues", "pull_requests", "repos"}).
25+
WithReadOnly(readOnly)
26+
if checker != nil {
27+
builder = builder.WithFeatureChecker(checker)
28+
}
29+
inv, err := builder.Build()
30+
require.NoError(t, err)
31+
return inv
32+
}
33+
34+
func TestToolsetInstructionsFeatureFlagGating(t *testing.T) {
35+
t.Run("flag disabled always advertises write guidance", func(t *testing.T) {
36+
// Even in read-only mode, with the flag off the original (unconditional)
37+
// guidance is returned so default users see no change.
38+
inv := buildInstructionsInventory(t, true, nil)
39+
40+
issues := generateIssuesToolsetInstructions(inv)
41+
require.Contains(t, issues, "Check 'list_issue_types' first")
42+
require.Contains(t, issues, "Always set 'state_reason' when closing issues.")
43+
44+
prs := generatePullRequestsToolsetInstructions(inv)
45+
require.Contains(t, prs, "PR review workflow")
46+
require.Contains(t, prs, "search for pull request templates")
47+
})
48+
49+
t.Run("flag enabled and write tools present keeps full guidance", func(t *testing.T) {
50+
inv := buildInstructionsInventory(t, false, featureCheckerFor(FeatureFlagFilteredWriteGuidance))
51+
52+
issues := generateIssuesToolsetInstructions(inv)
53+
require.Contains(t, issues, "Use 'search_issues' before creating new issues")
54+
require.Contains(t, issues, "Check 'list_issue_types' first")
55+
56+
prs := generatePullRequestsToolsetInstructions(inv)
57+
require.Contains(t, prs, "PR review workflow")
58+
require.Contains(t, prs, "search for pull request templates")
59+
})
60+
61+
t.Run("flag enabled and write tools filtered hides write guidance", func(t *testing.T) {
62+
inv := buildInstructionsInventory(t, true, featureCheckerFor(FeatureFlagFilteredWriteGuidance))
63+
64+
issues := generateIssuesToolsetInstructions(inv)
65+
require.Contains(t, issues, "Use 'search_issues' before creating new issues")
66+
require.NotContains(t, issues, "Check 'list_issue_types' first")
67+
require.NotContains(t, issues, "state_reason")
68+
69+
prs := generatePullRequestsToolsetInstructions(inv)
70+
require.NotContains(t, prs, "PR review workflow")
71+
require.NotContains(t, prs, "search for pull request templates")
72+
require.True(t, strings.TrimSpace(prs) == "", "expected empty PR instructions when write tools filtered, got %q", prs)
73+
})
74+
}

pkg/inventory/builder.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ var (
1919
// The value must match github.MCPAppsFeatureFlag.
2020
const mcpAppsFeatureFlag = "remote_mcp_ui_apps"
2121

22+
// filteredWriteGuidanceFeatureFlag gates hiding write-oriented prompts when the
23+
// tools they depend on have been filtered out. Defined here to avoid importing
24+
// pkg/github (which imports pkg/inventory). The value must match
25+
// github.FeatureFlagFilteredWriteGuidance.
26+
const filteredWriteGuidanceFeatureFlag = "filtered_write_guidance"
27+
2228
// ToolFilter is a function that determines if a tool should be included.
2329
// Returns true if the tool should be included, false to exclude it.
2430
type ToolFilter func(ctx context.Context, tool *ServerTool) (bool, error)

pkg/inventory/filters.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ func (r *Inventory) checkFeatureFlag(ctx context.Context, flagName string) bool
3636
return enabled
3737
}
3838

39+
// IsFeatureEnabled reports whether the named feature flag is enabled for this
40+
// inventory's feature checker. It is the exported entry point used by toolset
41+
// instruction functions (in pkg/github) to gate optional guidance text.
42+
func (r *Inventory) IsFeatureEnabled(ctx context.Context, flagName string) bool {
43+
return r.checkFeatureFlag(ctx, flagName)
44+
}
45+
3946
// featureFlagAllowed reports whether an item with the given enable/disable
4047
// flag pair is permitted under the supplied checker. The checker must be
4148
// non-nil — callers that don't want feature filtering should not call this at
@@ -222,10 +229,12 @@ func (r *Inventory) AvailablePrompts(ctx context.Context) []ServerPrompt {
222229
continue
223230
}
224231
requiredToolsAvailable := true
225-
for _, toolName := range prompt.RequiredTools {
226-
if !r.HasAvailableTool(ctx, toolName) {
227-
requiredToolsAvailable = false
228-
break
232+
if r.checkFeatureFlag(ctx, filteredWriteGuidanceFeatureFlag) {
233+
for _, toolName := range prompt.RequiredTools {
234+
if !r.HasAvailableTool(ctx, toolName) {
235+
requiredToolsAvailable = false
236+
break
237+
}
229238
}
230239
}
231240
if !requiredToolsAvailable {

pkg/inventory/registry_test.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,23 +1224,36 @@ func TestPromptRequiredToolsRespectReadOnlyAndExcludedTools(t *testing.T) {
12241224
},
12251225
}
12261226

1227-
reg := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}))
1227+
// The RequiredTools filtering is gated behind the filtered_write_guidance
1228+
// feature flag, so these cases enable it via a feature checker.
1229+
flagOn := func(_ context.Context, _ string) (bool, error) { return true, nil }
1230+
1231+
reg := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithFeatureChecker(flagOn))
12281232
available := reg.AvailablePrompts(context.Background())
12291233
if len(available) != 2 {
12301234
t.Fatalf("Expected 2 prompts before filtering, got %d", len(available))
12311235
}
12321236

1233-
readOnly := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithReadOnly(true))
1237+
readOnly := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithReadOnly(true).WithFeatureChecker(flagOn))
12341238
availableReadOnly := readOnly.AvailablePrompts(context.Background())
12351239
if len(availableReadOnly) != 1 || availableReadOnly[0].Prompt.Name != "always_available" {
12361240
t.Fatalf("Expected only always_available in read-only mode, got %#v", availableReadOnly)
12371241
}
12381242

1239-
excluded := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithExcludeTools([]string{"write_tool"}))
1243+
excluded := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithExcludeTools([]string{"write_tool"}).WithFeatureChecker(flagOn))
12401244
availableExcluded := excluded.AvailablePrompts(context.Background())
12411245
if len(availableExcluded) != 1 || availableExcluded[0].Prompt.Name != "always_available" {
12421246
t.Fatalf("Expected only always_available when write_tool is excluded, got %#v", availableExcluded)
12431247
}
1248+
1249+
// With the flag disabled, RequiredTools filtering is skipped entirely, so
1250+
// the write-gated prompt is advertised even in read-only mode.
1251+
flagOff := func(_ context.Context, _ string) (bool, error) { return false, nil }
1252+
flagDisabled := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithReadOnly(true).WithFeatureChecker(flagOff))
1253+
availableFlagDisabled := flagDisabled.AvailablePrompts(context.Background())
1254+
if len(availableFlagDisabled) != 2 {
1255+
t.Fatalf("Expected 2 prompts when filtered_write_guidance is disabled, got %#v", availableFlagDisabled)
1256+
}
12441257
}
12451258

12461259
func TestServerToolHasHandler(t *testing.T) {

0 commit comments

Comments
 (0)