Ensure allowed tool filter for mcp-servers section#19801
Conversation
… add compilation test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot do not ignore tool allowed list, pass it along remove warnings |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 98d2568. The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready. |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Agent Container Tool Check
Result: 11/12 tools available Overall Status: FAIL —
|
|
Commit pushed:
|
Smoke Test Results (Run §22754642699)
Overall: CC
|
There was a problem hiding this comment.
This PR correctly fixes the allowed tool filter for mcp-servers so it applies to all engines (not just Copilot). The logic change is minimal and well-targeted. Test coverage is thorough with cases for both Claude and Copilot engines, including positive and negative assertions.
📰 BREAKING: Report filed by Smoke Copilot
| if renderer.RequiresCopilotFields || len(mcpConfig.Allowed) > 0 { | ||
| existingProperties = append(existingProperties, prop) | ||
| } | ||
| case "container": |
There was a problem hiding this comment.
The condition renderer.RequiresCopilotFields || len(mcpConfig.Allowed) > 0 correctly extends tool filtering support beyond just Copilot. This ensures that when allowed is explicitly specified in mcp-servers, the filter is passed to the MCP gateway for all engines. Good fix! 🎯
|
|
||
| // TestDevModeAgenticWorkflowsContainer verifies that the agentic-workflows MCP server | ||
| // uses the locally built Docker image in dev mode instead of alpine:latest | ||
| func TestDevModeAgenticWorkflowsContainer(t *testing.T) { |
There was a problem hiding this comment.
Comprehensive test coverage with 5 test cases covering HTTP, stdio, no-allowed-field defaults, and both Claude and Copilot engines. The test for the Claude case without allowed verifying absence of "tools": is a nice negative assertion. Well-structured! ✅
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Smoke Test Results - Run §22754642684
Overall: PARTIAL (2 skipped, 0 failed)
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude
There was a problem hiding this comment.
Pull request overview
Ensures mcp-servers.allowed is propagated into the compiled MCP gateway config as the "tools" filter for all engines (not only Copilot), and adds coverage to prevent regressions.
Changes:
- Always includes
"tools"in HTTP JSON property ordering and conditionally renders it when Copilot fields are required or an explicitallowedlist is present. - Adds an end-to-end compilation test validating
mcp-servers.allowed→"tools"output across several Copilot and Claude scenarios. - Adds a changeset to ship the fix as a patch release.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/workflow/mcp_config_custom.go | Updates MCP server JSON rendering to propagate allowed tool filters via the "tools" field beyond Copilot-only output. |
| pkg/workflow/mcp_config_compilation_test.go | Adds an end-to-end compilation test verifying allowed tool filter rendering behavior. |
| .changeset/patch-propagate-mcp-allowed-filter.md | Declares a patch release for the MCP allowed/tools propagation fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // JSON format - include tools field for MCP gateway tool filtering (all engines) | ||
| // For HTTP MCP with secrets in headers, env passthrough is needed | ||
| if len(headerSecrets) > 0 { | ||
| propertyOrder = []string{"type", "url", "headers", "tools", "env"} | ||
| } else { |
There was a problem hiding this comment.
The comment says the JSON format includes the tools field for MCP gateway tool filtering “(all engines)”, but the tools field is still conditionally rendered later (only when RequiresCopilotFields is true or when Allowed is non-empty). Consider rewording this comment to reflect the actual behavior.
| name: "claude - http mcp server with specific allowed tools passes through", | ||
| workflowContent: `--- | ||
| on: | ||
| workflow_dispatch: | ||
| strict: false |
There was a problem hiding this comment.
The scenarios here cover allowed tool compilation for Copilot (http/stdio) and Claude (http), but there isn’t a case for a non-Copilot stdio MCP server with an explicit allowed list. Adding a Claude/Gemini/Codex stdio case would better lock in the behavior from the updated tools-field inclusion gate.
Summary
Ensures the
allowedtool filter inmcp-serversis passed through to the compiled MCP config output for all engines, not just Copilot.Changes
pkg/workflow/mcp_config_custom.go— The"tools"field (which carries theallowedlist) was previously only rendered whenRequiresCopilotFieldswas true (Copilot engine only). Fixed in two places:"tools"is now always included in the JSON format property order, not Copilot-only"tools"is now included wheneverRequiresCopilotFieldsis set or whenallowedtools are explicitly specified — so Claude, Codex, and Gemini engines now correctly pass the filter through to the MCP gateway configpkg/workflow/mcp_config_compilation_test.go— AddedTestMCPServersAllowedToolFilterCompilationto verify end-to-end that theallowedfield inmcp-serversentries compiles correctly into the"tools"field in the output. Covers five scenarios across Copilot and Claude engines:"tools": ["get_data", "list_items"]"tools": ["run_query", "fetch_results"]allowedfield → defaults to"tools": ["*"]allowedlist is passed throughallowedfield → notoolsfield rendered (no wildcard added for non-Copilot engines)Security Summary
No security vulnerabilities introduced or discovered. The changes ensure the MCP gateway tool filter is correctly propagated for all engines.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
✨ PR Review Safe Output Test - Run 22754642684