diff --git a/.github/workflows/daily-model-inventory.lock.yml b/.github/workflows/daily-model-inventory.lock.yml index 44b63278c67..6efe8711421 100644 --- a/.github/workflows/daily-model-inventory.lock.yml +++ b/.github/workflows/daily-model-inventory.lock.yml @@ -763,6 +763,7 @@ jobs: # --allow-tool shell(jq . /tmp/gh-aw/model-inventory/inventory.json) # --allow-tool shell(jq) # --allow-tool shell(ls) + # --allow-tool shell(playwright-cli:*) # --allow-tool shell(pwd) # --allow-tool shell(safeoutputs:*) # --allow-tool shell(sort) @@ -781,7 +782,7 @@ jobs: printf '%s\n' '{"$schema":"https://github.com/github/gh-aw-firewall/releases/download/v0.25.35/awf-config.schema.json","network":{"allowDomains":["api.business.githubcopilot.com","api.enterprise.githubcopilot.com","api.github.com","api.githubcopilot.com","api.individual.githubcopilot.com","api.snapcraft.io","archive.ubuntu.com","azure.archive.ubuntu.com","cdn.playwright.dev","crl.geotrust.com","crl.globalsign.com","crl.identrust.com","crl.sectigo.com","crl.thawte.com","crl.usertrust.com","crl.verisign.com","crl3.digicert.com","crl4.digicert.com","crls.ssl.com","github.com","host.docker.internal","json-schema.org","json.schemastore.org","keyserver.ubuntu.com","ocsp.digicert.com","ocsp.geotrust.com","ocsp.globalsign.com","ocsp.identrust.com","ocsp.sectigo.com","ocsp.ssl.com","ocsp.thawte.com","ocsp.usertrust.com","ocsp.verisign.com","packagecloud.io","packages.cloud.google.com","packages.microsoft.com","playwright.download.prss.microsoft.com","ppa.launchpad.net","raw.githubusercontent.com","registry.npmjs.org","s.symcb.com","s.symcd.com","security.ubuntu.com","telemetry.enterprise.githubcopilot.com","ts-crl.ws.symantec.com","ts-ocsp.ws.symantec.com","www.googleapis.com"]},"apiProxy":{"enabled":true},"container":{"imageTag":"0.25.35"}}' > "${RUNNER_TEMP}/gh-aw/awf-config.json" && cp "${RUNNER_TEMP}/gh-aw/awf-config.json" /tmp/gh-aw/awf-config.json # shellcheck disable=SC1003 sudo -E awf --config "${RUNNER_TEMP}/gh-aw/awf-config.json" --container-workdir "${GITHUB_WORKSPACE}" --mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro" --mount "${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro" --env-all --exclude-env COPILOT_GITHUB_TOKEN --exclude-env GITHUB_MCP_SERVER_TOKEN --exclude-env MCP_GATEWAY_API_KEY --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs --audit-dir /tmp/gh-aw/sandbox/firewall/audit --enable-host-access --allow-host-ports 80,443,8080 --skip-pull \ - -- /bin/bash -c 'export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH" && export PATH="$(find /opt/hostedtoolcache /home/runner/work/_tool -maxdepth 4 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true && GH_AW_NODE_EXEC="${GH_AW_NODE_BIN:-}"; if [ -z "$GH_AW_NODE_EXEC" ] || [ ! -x "$GH_AW_NODE_EXEC" ]; then GH_AW_NODE_EXEC="$(command -v node 2>/dev/null || echo node)"; fi; "$GH_AW_NODE_EXEC" ${RUNNER_TEMP}/gh-aw/actions/copilot_harness.cjs /usr/local/bin/copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/sandbox/agent/logs/ --disable-builtin-mcps --no-ask-user --allow-tool github --allow-tool safeoutputs --allow-tool '\''shell(cat /tmp/gh-aw/model-inventory/artifacts/copilot-billing-multipliers/multipliers.json)'\'' --allow-tool '\''shell(cat /tmp/gh-aw/model-inventory/inventory.json)'\'' --allow-tool '\''shell(cat pkg/cli/data/model_multipliers.json)'\'' --allow-tool '\''shell(cat pkg/workflow/model_aliases.go)'\'' --allow-tool '\''shell(cat)'\'' --allow-tool '\''shell(date)'\'' --allow-tool '\''shell(echo)'\'' --allow-tool '\''shell(find /tmp/gh-aw/model-inventory -type f)'\'' --allow-tool '\''shell(grep)'\'' --allow-tool '\''shell(head)'\'' --allow-tool '\''shell(jq . /tmp/gh-aw/model-inventory/artifacts/*/models.json)'\'' --allow-tool '\''shell(jq . /tmp/gh-aw/model-inventory/artifacts/*/raw.json)'\'' --allow-tool '\''shell(jq . /tmp/gh-aw/model-inventory/artifacts/copilot-billing-multipliers/multipliers.json)'\'' --allow-tool '\''shell(jq . /tmp/gh-aw/model-inventory/inventory.json)'\'' --allow-tool '\''shell(jq)'\'' --allow-tool '\''shell(ls)'\'' --allow-tool '\''shell(pwd)'\'' --allow-tool '\''shell(safeoutputs:*)'\'' --allow-tool '\''shell(sort)'\'' --allow-tool '\''shell(tail)'\'' --allow-tool '\''shell(uniq)'\'' --allow-tool '\''shell(wc)'\'' --allow-tool '\''shell(yq)'\'' --allow-tool write --allow-all-paths --add-dir "${GITHUB_WORKSPACE}" --prompt-file /tmp/gh-aw/aw-prompts/prompt.txt' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log + -- /bin/bash -c 'export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH" && export PATH="$(find /opt/hostedtoolcache /home/runner/work/_tool -maxdepth 4 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true && GH_AW_NODE_EXEC="${GH_AW_NODE_BIN:-}"; if [ -z "$GH_AW_NODE_EXEC" ] || [ ! -x "$GH_AW_NODE_EXEC" ]; then GH_AW_NODE_EXEC="$(command -v node 2>/dev/null || echo node)"; fi; "$GH_AW_NODE_EXEC" ${RUNNER_TEMP}/gh-aw/actions/copilot_harness.cjs /usr/local/bin/copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/sandbox/agent/logs/ --disable-builtin-mcps --no-ask-user --allow-tool github --allow-tool safeoutputs --allow-tool '\''shell(cat /tmp/gh-aw/model-inventory/artifacts/copilot-billing-multipliers/multipliers.json)'\'' --allow-tool '\''shell(cat /tmp/gh-aw/model-inventory/inventory.json)'\'' --allow-tool '\''shell(cat pkg/cli/data/model_multipliers.json)'\'' --allow-tool '\''shell(cat pkg/workflow/model_aliases.go)'\'' --allow-tool '\''shell(cat)'\'' --allow-tool '\''shell(date)'\'' --allow-tool '\''shell(echo)'\'' --allow-tool '\''shell(find /tmp/gh-aw/model-inventory -type f)'\'' --allow-tool '\''shell(grep)'\'' --allow-tool '\''shell(head)'\'' --allow-tool '\''shell(jq . /tmp/gh-aw/model-inventory/artifacts/*/models.json)'\'' --allow-tool '\''shell(jq . /tmp/gh-aw/model-inventory/artifacts/*/raw.json)'\'' --allow-tool '\''shell(jq . /tmp/gh-aw/model-inventory/artifacts/copilot-billing-multipliers/multipliers.json)'\'' --allow-tool '\''shell(jq . /tmp/gh-aw/model-inventory/inventory.json)'\'' --allow-tool '\''shell(jq)'\'' --allow-tool '\''shell(ls)'\'' --allow-tool '\''shell(playwright-cli:*)'\'' --allow-tool '\''shell(pwd)'\'' --allow-tool '\''shell(safeoutputs:*)'\'' --allow-tool '\''shell(sort)'\'' --allow-tool '\''shell(tail)'\'' --allow-tool '\''shell(uniq)'\'' --allow-tool '\''shell(wc)'\'' --allow-tool '\''shell(yq)'\'' --allow-tool write --allow-all-paths --add-dir "${GITHUB_WORKSPACE}" --prompt-file /tmp/gh-aw/aw-prompts/prompt.txt' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log env: COPILOT_AGENT_RUNNER_TYPE: STANDALONE COPILOT_API_KEY: dummy-byok-key-for-offline-mode diff --git a/cmd/gh-aw/argument_syntax_test.go b/cmd/gh-aw/argument_syntax_test.go index 7cefced16b1..2e2baa424cc 100644 --- a/cmd/gh-aw/argument_syntax_test.go +++ b/cmd/gh-aw/argument_syntax_test.go @@ -172,9 +172,9 @@ func TestMCPSubcommandArgumentSyntax(t *testing.T) { expectedUse: "add [workflow] [server]", }, { - name: "mcp list-tools requires server with optional workflow", + name: "mcp list-tools has optional workflow", subcommand: "list-tools", - expectedUse: "list-tools [workflow]", + expectedUse: "list-tools [workflow]", }, } diff --git a/pkg/cli/completions_integration_test.go b/pkg/cli/completions_integration_test.go index ca40e6fe5c7..5cc01fc81b9 100644 --- a/pkg/cli/completions_integration_test.go +++ b/pkg/cli/completions_integration_test.go @@ -105,22 +105,37 @@ func TestCompletionMCPListToolsIntegration(t *testing.T) { setup := setupIntegrationTest(t) defer setup.cleanup() - // Test completion for mcp list-tools first argument (common server names) + // Create a test workflow file so there's something to complete + workflowPath := filepath.Join(setup.workflowsDir, "test-mcp.md") + content := `--- +name: Test MCP Workflow +on: + workflow_dispatch: +permissions: + contents: read +engine: copilot +--- + +# Test MCP Workflow +` + if err := os.WriteFile(workflowPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write test workflow file: %v", err) + } + + // Test completion for mcp list-tools first argument (workflow names) cmd := exec.Command(setup.binaryPath, "__complete", "mcp", "list-tools", "") output, err := cmd.CombinedOutput() require.NoError(t, err, "CLI __complete command for mcp list-tools failed: %s", string(output)) outputStr := string(output) - // Should contain common MCP server names - assert.Contains(t, outputStr, "github") - assert.Contains(t, outputStr, "playwright") - assert.Contains(t, outputStr, "tavily") - assert.Contains(t, outputStr, "safe-outputs") + // Should contain workflow name (without .md extension) + assert.Contains(t, outputStr, "test-mcp") } -// TestCompletionMCPListToolsWorkflowIntegration tests the MCP list-tools second argument completion -func TestCompletionMCPListToolsWorkflowIntegration(t *testing.T) { +// TestCompletionMCPListToolsNoSecondPositionalArgIntegration verifies that mcp list-tools +// does not offer completions for a second positional argument (only [workflow] is accepted). +func TestCompletionMCPListToolsNoSecondPositionalArgIntegration(t *testing.T) { setup := setupIntegrationTest(t) defer setup.cleanup() @@ -141,15 +156,16 @@ engine: copilot t.Fatalf("Failed to write test workflow file: %v", err) } - // Test completion for mcp list-tools second argument (workflow names after server name) - cmd := exec.Command(setup.binaryPath, "__complete", "mcp", "list-tools", "github", "") + // Test completion for mcp list-tools with workflow argument (only 1 positional arg allowed now) + cmd := exec.Command(setup.binaryPath, "__complete", "mcp", "list-tools", "test-mcp", "") output, err := cmd.CombinedOutput() + // With only 1 positional arg allowed, second arg should show no completions require.NoError(t, err, "CLI __complete command for mcp list-tools workflow failed: %s", string(output)) outputStr := string(output) - // Should contain workflow name (without .md extension) - assert.Contains(t, outputStr, "test-mcp") + // Should not contain workflow name (second positional arg not accepted) + assert.NotContains(t, outputStr, "test-mcp") } // TestCompletionMCPInspectIntegration tests the MCP inspect completion via the CLI binary diff --git a/pkg/cli/init_command.go b/pkg/cli/init_command.go index fa13da01708..7550d417c08 100644 --- a/pkg/cli/init_command.go +++ b/pkg/cli/init_command.go @@ -39,7 +39,7 @@ With --codespaces flag: - Configures permissions for current repo: actions:write, contents:write, discussions:read, issues:read, pull-requests:write, workflows:write - Configures permissions for additional repos (in same org): actions:read, contents:read, discussions:read, issues:read, pull-requests:read, workflows:read - Adds GitHub Copilot extensions and gh aw CLI installation -- Use without value (--codespaces) for current repo only, or with comma-separated repos (--codespaces repo1,repo2) +- Use with an empty value (--codespaces "") for current repo only, or with comma-separated repos (--codespaces repo1,repo2) With --completions flag: - Automatically detects your shell (bash, zsh, fish, or PowerShell) @@ -56,7 +56,7 @@ Examples: ` + string(constants.CLIExtensionPrefix) + ` init # Initialize repository with defaults ` + string(constants.CLIExtensionPrefix) + ` init -v # Initialize with verbose output ` + string(constants.CLIExtensionPrefix) + ` init --no-mcp # Skip MCP configuration - ` + string(constants.CLIExtensionPrefix) + ` init --codespaces # Configure Codespaces + ` + string(constants.CLIExtensionPrefix) + ` init --codespaces "" # Configure Codespaces for current repo only ` + string(constants.CLIExtensionPrefix) + ` init --codespaces repo1,repo2 # Codespaces with additional repos ` + string(constants.CLIExtensionPrefix) + ` init --completions # Install shell completions ` + string(constants.CLIExtensionPrefix) + ` init --create-pull-request # Initialize and create a pull request`, @@ -79,7 +79,7 @@ Examples: mcp = mcpFlag } - // Trim the codespace repos string (NoOptDefVal uses a space) + // Trim the codespace repos string (explicit value required; use --codespaces "" for current repo only) codespaceReposStr = strings.TrimSpace(codespaceReposStr) // Parse codespace repos from comma-separated string @@ -114,9 +114,7 @@ Examples: cmd.Flags().StringP("engine", "e", "", "Override AI engine (copilot, claude, codex, gemini, crush)") _ = cmd.Flags().MarkHidden("engine") // Hide the engine flag from help output (internal use only) cmd.Flags().Bool("no-mcp", false, "Skip configuring gh-aw MCP server integration for GitHub Copilot Agent") - cmd.Flags().String("codespaces", "", "Create devcontainer.json for GitHub Codespaces with agentic workflows support. Specify comma-separated repository names in the same organization (e.g., repo1,repo2), or use without value for current repo only") - // NoOptDefVal allows using --codespaces without a value (returns empty string when no value provided) - cmd.Flags().Lookup("codespaces").NoOptDefVal = " " + cmd.Flags().String("codespaces", "", "Create devcontainer.json for GitHub Codespaces with agentic workflows support. Specify comma-separated repository names in the same organization (e.g., repo1,repo2), or use with empty value for current repo only") cmd.Flags().Bool("completions", false, "Install shell completion for the detected shell (bash, zsh, fish, or PowerShell)") cmd.Flags().Bool("create-pull-request", false, "Create a pull request with the initialization changes") cmd.Flags().Bool("pr", false, "Alias for --create-pull-request") diff --git a/pkg/cli/init_command_test.go b/pkg/cli/init_command_test.go index 564fe7e11c2..4c239f7f4ca 100644 --- a/pkg/cli/init_command_test.go +++ b/pkg/cli/init_command_test.go @@ -66,14 +66,14 @@ func TestNewInitCommand(t *testing.T) { return } - // String flags with NoOptDefVal have "" as default value + // String flags without NoOptDefVal require an explicit value if codespaceFlag.DefValue != "" { t.Errorf("Expected codespaces flag default to be '', got %q", codespaceFlag.DefValue) } - // Verify NoOptDefVal is set to a space (allows --codespaces without value) - if codespaceFlag.NoOptDefVal != " " { - t.Errorf("Expected codespaces flag NoOptDefVal to be ' ' (space), got %q", codespaceFlag.NoOptDefVal) + // Verify NoOptDefVal is empty (flag always requires an explicit value, avoids string[=" "] in help) + if codespaceFlag.NoOptDefVal != "" { + t.Errorf("Expected codespaces flag NoOptDefVal to be '' (empty), got %q", codespaceFlag.NoOptDefVal) } // Check create-pull-request flags diff --git a/pkg/cli/logs_command.go b/pkg/cli/logs_command.go index a503f5d3c88..69de32edb8e 100644 --- a/pkg/cli/logs_command.go +++ b/pkg/cli/logs_command.go @@ -103,7 +103,7 @@ Examples: ` + string(constants.CLIExtensionPrefix) + ` logs --after -1w # Delete cached run folders older than 1 week ` + string(constants.CLIExtensionPrefix) + ` logs --after -30d # Delete cached run folders older than 30 days ` + string(constants.CLIExtensionPrefix) + ` logs --after -1mo # Delete cached run folders older than 1 month - ` + string(constants.CLIExtensionPrefix) + ` logs --after 2024-01-01 # Delete cached run folders from before 2024-01-01`, + ` + string(constants.CLIExtensionPrefix) + ` logs --after 2024-01-01 # Delete cached run folders older than 2024-01-01`, RunE: func(cmd *cobra.Command, args []string) error { logsCommandLog.Printf("Starting logs command: args=%d", len(args)) diff --git a/pkg/cli/mcp.go b/pkg/cli/mcp.go index 8ab0a577fb2..161180d82fe 100644 --- a/pkg/cli/mcp.go +++ b/pkg/cli/mcp.go @@ -23,7 +23,7 @@ Available subcommands: • list - List MCP servers defined in agentic workflows • list-tools - List available tools for a specific MCP server • inspect - Inspect MCP servers and list available tools, resources, and roots - • add - Add an MCP tool to an agentic workflow + • add - Add an MCP server to an agentic workflow Examples: gh aw mcp list # List all workflows with MCP servers diff --git a/pkg/cli/mcp_list_tools.go b/pkg/cli/mcp_list_tools.go index 57f459496b9..dafc6978cc0 100644 --- a/pkg/cli/mcp_list_tools.go +++ b/pkg/cli/mcp_list_tools.go @@ -132,7 +132,7 @@ func findWorkflowsWithMCPServer(workflowsDir string, mcpServerName string, verbo // Display matching workflows and suggest using one fmt.Fprintf(os.Stderr, "Found MCP server '%s' in %d workflow(s): %s\n", mcpServerName, len(matchingWorkflows), strings.Join(matchingWorkflows, ", ")) - fmt.Fprintf(os.Stderr, "\nRun 'gh aw mcp list-tools %s ' to list tools for a specific workflow\n", mcpServerName) + fmt.Fprintf(os.Stderr, "\nRun 'gh aw mcp list-tools --server %s' to list tools for a specific workflow\n", mcpServerName) return nil } @@ -167,8 +167,10 @@ func displayToolsList(info *parser.MCPServerInfo, verbose bool) { // NewMCPListToolsSubcommand creates the mcp list-tools subcommand func NewMCPListToolsSubcommand() *cobra.Command { + var serverFilter string + cmd := &cobra.Command{ - Use: "list-tools [workflow]", + Use: "list-tools [workflow]", Short: "List available tools for a specific MCP server", Long: `List available tools for a specific MCP server. @@ -181,46 +183,31 @@ The workflow-id-or-file can be: - A file path (e.g., "weekly-research.md" or ".github/workflows/weekly-research.md") Examples: - gh aw mcp list-tools github # Find workflows with 'github' MCP server - gh aw mcp list-tools github weekly-research # List tools for 'github' server in weekly-research.md - gh aw mcp list-tools safe-outputs issue-triage # List tools for 'safe-outputs' server in issue-triage.md - gh aw mcp list-tools playwright test-workflow -v # Verbose output with tool descriptions + gh aw mcp list-tools --server github # Find workflows with 'github' MCP server + gh aw mcp list-tools weekly-research --server github # List tools for 'github' server in weekly-research.md + gh aw mcp list-tools issue-triage --server safe-outputs # List tools for 'safe-outputs' server in issue-triage.md + gh aw mcp list-tools test-workflow --server playwright -v # Verbose output with tool descriptions The command will: - Parse the workflow to find the specified MCP server configuration - Connect to the MCP server using the same logic as 'mcp inspect' - Display available tools with their descriptions and allowance status`, - Args: cobra.RangeArgs(1, 2), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - mcpServerName := args[0] var workflowFile string - if len(args) > 1 { - workflowFile = args[1] + if len(args) > 0 { + workflowFile = args[0] } verbose, _ := cmd.Flags().GetBool("verbose") - return ListToolsForMCP(workflowFile, mcpServerName, verbose) + return ListToolsForMCP(workflowFile, serverFilter, verbose) }, - ValidArgsFunction: completeMCPListToolsArgs, + ValidArgsFunction: CompleteWorkflowNames, } - return cmd -} + cmd.Flags().StringVar(&serverFilter, "server", "", "MCP server name to list tools for") + _ = cmd.MarkFlagRequired("server") -// commonMCPServerNames contains commonly used MCP server names for shell completion -var commonMCPServerNames = []string{"github", "playwright", "tavily", "safe-outputs"} - -// completeMCPListToolsArgs provides completion for mcp list-tools command arguments -func completeMCPListToolsArgs(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - // First argument: MCP server names are not easily discoverable without a workflow - // For now, provide no file completion but suggest common server names - if len(args) == 0 { - filtered := sliceutil.Filter(commonMCPServerNames, func(s string) bool { - return toComplete == "" || strings.HasPrefix(s, toComplete) - }) - return filtered, cobra.ShellCompDirectiveNoFileComp - } - // Second argument: complete workflow names - return CompleteWorkflowNames(cmd, args, toComplete) + return cmd } diff --git a/pkg/cli/mcp_list_tools_test.go b/pkg/cli/mcp_list_tools_test.go index 286cac86c6b..2c91d4f4839 100644 --- a/pkg/cli/mcp_list_tools_test.go +++ b/pkg/cli/mcp_list_tools_test.go @@ -321,11 +321,18 @@ func TestDisplayToolsList(t *testing.T) { func TestNewMCPListToolsSubcommand(t *testing.T) { cmd := NewMCPListToolsSubcommand() - if cmd.Use != "list-tools [workflow]" { - t.Errorf("Expected Use to be 'list-tools [workflow]', got: %s", cmd.Use) + if cmd.Use != "list-tools [workflow]" { + t.Errorf("Expected Use to be 'list-tools [workflow]', got: %s", cmd.Use) } if cmd.Short != "List available tools for a specific MCP server" { t.Errorf("Expected Short description, got: %s", cmd.Short) } + + serverFlag := cmd.Flags().Lookup("server") + if serverFlag == nil { + t.Error("Expected 'server' flag to be defined") + } else if serverFlag.Annotations == nil || len(serverFlag.Annotations["cobra_annotation_bash_completion_one_required_flag"]) == 0 { + t.Error("Expected 'server' flag to be marked as required") + } }