Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot use "--deny-tool github" when GitHub is disabled |
- Modified computeCopilotToolArguments to add --deny-tool github for empty allowed arrays, nil, false values - Updated sorting logic to preserve flag types (--allow-tool vs --deny-tool) - Modified applyDefaultTools to respect explicitly empty allowed arrays - Added test case for github: false - Updated tests to expect --deny-tool github for disabled cases Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added When GitHub is disabled (empty allowed array, nil, or false), the Copilot CLI command now includes copilot --deny-tool github --prompt "$INSTRUCTION"The compiler also now respects explicitly empty allowed arrays ( |
|
@copilot remove --deny-tool support since github is only enabled on demand. |
Removed GitHub is now only enabled on demand when explicitly configured with tools. When GitHub is not configured or has an empty allowed array, no arguments are added to the Copilot CLI command. This is the correct behavior since GitHub is a built-in tool that doesn't need to be explicitly denied. |
Reverted all --deny-tool logic since GitHub is only enabled on demand: - Simplified GitHub handling in copilotengine.go to only add --allow-tool for configured tools - Reverted sorting logic to simpler version that assumes all flags are --allow-tool - Reverted compiler changes to always add default GitHub tools - Updated test expectations to not expect --deny-tool Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Removed all The code now only adds |
Added TestCopilotEngineGitHubToolsShellEscaping to explicitly verify that GitHub tool arguments like 'github(add_issue_comment)' are properly wrapped in single quotes by the shellEscapeArg function. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
The GitHub tool arguments are already being single-quoted correctly. The Added test Example from a compiled workflow: copilot --allow-tool 'github(get_issue)' --allow-tool 'github(add_issue_comment)' |
| // Handle GitHub MCP tools - GitHub is built-in to Copilot CLI but still needs tool allowlist | ||
| if githubConfig, hasGithub := tools["github"]; hasGithub { | ||
| if githubConfigMap, ok := githubConfig.(map[string]any); ok { | ||
| // GitHub is built-in, so we don't add --allow-tool github itself |
There was a problem hiding this comment.
@copilot GitHub is not builtin in cli mode (only interactive mode) so you need to treat like any other tool and emit the proper arguments.
There was a problem hiding this comment.
Updated in commit ee6281c to treat GitHub as an MCP server tool in CLI mode.
Now emits both:
--allow-tool github(the MCP server itself)--allow-tool 'github(tool_name)'for each allowed tool
Example generated command:
copilot --allow-tool github --allow-tool 'github(get_issue)' --allow-tool 'github(add_issue_comment)'GitHub is not built-in in CLI mode (only in interactive mode), so it needs to be treated like any other MCP server tool. Updated to: - Remove GitHub from builtInTools list - Add special handling for GitHub in MCP server loop - Emit both --allow-tool github and --allow-tool github(tool_name) for each allowed tool - Updated tests to expect --allow-tool github in addition to individual tool permissions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot only add --allow-tool GitHub when allowed is "*" |
Modified GitHub handling to only emit --allow-tool github (server-level permission) when the allowed array contains the "*" wildcard. When specific tools are listed, only emit --allow-tool github(tool_name) for each tool. This allows more granular control - users can specify just the tools they need without enabling the entire GitHub MCP server. Updated tests to: - Remove --allow-tool github from tests with specific tools - Add new test cases for wildcard behavior - Add test for wildcard combined with specific tools Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Updated in commit 565ca88 to only add When specific tools are listed, only the individual tool permissions are emitted: copilot --allow-tool 'github(get_issue)' --allow-tool 'github(add_issue_comment)'When wildcard is used: copilot --allow-tool github |
|
@copilot review the wildcard treatment and make sure you are not tearing it twice. Simplify, add test cases |
Refactored GitHub tool handling to process the allowed array only once instead of twice: - Single loop processes both wildcard detection and individual tool permissions - Wildcard "*" adds server-level permission (--allow-tool github) - Other tools add individual permissions (--allow-tool github(tool_name)) - More efficient and easier to understand All existing tests pass without changes. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Simplified in commit 488ddd3 to process the allowed list in a single pass instead of iterating twice. The refactored code:
|
Fix Copilot Engine GitHub Tool Allow-List Generation
The Copilot agentic engine was not generating
--allow-toolarguments for GitHub MCP tools correctly. This caused GitHub API operations to fail during workflow execution.Problem Analysis
--allow-tool githubwhen using wildcard"*"--allow-tool github(tool_name)builtInToolsmapapplyDefaultToolsand need to be converted to--allow-toolargumentsSolution
Modified
computeCopilotToolArgumentsinpkg/workflow/copilot_engine.goto handle GitHub as an MCP server tool with conditional server-level permission:--allow-tool githubwhenallowedcontains"*"wildcard--allow-tool github(tool_name)for each specific tool (excluding wildcard)shellEscapeArgfunctionBefore
copilot --add-dir /tmp/ --log-level all --prompt "$INSTRUCTION"After (with specific tools)
After (with wildcard)
copilot --add-dir /tmp/ --log-level all \ --allow-tool github \ --prompt "$INSTRUCTION"After (GitHub not configured)
copilot --add-dir /tmp/ --log-level all \ --prompt "$INSTRUCTION"Testing
--allow-tool githubwhen not using wildcardChanges
pkg/workflow/copilot_engine.go: Simplified wildcard handling with single-pass processing, only emit server-level permission with wildcardpkg/workflow/copilot_engine_test.go: Updated comprehensive test cases including wildcard scenariospkg/workflow/shell.go: Existing shell escaping logic handles parentheses correctlyOriginal prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.