From 3a3ae8c4d0e11937a909340b4a0a9eafc07f71fc Mon Sep 17 00:00:00 2001 From: David Ahmann Date: Tue, 17 Mar 2026 08:25:09 -0400 Subject: [PATCH] Hide write prompt guidance when tools are filtered --- pkg/github/copilot.go | 3 ++- pkg/github/toolset_instructions.go | 29 ++++++++++++++++++----- pkg/github/workflow_prompts.go | 5 +++- pkg/inventory/filters.go | 21 +++++++++++++++++ pkg/inventory/instructions_test.go | 38 ++++++++++++++++++++++++++++++ pkg/inventory/prompts.go | 16 +++++++++++++ pkg/inventory/registry_test.go | 33 ++++++++++++++++++++++++++ 7 files changed, 137 insertions(+), 8 deletions(-) diff --git a/pkg/github/copilot.go b/pkg/github/copilot.go index d95357e73..e3bd3d2e2 100644 --- a/pkg/github/copilot.go +++ b/pkg/github/copilot.go @@ -545,7 +545,7 @@ func RequestCopilotReview(t translations.TranslationHelperFunc) inventory.Server } func AssignCodingAgentPrompt(t translations.TranslationHelperFunc) inventory.ServerPrompt { - return inventory.NewServerPrompt( + return inventory.NewServerPromptWithRequiredTools( ToolsetMetadataIssues, mcp.Prompt{ Name: "AssignCodingAgent", @@ -603,5 +603,6 @@ func AssignCodingAgentPrompt(t translations.TranslationHelperFunc) inventory.Ser Messages: messages, }, nil }, + "assign_copilot_to_issue", ) } diff --git a/pkg/github/toolset_instructions.go b/pkg/github/toolset_instructions.go index bc9da4e65..5f932c1d7 100644 --- a/pkg/github/toolset_instructions.go +++ b/pkg/github/toolset_instructions.go @@ -1,6 +1,10 @@ package github -import "github.com/github/github-mcp-server/pkg/inventory" +import ( + "context" + + "github.com/github/github-mcp-server/pkg/inventory" +) // Toolset instruction functions - these generate context-aware instructions for each toolset. // They are called during inventory build to generate server instructions. @@ -9,18 +13,31 @@ func generateContextToolsetInstructions(_ *inventory.Inventory) string { return "Always call 'get_me' first to understand current user permissions and context." } -func generateIssuesToolsetInstructions(_ *inventory.Inventory) string { - return `## Issues +func generateIssuesToolsetInstructions(inv *inventory.Inventory) string { + instructions := `## Issues + +Use 'search_issues' before creating new issues to avoid duplicates.` + if inv.HasAvailableTool(context.Background(), "issue_write") { + instructions += ` -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.` +Check 'list_issue_types' first for organizations to use proper issue types. Always set 'state_reason' when closing issues.` + } + return instructions } func generatePullRequestsToolsetInstructions(inv *inventory.Inventory) string { - instructions := `## Pull Requests + instructions := "" + + if inv.HasAvailableTool(context.Background(), "pull_request_review_write") { + instructions = `## Pull Requests 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.` + } - if inv.HasToolset("repos") { + if inv.HasAvailableTool(context.Background(), "create_pull_request") && inv.HasToolset("repos") { + if instructions == "" { + instructions = "## Pull Requests" + } instructions += ` 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.` diff --git a/pkg/github/workflow_prompts.go b/pkg/github/workflow_prompts.go index e85c93348..aa00db696 100644 --- a/pkg/github/workflow_prompts.go +++ b/pkg/github/workflow_prompts.go @@ -11,7 +11,7 @@ import ( // IssueToFixWorkflowPrompt provides a guided workflow for creating an issue and then generating a PR to fix it func IssueToFixWorkflowPrompt(t translations.TranslationHelperFunc) inventory.ServerPrompt { - return inventory.NewServerPrompt( + return inventory.NewServerPromptWithRequiredTools( ToolsetMetadataIssues, mcp.Prompt{ Name: "issue_to_fix_workflow", @@ -106,5 +106,8 @@ func IssueToFixWorkflowPrompt(t translations.TranslationHelperFunc) inventory.Se Messages: messages, }, nil }, + "issue_write", + "assign_copilot_to_issue", + "create_pull_request", ) } diff --git a/pkg/inventory/filters.go b/pkg/inventory/filters.go index 707457853..88637111a 100644 --- a/pkg/inventory/filters.go +++ b/pkg/inventory/filters.go @@ -100,6 +100,17 @@ func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool) bool { return true } +// HasAvailableTool reports whether a tool with the given name survives the current filters. +func (r *Inventory) HasAvailableTool(ctx context.Context, toolName string) bool { + for _, tool := range r.filterToolsByName(toolName) { + toolCopy := tool + if r.isToolEnabled(ctx, &toolCopy) { + return true + } + } + return false +} + // AvailableTools returns the tools that pass all current filters, // sorted deterministically by toolset ID, then tool name. // The context is used for feature flag evaluation. @@ -161,6 +172,16 @@ func (r *Inventory) AvailablePrompts(ctx context.Context) []ServerPrompt { if !r.isFeatureFlagAllowed(ctx, prompt.FeatureFlagEnable, prompt.FeatureFlagDisable) { continue } + requiredToolsAvailable := true + for _, toolName := range prompt.RequiredTools { + if !r.HasAvailableTool(ctx, toolName) { + requiredToolsAvailable = false + break + } + } + if !requiredToolsAvailable { + continue + } if r.isToolsetEnabled(prompt.Toolset.ID) { result = append(result, *prompt) } diff --git a/pkg/inventory/instructions_test.go b/pkg/inventory/instructions_test.go index e8e369b3d..75fad9b5f 100644 --- a/pkg/inventory/instructions_test.go +++ b/pkg/inventory/instructions_test.go @@ -1,9 +1,12 @@ package inventory import ( + "context" "os" "strings" "testing" + + "github.com/modelcontextprotocol/go-sdk/mcp" ) // createTestInventory creates an inventory with the specified toolsets for testing. @@ -263,3 +266,38 @@ func TestGenerateInstructionsOnlyEnabledToolsets(t *testing.T) { t.Errorf("Did not expect instructions to contain 'PRS_INSTRUCTIONS' for disabled toolset, but it did. Result: %s", result) } } + +func TestToolsetInstructionsOmitWriteGuidanceWhenWriteToolsAreFiltered(t *testing.T) { + issuesToolset := ToolsetMetadata{ + ID: "issues", + Description: "Issue tools", + InstructionsFunc: func(inv *Inventory) string { + instructions := "Use search_issues before creating new issues." + if inv.HasAvailableTool(context.Background(), "issue_write") { + instructions += " Always set state_reason when closing issues." + } + return instructions + }, + } + + tools := []ServerTool{ + { + Tool: mcp.Tool{Name: "search_issues", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true}}, + Toolset: issuesToolset, + }, + { + Tool: mcp.Tool{Name: "issue_write", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: false}}, + Toolset: issuesToolset, + }, + } + + reg := mustBuild(t, NewBuilder().SetTools(tools).WithToolsets([]string{"all"}).WithServerInstructions()) + if !strings.Contains(reg.instructions, "Always set state_reason when closing issues.") { + t.Fatalf("Expected write guidance when issue_write is available, got %q", reg.instructions) + } + + readOnly := mustBuild(t, NewBuilder().SetTools(tools).WithToolsets([]string{"all"}).WithReadOnly(true).WithServerInstructions()) + if strings.Contains(readOnly.instructions, "Always set state_reason when closing issues.") { + t.Fatalf("Did not expect write guidance in read-only mode, got %q", readOnly.instructions) + } +} diff --git a/pkg/inventory/prompts.go b/pkg/inventory/prompts.go index 648f20f9c..e3ca11abd 100644 --- a/pkg/inventory/prompts.go +++ b/pkg/inventory/prompts.go @@ -14,6 +14,9 @@ type ServerPrompt struct { // FeatureFlagDisable specifies a feature flag that, when enabled, causes this prompt // to be omitted. Used to disable prompts when a feature flag is on. FeatureFlagDisable string + // RequiredTools lists tools that must remain available after filtering for this prompt + // to be exposed. This keeps prompts from advertising capabilities that policy has hidden. + RequiredTools []string } // NewServerPrompt creates a new ServerPrompt with toolset metadata. @@ -24,3 +27,16 @@ func NewServerPrompt(toolset ToolsetMetadata, prompt mcp.Prompt, handler mcp.Pro Toolset: toolset, } } + +// NewServerPromptWithRequiredTools creates a new ServerPrompt that is only exposed +// when the given tools remain available after filtering. +func NewServerPromptWithRequiredTools( + toolset ToolsetMetadata, + prompt mcp.Prompt, + handler mcp.PromptHandler, + requiredTools ...string, +) ServerPrompt { + serverPrompt := NewServerPrompt(toolset, prompt, handler) + serverPrompt.RequiredTools = append(serverPrompt.RequiredTools, requiredTools...) + return serverPrompt +} diff --git a/pkg/inventory/registry_test.go b/pkg/inventory/registry_test.go index 207e65dba..8d40fbdd1 100644 --- a/pkg/inventory/registry_test.go +++ b/pkg/inventory/registry_test.go @@ -1250,6 +1250,39 @@ func TestFeatureFlagPrompts(t *testing.T) { } } +func TestPromptRequiredToolsRespectReadOnlyAndExcludedTools(t *testing.T) { + tools := []ServerTool{ + mockTool("read_tool", "toolset1", true), + mockTool("write_tool", "toolset1", false), + } + prompts := []ServerPrompt{ + mockPrompt("always_available", "toolset1"), + { + Prompt: mcp.Prompt{Name: "needs_write"}, + Toolset: testToolsetMetadata("toolset1"), + RequiredTools: []string{"write_tool"}, + }, + } + + reg := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"})) + available := reg.AvailablePrompts(context.Background()) + if len(available) != 2 { + t.Fatalf("Expected 2 prompts before filtering, got %d", len(available)) + } + + readOnly := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithReadOnly(true)) + availableReadOnly := readOnly.AvailablePrompts(context.Background()) + if len(availableReadOnly) != 1 || availableReadOnly[0].Prompt.Name != "always_available" { + t.Fatalf("Expected only always_available in read-only mode, got %#v", availableReadOnly) + } + + excluded := mustBuild(t, NewBuilder().SetTools(tools).SetPrompts(prompts).WithToolsets([]string{"all"}).WithExcludeTools([]string{"write_tool"})) + availableExcluded := excluded.AvailablePrompts(context.Background()) + if len(availableExcluded) != 1 || availableExcluded[0].Prompt.Name != "always_available" { + t.Fatalf("Expected only always_available when write_tool is excluded, got %#v", availableExcluded) + } +} + func TestServerToolHasHandler(t *testing.T) { // Tool with handler toolWithHandler := mockTool("has_handler", "toolset1", true)