Skip to content

Commit 3a3ae8c

Browse files
committed
Hide write prompt guidance when tools are filtered
1 parent b1575ed commit 3a3ae8c

File tree

7 files changed

+137
-8
lines changed

7 files changed

+137
-8
lines changed

pkg/github/copilot.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ func RequestCopilotReview(t translations.TranslationHelperFunc) inventory.Server
545545
}
546546

547547
func AssignCodingAgentPrompt(t translations.TranslationHelperFunc) inventory.ServerPrompt {
548-
return inventory.NewServerPrompt(
548+
return inventory.NewServerPromptWithRequiredTools(
549549
ToolsetMetadataIssues,
550550
mcp.Prompt{
551551
Name: "AssignCodingAgent",
@@ -603,5 +603,6 @@ func AssignCodingAgentPrompt(t translations.TranslationHelperFunc) inventory.Ser
603603
Messages: messages,
604604
}, nil
605605
},
606+
"assign_copilot_to_issue",
606607
)
607608
}

pkg/github/toolset_instructions.go

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package github
22

3-
import "github.com/github/github-mcp-server/pkg/inventory"
3+
import (
4+
"context"
5+
6+
"github.com/github/github-mcp-server/pkg/inventory"
7+
)
48

59
// Toolset instruction functions - these generate context-aware instructions for each toolset.
610
// They are called during inventory build to generate server instructions.
@@ -9,18 +13,31 @@ func generateContextToolsetInstructions(_ *inventory.Inventory) string {
913
return "Always call 'get_me' first to understand current user permissions and context."
1014
}
1115

12-
func generateIssuesToolsetInstructions(_ *inventory.Inventory) string {
13-
return `## Issues
16+
func generateIssuesToolsetInstructions(inv *inventory.Inventory) string {
17+
instructions := `## Issues
18+
19+
Use 'search_issues' before creating new issues to avoid duplicates.`
20+
if inv.HasAvailableTool(context.Background(), "issue_write") {
21+
instructions += `
1422
15-
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.`
23+
Check 'list_issue_types' first for organizations to use proper issue types. Always set 'state_reason' when closing issues.`
24+
}
25+
return instructions
1626
}
1727

1828
func generatePullRequestsToolsetInstructions(inv *inventory.Inventory) string {
19-
instructions := `## Pull Requests
29+
instructions := ""
30+
31+
if inv.HasAvailableTool(context.Background(), "pull_request_review_write") {
32+
instructions = `## Pull Requests
2033
2134
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.`
35+
}
2236

23-
if inv.HasToolset("repos") {
37+
if inv.HasAvailableTool(context.Background(), "create_pull_request") && inv.HasToolset("repos") {
38+
if instructions == "" {
39+
instructions = "## Pull Requests"
40+
}
2441
instructions += `
2542
2643
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.`

pkg/github/workflow_prompts.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111

1212
// IssueToFixWorkflowPrompt provides a guided workflow for creating an issue and then generating a PR to fix it
1313
func IssueToFixWorkflowPrompt(t translations.TranslationHelperFunc) inventory.ServerPrompt {
14-
return inventory.NewServerPrompt(
14+
return inventory.NewServerPromptWithRequiredTools(
1515
ToolsetMetadataIssues,
1616
mcp.Prompt{
1717
Name: "issue_to_fix_workflow",
@@ -106,5 +106,8 @@ func IssueToFixWorkflowPrompt(t translations.TranslationHelperFunc) inventory.Se
106106
Messages: messages,
107107
}, nil
108108
},
109+
"issue_write",
110+
"assign_copilot_to_issue",
111+
"create_pull_request",
109112
)
110113
}

pkg/inventory/filters.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,17 @@ func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool) bool {
100100
return true
101101
}
102102

103+
// HasAvailableTool reports whether a tool with the given name survives the current filters.
104+
func (r *Inventory) HasAvailableTool(ctx context.Context, toolName string) bool {
105+
for _, tool := range r.filterToolsByName(toolName) {
106+
toolCopy := tool
107+
if r.isToolEnabled(ctx, &toolCopy) {
108+
return true
109+
}
110+
}
111+
return false
112+
}
113+
103114
// AvailableTools returns the tools that pass all current filters,
104115
// sorted deterministically by toolset ID, then tool name.
105116
// The context is used for feature flag evaluation.
@@ -161,6 +172,16 @@ func (r *Inventory) AvailablePrompts(ctx context.Context) []ServerPrompt {
161172
if !r.isFeatureFlagAllowed(ctx, prompt.FeatureFlagEnable, prompt.FeatureFlagDisable) {
162173
continue
163174
}
175+
requiredToolsAvailable := true
176+
for _, toolName := range prompt.RequiredTools {
177+
if !r.HasAvailableTool(ctx, toolName) {
178+
requiredToolsAvailable = false
179+
break
180+
}
181+
}
182+
if !requiredToolsAvailable {
183+
continue
184+
}
164185
if r.isToolsetEnabled(prompt.Toolset.ID) {
165186
result = append(result, *prompt)
166187
}

pkg/inventory/instructions_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package inventory
22

33
import (
4+
"context"
45
"os"
56
"strings"
67
"testing"
8+
9+
"github.com/modelcontextprotocol/go-sdk/mcp"
710
)
811

912
// createTestInventory creates an inventory with the specified toolsets for testing.
@@ -263,3 +266,38 @@ func TestGenerateInstructionsOnlyEnabledToolsets(t *testing.T) {
263266
t.Errorf("Did not expect instructions to contain 'PRS_INSTRUCTIONS' for disabled toolset, but it did. Result: %s", result)
264267
}
265268
}
269+
270+
func TestToolsetInstructionsOmitWriteGuidanceWhenWriteToolsAreFiltered(t *testing.T) {
271+
issuesToolset := ToolsetMetadata{
272+
ID: "issues",
273+
Description: "Issue tools",
274+
InstructionsFunc: func(inv *Inventory) string {
275+
instructions := "Use search_issues before creating new issues."
276+
if inv.HasAvailableTool(context.Background(), "issue_write") {
277+
instructions += " Always set state_reason when closing issues."
278+
}
279+
return instructions
280+
},
281+
}
282+
283+
tools := []ServerTool{
284+
{
285+
Tool: mcp.Tool{Name: "search_issues", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}},
286+
Toolset: issuesToolset,
287+
},
288+
{
289+
Tool: mcp.Tool{Name: "issue_write", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: false}},
290+
Toolset: issuesToolset,
291+
},
292+
}
293+
294+
reg := mustBuild(t, NewBuilder().SetTools(tools).WithToolsets([]string{"all"}).WithServerInstructions())
295+
if !strings.Contains(reg.instructions, "Always set state_reason when closing issues.") {
296+
t.Fatalf("Expected write guidance when issue_write is available, got %q", reg.instructions)
297+
}
298+
299+
readOnly := mustBuild(t, NewBuilder().SetTools(tools).WithToolsets([]string{"all"}).WithReadOnly(true).WithServerInstructions())
300+
if strings.Contains(readOnly.instructions, "Always set state_reason when closing issues.") {
301+
t.Fatalf("Did not expect write guidance in read-only mode, got %q", readOnly.instructions)
302+
}
303+
}

pkg/inventory/prompts.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ type ServerPrompt struct {
1414
// FeatureFlagDisable specifies a feature flag that, when enabled, causes this prompt
1515
// to be omitted. Used to disable prompts when a feature flag is on.
1616
FeatureFlagDisable string
17+
// RequiredTools lists tools that must remain available after filtering for this prompt
18+
// to be exposed. This keeps prompts from advertising capabilities that policy has hidden.
19+
RequiredTools []string
1720
}
1821

1922
// NewServerPrompt creates a new ServerPrompt with toolset metadata.
@@ -24,3 +27,16 @@ func NewServerPrompt(toolset ToolsetMetadata, prompt mcp.Prompt, handler mcp.Pro
2427
Toolset: toolset,
2528
}
2629
}
30+
31+
// NewServerPromptWithRequiredTools creates a new ServerPrompt that is only exposed
32+
// when the given tools remain available after filtering.
33+
func NewServerPromptWithRequiredTools(
34+
toolset ToolsetMetadata,
35+
prompt mcp.Prompt,
36+
handler mcp.PromptHandler,
37+
requiredTools ...string,
38+
) ServerPrompt {
39+
serverPrompt := NewServerPrompt(toolset, prompt, handler)
40+
serverPrompt.RequiredTools = append(serverPrompt.RequiredTools, requiredTools...)
41+
return serverPrompt
42+
}

pkg/inventory/registry_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,6 +1250,39 @@ func TestFeatureFlagPrompts(t *testing.T) {
12501250
}
12511251
}
12521252

1253+
func TestPromptRequiredToolsRespectReadOnlyAndExcludedTools(t *testing.T) {
1254+
tools := []ServerTool{
1255+
mockTool("read_tool", "toolset1", true),
1256+
mockTool("write_tool", "toolset1", false),
1257+
}
1258+
prompts := []ServerPrompt{
1259+
mockPrompt("always_available", "toolset1"),
1260+
{
1261+
Prompt: mcp.Prompt{Name: "needs_write"},
1262+
Toolset: testToolsetMetadata("toolset1"),
1263+
RequiredTools: []string{"write_tool"},
1264+
},
1265+
}
1266+
1267+
reg := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}))
1268+
available := reg.AvailablePrompts(context.Background())
1269+
if len(available) != 2 {
1270+
t.Fatalf("Expected 2 prompts before filtering, got %d", len(available))
1271+
}
1272+
1273+
readOnly := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithReadOnly(true))
1274+
availableReadOnly := readOnly.AvailablePrompts(context.Background())
1275+
if len(availableReadOnly) != 1 || availableReadOnly[0].Prompt.Name != "always_available" {
1276+
t.Fatalf("Expected only always_available in read-only mode, got %#v", availableReadOnly)
1277+
}
1278+
1279+
excluded := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithExcludeTools([]string{"write_tool"}))
1280+
availableExcluded := excluded.AvailablePrompts(context.Background())
1281+
if len(availableExcluded) != 1 || availableExcluded[0].Prompt.Name != "always_available" {
1282+
t.Fatalf("Expected only always_available when write_tool is excluded, got %#v", availableExcluded)
1283+
}
1284+
}
1285+
12531286
func TestServerToolHasHandler(t *testing.T) {
12541287
// Tool with handler
12551288
toolWithHandler := mockTool("has_handler", "toolset1", true)

0 commit comments

Comments
 (0)