diff --git a/pkg/cli/mcp_inspect_mcp.go b/pkg/cli/mcp_inspect_mcp.go index 0657df2e91..035db66432 100644 --- a/pkg/cli/mcp_inspect_mcp.go +++ b/pkg/cli/mcp_inspect_mcp.go @@ -325,49 +325,18 @@ func displayServerCapabilities(info *parser.MCPServerInfo, toolFilter string) { } else { fmt.Printf("\n%s\n", headerStyle.Render("šŸ› ļø Tool Access Status")) - // Create a map for quick lookup of allowed tools - allowedMap := make(map[string]bool) - for _, allowed := range info.Config.Allowed { - allowedMap[allowed] = true + // Configure options for inspect command + // Use a slightly shorter truncation length than list-tools for better fit + opts := MCPToolTableOptions{ + TruncateLength: 50, + ShowSummary: true, + ShowVerboseHint: false, } - headers := []string{"Tool Name", "Allow", "Description"} - rows := make([][]string, 0, len(info.Tools)) - - for _, tool := range info.Tools { - description := tool.Description - if len(description) > 50 { - description = description[:47] + "..." - } - - // Determine status - status := "šŸš«" - if len(info.Config.Allowed) == 0 { - // If no allowed list is specified, assume all tools are allowed - status = "āœ…" - } else if allowedMap[tool.Name] { - status = "āœ…" - } - - rows = append(rows, []string{tool.Name, status, description}) - } - - table := console.RenderTable(console.TableConfig{ - Headers: headers, - Rows: rows, - }) + // Render the table using the shared helper + table := renderMCPToolTable(info, opts) fmt.Print(table) - // Display summary - allowedCount := 0 - for _, tool := range info.Tools { - if len(info.Config.Allowed) == 0 || allowedMap[tool.Name] { - allowedCount++ - } - } - fmt.Printf("\nšŸ“Š Summary: %d allowed, %d not allowed out of %d total tools\n", - allowedCount, len(info.Tools)-allowedCount, len(info.Tools)) - // Add helpful hint about how to allow tools in workflow frontmatter displayToolAllowanceHint(info) } diff --git a/pkg/cli/mcp_list_tools.go b/pkg/cli/mcp_list_tools.go index d5acf89fc5..35c915cf4d 100644 --- a/pkg/cli/mcp_list_tools.go +++ b/pkg/cli/mcp_list_tools.go @@ -17,8 +17,6 @@ var mcpListToolsLog = logger.New("cli:mcp_list_tools") const ( // maxDescriptionLength is the maximum length for tool descriptions before truncation maxDescriptionLength = 60 - // truncationLength is the length at which to truncate descriptions (leaving room for "...") - truncationLength = 57 ) // ListToolsForMCP lists available tools for a specific MCP server @@ -144,85 +142,24 @@ func displayToolsList(info *parser.MCPServerInfo, verbose bool) { fmt.Printf("\n%s\n", console.FormatInfoMessage(fmt.Sprintf("šŸ› ļø Available Tools (%d total)", len(info.Tools)))) - // Create a map for quick lookup of allowed tools from workflow configuration - allowedMap := make(map[string]bool) - - // Check for wildcard "*" which means all tools are allowed - hasWildcard := false - for _, allowed := range info.Config.Allowed { - if allowed == "*" { - hasWildcard = true - } - allowedMap[allowed] = true + // Configure options based on verbose flag + opts := MCPToolTableOptions{ + ShowSummary: true, } if verbose { - // Detailed table with full descriptions - headers := []string{"Tool Name", "Allow", "Description"} - rows := make([][]string, 0, len(info.Tools)) - - for _, tool := range info.Tools { - // In verbose mode, show full descriptions without truncation - description := tool.Description - - // Determine status - status := "šŸš«" - if len(info.Config.Allowed) == 0 || hasWildcard { - // If no allowed list is specified or "*" wildcard is present, assume all tools are allowed - status = "āœ…" - } else if allowedMap[tool.Name] { - status = "āœ…" - } - - rows = append(rows, []string{tool.Name, status, description}) - } - - table := console.RenderTable(console.TableConfig{ - Headers: headers, - Rows: rows, - }) - fmt.Print(table) - - // Display summary - allowedCount := 0 - for _, tool := range info.Tools { - if len(info.Config.Allowed) == 0 || hasWildcard || allowedMap[tool.Name] { - allowedCount++ - } - } - fmt.Printf("\nšŸ“Š Summary: %d allowed, %d not allowed out of %d total tools\n", - allowedCount, len(info.Tools)-allowedCount, len(info.Tools)) + // In verbose mode, show full descriptions without truncation + opts.TruncateLength = 0 + opts.ShowVerboseHint = false } else { - // Compact table with truncated descriptions for single-line display - headers := []string{"Tool Name", "Allow", "Description"} - rows := make([][]string, 0, len(info.Tools)) - - for _, tool := range info.Tools { - // In non-verbose mode, truncate descriptions to keep tools on single lines - description := tool.Description - if len(description) > maxDescriptionLength { - description = description[:truncationLength] + "..." - } - - // Determine status - status := "šŸš«" - if len(info.Config.Allowed) == 0 || hasWildcard { - // If no allowed list is specified or "*" wildcard is present, assume all tools are allowed - status = "āœ…" - } else if allowedMap[tool.Name] { - status = "āœ…" - } - - rows = append(rows, []string{tool.Name, status, description}) - } - - table := console.RenderTable(console.TableConfig{ - Headers: headers, - Rows: rows, - }) - fmt.Print(table) - fmt.Printf("\nRun with --verbose for detailed information\n") + // In non-verbose mode, truncate descriptions to keep tools on single lines + opts.TruncateLength = maxDescriptionLength + opts.ShowVerboseHint = true } + + // Render the table using the shared helper + table := renderMCPToolTable(info, opts) + fmt.Print(table) } // NewMCPListToolsSubcommand creates the mcp list-tools subcommand diff --git a/pkg/cli/mcp_tool_table.go b/pkg/cli/mcp_tool_table.go new file mode 100644 index 0000000000..792c288d06 --- /dev/null +++ b/pkg/cli/mcp_tool_table.go @@ -0,0 +1,112 @@ +package cli + +import ( + "fmt" + + "github.com/githubnext/gh-aw/pkg/console" + "github.com/githubnext/gh-aw/pkg/parser" +) + +// MCPToolTableOptions configures how the MCP tool table is rendered +type MCPToolTableOptions struct { + // TruncateLength is the maximum length for tool descriptions before truncation + // A value of 0 means no truncation + TruncateLength int + // ShowSummary controls whether to display the summary line at the bottom + ShowSummary bool + // SummaryFormat is the format string for the summary (default: "šŸ“Š Summary: %d allowed, %d not allowed out of %d total tools\n") + SummaryFormat string + // ShowVerboseHint controls whether to show the "Run with --verbose" hint in non-verbose mode + ShowVerboseHint bool +} + +// DefaultMCPToolTableOptions returns the default options for rendering MCP tool tables +func DefaultMCPToolTableOptions() MCPToolTableOptions { + return MCPToolTableOptions{ + TruncateLength: 60, + ShowSummary: true, + SummaryFormat: "\nšŸ“Š Summary: %d allowed, %d not allowed out of %d total tools\n", + ShowVerboseHint: false, + } +} + +// renderMCPToolTable renders an MCP tool table with configurable options +// This is the shared rendering logic used by both mcp list-tools and mcp inspect commands +func renderMCPToolTable(info *parser.MCPServerInfo, opts MCPToolTableOptions) string { + if len(info.Tools) == 0 { + return "" + } + + // Create a map for quick lookup of allowed tools from workflow configuration + allowedMap := make(map[string]bool) + + // Check for wildcard "*" which means all tools are allowed + hasWildcard := false + for _, allowed := range info.Config.Allowed { + if allowed == "*" { + hasWildcard = true + } + allowedMap[allowed] = true + } + + // Build table headers and rows + headers := []string{"Tool Name", "Allow", "Description"} + rows := make([][]string, 0, len(info.Tools)) + + for _, tool := range info.Tools { + description := tool.Description + + // Apply truncation if requested + if opts.TruncateLength > 0 && len(description) > opts.TruncateLength { + // Leave room for "..." + truncateAt := opts.TruncateLength - 3 + if truncateAt > 0 { + description = description[:truncateAt] + "..." + } + } + + // Determine status + status := "šŸš«" + if len(info.Config.Allowed) == 0 || hasWildcard { + // If no allowed list is specified or "*" wildcard is present, assume all tools are allowed + status = "āœ…" + } else if allowedMap[tool.Name] { + status = "āœ…" + } + + rows = append(rows, []string{tool.Name, status, description}) + } + + // Render the table + table := console.RenderTable(console.TableConfig{ + Headers: headers, + Rows: rows, + }) + + result := table + + // Add summary if requested + if opts.ShowSummary { + allowedCount := 0 + for _, tool := range info.Tools { + if len(info.Config.Allowed) == 0 || hasWildcard || allowedMap[tool.Name] { + allowedCount++ + } + } + + summaryFormat := opts.SummaryFormat + if summaryFormat == "" { + summaryFormat = "\nšŸ“Š Summary: %d allowed, %d not allowed out of %d total tools\n" + } + + result += fmt.Sprintf(summaryFormat, + allowedCount, len(info.Tools)-allowedCount, len(info.Tools)) + } + + // Add verbose hint if requested + if opts.ShowVerboseHint { + result += "\nRun with --verbose for detailed information\n" + } + + return result +} diff --git a/pkg/cli/mcp_tool_table_test.go b/pkg/cli/mcp_tool_table_test.go new file mode 100644 index 0000000000..9bd8ac6838 --- /dev/null +++ b/pkg/cli/mcp_tool_table_test.go @@ -0,0 +1,256 @@ +package cli + +import ( + "strings" + "testing" + + "github.com/githubnext/gh-aw/pkg/parser" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +func TestRenderMCPToolTable(t *testing.T) { + // Create mock data + mockInfo := &parser.MCPServerInfo{ + Config: parser.MCPServerConfig{ + Name: "test-server", + Type: "stdio", + Command: "test", + Allowed: []string{"tool1", "tool3"}, // Only tool1 and tool3 are allowed + }, + Tools: []*mcp.Tool{ + { + Name: "tool1", + Description: "This is a short description", + }, + { + Name: "tool2", + Description: "This is a very long description that exceeds the maximum length limit and should be truncated in non-verbose mode", + }, + { + Name: "tool3", + Description: "Another tool with a medium-length description", + }, + }, + } + + t.Run("empty_tools_list_returns_empty_string", func(t *testing.T) { + emptyInfo := &parser.MCPServerInfo{ + Config: parser.MCPServerConfig{Name: "empty-server"}, + Tools: []*mcp.Tool{}, + } + + opts := DefaultMCPToolTableOptions() + result := renderMCPToolTable(emptyInfo, opts) + + if result != "" { + t.Errorf("Expected empty string for empty tools list, got: %s", result) + } + }) + + t.Run("renders_table_with_default_options", func(t *testing.T) { + opts := DefaultMCPToolTableOptions() + result := renderMCPToolTable(mockInfo, opts) + + // Verify table is rendered + if result == "" { + t.Error("Expected non-empty result") + } + + // Check for table headers + if !strings.Contains(result, "Tool Name") { + t.Error("Expected 'Tool Name' header") + } + if !strings.Contains(result, "Allow") { + t.Error("Expected 'Allow' header") + } + if !strings.Contains(result, "Description") { + t.Error("Expected 'Description' header") + } + + // Check for tool names + if !strings.Contains(result, "tool1") { + t.Error("Expected 'tool1' in output") + } + if !strings.Contains(result, "tool2") { + t.Error("Expected 'tool2' in output") + } + + // Check for allow/disallow indicators + if !strings.Contains(result, "āœ…") { + t.Error("Expected 'āœ…' for allowed tools") + } + if !strings.Contains(result, "šŸš«") { + t.Error("Expected 'šŸš«' for disallowed tools") + } + + // Check for summary + if !strings.Contains(result, "Summary") { + t.Error("Expected summary line") + } + if !strings.Contains(result, "2 allowed") { + t.Error("Expected '2 allowed' in summary") + } + if !strings.Contains(result, "1 not allowed") { + t.Error("Expected '1 not allowed' in summary") + } + }) + + t.Run("truncates_descriptions_when_requested", func(t *testing.T) { + opts := MCPToolTableOptions{ + TruncateLength: 30, + ShowSummary: false, + ShowVerboseHint: false, + } + result := renderMCPToolTable(mockInfo, opts) + + // Long description should be truncated + if strings.Contains(result, "exceeds the maximum") { + t.Error("Long description should be truncated") + } + if !strings.Contains(result, "...") { + t.Error("Truncated descriptions should end with '...'") + } + }) + + t.Run("no_truncation_when_length_is_zero", func(t *testing.T) { + opts := MCPToolTableOptions{ + TruncateLength: 0, // No truncation + ShowSummary: false, + ShowVerboseHint: false, + } + result := renderMCPToolTable(mockInfo, opts) + + // Full description should be present + if !strings.Contains(result, "exceeds the maximum length limit") { + t.Error("Full description should be present when truncation is disabled") + } + }) + + t.Run("hides_summary_when_disabled", func(t *testing.T) { + opts := MCPToolTableOptions{ + TruncateLength: 60, + ShowSummary: false, + ShowVerboseHint: false, + } + result := renderMCPToolTable(mockInfo, opts) + + if strings.Contains(result, "Summary") { + t.Error("Summary should not be present when ShowSummary is false") + } + }) + + t.Run("shows_verbose_hint_when_enabled", func(t *testing.T) { + opts := MCPToolTableOptions{ + TruncateLength: 60, + ShowSummary: false, + ShowVerboseHint: true, + } + result := renderMCPToolTable(mockInfo, opts) + + if !strings.Contains(result, "Run with --verbose") { + t.Error("Expected verbose hint when ShowVerboseHint is true") + } + }) + + t.Run("custom_summary_format", func(t *testing.T) { + opts := MCPToolTableOptions{ + TruncateLength: 60, + ShowSummary: true, + SummaryFormat: "\nCustom: %d/%d/%d\n", + ShowVerboseHint: false, + } + result := renderMCPToolTable(mockInfo, opts) + + if !strings.Contains(result, "Custom: 2/1/3") { + t.Error("Expected custom summary format") + } + }) + + t.Run("no_allowed_tools_means_all_allowed", func(t *testing.T) { + noAllowedInfo := &parser.MCPServerInfo{ + Config: parser.MCPServerConfig{ + Name: "no-allowed-server", + Type: "stdio", + Command: "test", + Allowed: []string{}, // Empty allowed list means all tools allowed + }, + Tools: []*mcp.Tool{ + { + Name: "any_tool", + Description: "Any tool should be allowed", + }, + }, + } + + opts := MCPToolTableOptions{ + TruncateLength: 60, + ShowSummary: true, + ShowVerboseHint: false, + } + result := renderMCPToolTable(noAllowedInfo, opts) + + // Should show all tools as allowed + if !strings.Contains(result, "1 allowed") { + t.Error("Expected all tools to be allowed when no allowed list is specified") + } + if !strings.Contains(result, "0 not allowed") { + t.Error("Expected 0 not allowed when no allowed list is specified") + } + }) + + t.Run("wildcard_allows_all_tools", func(t *testing.T) { + wildcardInfo := &parser.MCPServerInfo{ + Config: parser.MCPServerConfig{ + Name: "wildcard-server", + Type: "stdio", + Command: "test", + Allowed: []string{"*"}, // Wildcard in workflow config + }, + Tools: []*mcp.Tool{ + { + Name: "any_tool1", + Description: "First tool", + }, + { + Name: "any_tool2", + Description: "Second tool", + }, + }, + } + + opts := MCPToolTableOptions{ + TruncateLength: 60, + ShowSummary: true, + ShowVerboseHint: false, + } + result := renderMCPToolTable(wildcardInfo, opts) + + // All tools should be allowed due to wildcard + if !strings.Contains(result, "2 allowed") { + t.Error("Expected all tools to be allowed with wildcard") + } + if !strings.Contains(result, "0 not allowed") { + t.Error("Expected 0 not allowed with wildcard") + } + }) +} + +func TestDefaultMCPToolTableOptions(t *testing.T) { + opts := DefaultMCPToolTableOptions() + + if opts.TruncateLength != 60 { + t.Errorf("Expected default TruncateLength to be 60, got %d", opts.TruncateLength) + } + + if !opts.ShowSummary { + t.Error("Expected default ShowSummary to be true") + } + + if opts.SummaryFormat != "\nšŸ“Š Summary: %d allowed, %d not allowed out of %d total tools\n" { + t.Errorf("Expected default SummaryFormat, got: %s", opts.SummaryFormat) + } + + if opts.ShowVerboseHint { + t.Error("Expected default ShowVerboseHint to be false") + } +}