diff --git a/DEADCODE.md b/DEADCODE.md index 519afa9f52..07bd662d83 100644 --- a/DEADCODE.md +++ b/DEADCODE.md @@ -36,6 +36,8 @@ make fmt - `compiler.ParseWorkflowString` - `compiler.CompileToYAML` +**`pkg/console/console_wasm.go`** — this file provides WASM-specific stub implementations of many `pkg/console` functions (gated with `//go:build js || wasm`). Before deleting any function from `pkg/console/`, `grep` for it in `console_wasm.go`. If the function is called there, either inline the logic in `console_wasm.go` or delete the call. Batch 10 mistake: deleted `renderTreeSimple` from `render.go` but `console_wasm.go`'s `RenderTree` still called it, breaking the WASM build. Fix: replaced the `RenderTree` body in `console_wasm.go` with an inlined closure that no longer calls the deleted helper. + **`compiler_test_helpers.go`** — shows 3 dead functions but serves as shared test infrastructure for ≥15 test files. Do not delete. **Constant/embed rescue** — Some otherwise-dead files contain live constants or `//go:embed` directives. Extract them before deleting the file. @@ -163,7 +165,9 @@ For each batch: - [ ] Delete the function - [ ] Delete test functions that exclusively call the deleted function (not shared helpers) - [ ] Check for now-unused imports in edited files +- [ ] If editing `pkg/console/`, check `pkg/console/console_wasm.go` for calls to the deleted functions - [ ] `go build ./...` +- [ ] `GOARCH=wasm GOOS=js go build ./pkg/console/...` (if `pkg/console/` was touched) - [ ] `go vet ./...` - [ ] `go vet -tags=integration ./...` - [ ] `make fmt` diff --git a/pkg/cli/mcp_tool_table.go b/pkg/cli/mcp_tool_table.go index 974801032e..93796e4960 100644 --- a/pkg/cli/mcp_tool_table.go +++ b/pkg/cli/mcp_tool_table.go @@ -119,118 +119,3 @@ func renderMCPToolTable(info *parser.MCPServerInfo, opts MCPToolTableOptions) st return result } - -// renderMCPHierarchyTree renders all MCP servers and their tools as a tree structure -// This provides a hierarchical view of the MCP configuration -func renderMCPHierarchyTree(configs []parser.MCPServerConfig, serverInfos map[string]*parser.MCPServerInfo) string { - mcpToolTableLog.Printf("Rendering MCP hierarchy tree: server_count=%d", len(configs)) - - if len(configs) == 0 { - mcpToolTableLog.Print("No MCP servers to render") - return "" - } - - // Build tree structure - root := console.TreeNode{ - Value: "MCP Servers", - Children: make([]console.TreeNode, 0, len(configs)), - } - - for _, config := range configs { - serverNode := console.TreeNode{ - Value: fmt.Sprintf("📦 %s (%s)", config.Name, config.Type), - Children: []console.TreeNode{}, - } - - // Add server info if available - if info, ok := serverInfos[config.Name]; ok && info != nil { - // Create a map for quick lookup of allowed tools - allowedMap := make(map[string]bool) - hasWildcard := false - for _, allowed := range config.Allowed { - if allowed == "*" { - hasWildcard = true - } - allowedMap[allowed] = true - } - - // Add tools section - if len(info.Tools) > 0 { - toolsNode := console.TreeNode{ - Value: fmt.Sprintf("🛠️ Tools (%d)", len(info.Tools)), - Children: make([]console.TreeNode, 0, len(info.Tools)), - } - - for _, tool := range info.Tools { - // Determine if tool is allowed - isAllowed := len(config.Allowed) == 0 || hasWildcard || allowedMap[tool.Name] - allowIcon := "🚫" - if isAllowed { - allowIcon = "✅" - } - - // Create tool node with truncated description - toolDesc := tool.Description - if len(toolDesc) > 50 { - toolDesc = toolDesc[:47] + "..." - } - - toolValue := fmt.Sprintf("%s %s - %s", allowIcon, tool.Name, toolDesc) - toolsNode.Children = append(toolsNode.Children, console.TreeNode{ - Value: toolValue, - Children: []console.TreeNode{}, - }) - } - - serverNode.Children = append(serverNode.Children, toolsNode) - } - - // Add resources section - if len(info.Resources) > 0 { - resourcesNode := console.TreeNode{ - Value: fmt.Sprintf("📚 Resources (%d)", len(info.Resources)), - Children: make([]console.TreeNode, 0, len(info.Resources)), - } - - for _, resource := range info.Resources { - resourceValue := fmt.Sprintf("%s - %s", resource.Name, resource.URI) - resourcesNode.Children = append(resourcesNode.Children, console.TreeNode{ - Value: resourceValue, - Children: []console.TreeNode{}, - }) - } - - serverNode.Children = append(serverNode.Children, resourcesNode) - } - - // Add roots section - if len(info.Roots) > 0 { - rootsNode := console.TreeNode{ - Value: fmt.Sprintf("🌳 Roots (%d)", len(info.Roots)), - Children: make([]console.TreeNode, 0, len(info.Roots)), - } - - for _, root := range info.Roots { - rootValue := fmt.Sprintf("%s - %s", root.Name, root.URI) - rootsNode.Children = append(rootsNode.Children, console.TreeNode{ - Value: rootValue, - Children: []console.TreeNode{}, - }) - } - - serverNode.Children = append(serverNode.Children, rootsNode) - } - } else { - // Server info not available (error or not yet queried) - serverNode.Children = append(serverNode.Children, console.TreeNode{ - Value: "⚠️ Server info not available", - Children: []console.TreeNode{}, - }) - } - - root.Children = append(root.Children, serverNode) - } - - // Render the tree - return console.RenderTree(root) -} diff --git a/pkg/cli/mcp_tool_table_test.go b/pkg/cli/mcp_tool_table_test.go index 7d0bd8c646..6eb5a04f0b 100644 --- a/pkg/cli/mcp_tool_table_test.go +++ b/pkg/cli/mcp_tool_table_test.go @@ -255,115 +255,3 @@ func TestDefaultMCPToolTableOptions(t *testing.T) { t.Error("Expected default ShowVerboseHint to be false") } } - -func TestRenderMCPHierarchyTree(t *testing.T) { - // Create test configs - configs := []parser.MCPServerConfig{ - { - BaseMCPServerConfig: types.BaseMCPServerConfig{ - Type: "stdio", - }, - Name: "github", - Allowed: []string{"list_issues", "create_issue"}, - }, - { - BaseMCPServerConfig: types.BaseMCPServerConfig{ - Type: "stdio", - }, - Name: "filesystem", - Allowed: []string{"*"}, - }, - } - - // Create test server infos - serverInfos := map[string]*parser.MCPServerInfo{ - "github": { - Config: parser.MCPServerConfig{ - Name: "github", - Allowed: []string{"list_issues", "create_issue"}, - }, - Tools: []*mcp.Tool{ - {Name: "list_issues", Description: "List GitHub issues"}, - {Name: "create_issue", Description: "Create a new GitHub issue"}, - {Name: "list_pull_requests", Description: "List pull requests"}, - }, - Resources: []*mcp.Resource{ - {Name: "repo", URI: "github://repo"}, - }, - Roots: []*mcp.Root{ - {Name: "root", URI: "github://root"}, - }, - }, - "filesystem": { - Config: parser.MCPServerConfig{ - Name: "filesystem", - Allowed: []string{"*"}, - }, - Tools: []*mcp.Tool{ - {Name: "read_file", Description: "Read a file"}, - {Name: "write_file", Description: "Write to a file"}, - }, - }, - } - - // Render tree - output := renderMCPHierarchyTree(configs, serverInfos) - - // Verify output contains expected elements - expectedStrings := []string{ - "MCP Servers", - "github", - "filesystem", - "list_issues", - "create_issue", - "read_file", - "write_file", - "Tools", - "Resources", - "Roots", - } - - for _, expected := range expectedStrings { - if !strings.Contains(output, expected) { - t.Errorf("Expected output to contain '%s', but it didn't.\nGot:\n%s", expected, output) - } - } - - // Verify output is not empty - if output == "" { - t.Error("renderMCPHierarchyTree returned empty string") - } -} - -func TestRenderMCPHierarchyTree_EmptyConfigs(t *testing.T) { - configs := []parser.MCPServerConfig{} - serverInfos := map[string]*parser.MCPServerInfo{} - - output := renderMCPHierarchyTree(configs, serverInfos) - - if output != "" { - t.Errorf("Expected empty output for empty configs, got: %s", output) - } -} - -func TestRenderMCPHierarchyTree_MissingServerInfo(t *testing.T) { - configs := []parser.MCPServerConfig{ - { - BaseMCPServerConfig: types.BaseMCPServerConfig{ - Type: "stdio", - }, - Name: "missing-server", - }, - } - serverInfos := map[string]*parser.MCPServerInfo{} - - output := renderMCPHierarchyTree(configs, serverInfos) - - // Should still render, but with a warning - if !strings.Contains(output, "missing-server") { - t.Errorf("Expected output to contain server name, got: %s", output) - } - if !strings.Contains(output, "Server info not available") { - t.Errorf("Expected output to contain warning about missing info, got: %s", output) - } -} diff --git a/pkg/console/console.go b/pkg/console/console.go index e0f9430e0c..35e0030da1 100644 --- a/pkg/console/console.go +++ b/pkg/console/console.go @@ -10,7 +10,6 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/charmbracelet/lipgloss/table" - "github.com/charmbracelet/lipgloss/tree" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/styles" "github.com/github/gh-aw/pkg/tty" @@ -200,11 +199,6 @@ func RenderTable(config TableConfig) string { return output.String() } -// FormatLocationMessage formats a file/directory location message -func FormatLocationMessage(message string) string { - return applyStyle(styles.Location, "📁 ") + message -} - // FormatCommandMessage formats a command execution message func FormatCommandMessage(command string) string { return applyStyle(styles.Command, "⚡ ") + command @@ -220,21 +214,11 @@ func FormatPromptMessage(message string) string { return applyStyle(styles.Prompt, "❓ ") + message } -// FormatCountMessage formats a count/numeric status message -func FormatCountMessage(message string) string { - return applyStyle(styles.Count, "📊 ") + message -} - // FormatVerboseMessage formats verbose debugging output func FormatVerboseMessage(message string) string { return applyStyle(styles.Verbose, "🔍 ") + message } -// FormatListHeader formats a section header for lists -func FormatListHeader(header string) string { - return applyStyle(styles.ListHeader, header) -} - // FormatListItem formats an item in a list func FormatListItem(item string) string { return applyStyle(styles.ListItem, " • "+item) @@ -322,34 +306,3 @@ func RenderComposedSections(sections []string) { fmt.Fprintln(os.Stderr, "") } } - -// RenderTree renders a hierarchical tree structure using lipgloss/tree package -func RenderTree(root TreeNode) string { - if !isTTY() { - return renderTreeSimple(root, "", true) - } - - lipglossTree := buildLipglossTree(root) - return lipglossTree.String() -} - -// buildLipglossTree converts our TreeNode structure to lipgloss/tree format -func buildLipglossTree(node TreeNode) *tree.Tree { - t := tree.Root(node.Value). - EnumeratorStyle(styles.TreeEnumerator). - ItemStyle(styles.TreeNode) - - if len(node.Children) > 0 { - children := make([]any, len(node.Children)) - for i, child := range node.Children { - if len(child.Children) > 0 { - children[i] = buildLipglossTree(child) - } else { - children[i] = child.Value - } - } - t.Child(children...) - } - - return t -} diff --git a/pkg/console/console_formatting_test.go b/pkg/console/console_formatting_test.go index ed57c367e0..f96cd19e06 100644 --- a/pkg/console/console_formatting_test.go +++ b/pkg/console/console_formatting_test.go @@ -147,51 +147,6 @@ func TestFormatPromptMessage(t *testing.T) { } } -func TestFormatCountMessage(t *testing.T) { - tests := []struct { - name string - message string - expected string - }{ - { - name: "file count", - message: "Found 15 workflows to compile", - expected: "Found 15 workflows to compile", - }, - { - name: "zero count", - message: "Found 0 issues", - expected: "Found 0 issues", - }, - { - name: "percentage", - message: "Coverage: 85.5%", - expected: "Coverage: 85.5%", - }, - { - name: "empty count message", - message: "", - expected: "", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := FormatCountMessage(tt.message) - - // Should contain the chart emoji prefix - if !strings.Contains(result, "📊") { - t.Errorf("FormatCountMessage() should contain 📊 prefix") - } - - // Should contain the message text - if !strings.Contains(result, tt.expected) { - t.Errorf("FormatCountMessage() = %v, should contain %v", result, tt.expected) - } - }) - } -} - func TestFormatVerboseMessage(t *testing.T) { tests := []struct { name string @@ -232,51 +187,6 @@ func TestFormatVerboseMessage(t *testing.T) { } } -func TestFormatListHeader(t *testing.T) { - tests := []struct { - name string - header string - expected string - }{ - { - name: "simple header", - header: "Available Workflows", - expected: "Available Workflows", - }, - { - name: "header with underscores", - header: "==================", - expected: "==================", - }, - { - name: "empty header", - header: "", - expected: "", - }, - { - name: "header with numbers", - header: "Section 1: Configuration", - expected: "Section 1: Configuration", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := FormatListHeader(tt.header) - - // Should contain the header text - if !strings.Contains(result, tt.expected) { - t.Errorf("FormatListHeader() = %v, should contain %v", result, tt.expected) - } - - // Result should not be empty unless input was empty - if tt.header != "" && result == "" { - t.Errorf("FormatListHeader() returned empty string for non-empty input") - } - }) - } -} - func TestFormatListItem(t *testing.T) { tests := []struct { name string @@ -432,9 +342,7 @@ func TestFormattingFunctionsWithSpecialCharacters(t *testing.T) { FormatCommandMessage(specialChars) FormatProgressMessage(specialChars) FormatPromptMessage(specialChars) - FormatCountMessage(specialChars) FormatVerboseMessage(specialChars) - FormatListHeader(specialChars) FormatListItem(specialChars) FormatErrorMessage(specialChars) }) diff --git a/pkg/console/console_test.go b/pkg/console/console_test.go index 3be2edcd5e..ce95e7f715 100644 --- a/pkg/console/console_test.go +++ b/pkg/console/console_test.go @@ -268,16 +268,6 @@ func TestRenderTable(t *testing.T) { } } -func TestFormatLocationMessage(t *testing.T) { - output := FormatLocationMessage("Downloaded to: /path/to/logs") - if !strings.Contains(output, "Downloaded to: /path/to/logs") { - t.Errorf("Expected output to contain message, got: %s", output) - } - if !strings.Contains(output, "📁") { - t.Errorf("Expected output to contain folder icon, got: %s", output) - } -} - func TestToRelativePath(t *testing.T) { tests := []struct { name string @@ -370,62 +360,6 @@ func TestFormatErrorWithAbsolutePaths(t *testing.T) { } } -func TestRenderTableAsJSON(t *testing.T) { - tests := []struct { - name string - config TableConfig - wantErr bool - }{ - { - name: "simple table", - config: TableConfig{ - Headers: []string{"Name", "Status"}, - Rows: [][]string{ - {"workflow1", "active"}, - {"workflow2", "disabled"}, - }, - }, - wantErr: false, - }, - { - name: "table with spaces in headers", - config: TableConfig{ - Headers: []string{"Workflow Name", "Agent Type", "Is Compiled"}, - Rows: [][]string{ - {"test", "copilot", "Yes"}, - }, - }, - wantErr: false, - }, - { - name: "empty table", - config: TableConfig{ - Headers: []string{}, - Rows: [][]string{}, - }, - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result, err := RenderTableAsJSON(tt.config) - if (err != nil) != tt.wantErr { - t.Errorf("RenderTableAsJSON() error = %v, wantErr %v", err, tt.wantErr) - return - } - // Verify it's valid JSON - if result == "" && len(tt.config.Headers) > 0 { - t.Error("RenderTableAsJSON() returned empty string for non-empty config") - } - // For empty config, should return "[]" - if len(tt.config.Headers) == 0 && result != "[]" { - t.Errorf("RenderTableAsJSON() = %v, want []", result) - } - }) - } -} - func TestClearScreen(t *testing.T) { // ClearScreen should not panic when called // It only clears if stdout is a TTY, so we can't easily test the output @@ -454,205 +388,6 @@ func TestClearLine(t *testing.T) { }) } -func TestRenderTree(t *testing.T) { - tests := []struct { - name string - tree TreeNode - expected []string // Substrings that should be present in output - }{ - { - name: "simple tree with no children", - tree: TreeNode{ - Value: "Root", - Children: []TreeNode{}, - }, - expected: []string{"Root"}, - }, - { - name: "tree with single level children", - tree: TreeNode{ - Value: "Root", - Children: []TreeNode{ - {Value: "Child1", Children: []TreeNode{}}, - {Value: "Child2", Children: []TreeNode{}}, - {Value: "Child3", Children: []TreeNode{}}, - }, - }, - expected: []string{ - "Root", - "Child1", - "Child2", - "Child3", - }, - }, - { - name: "tree with nested children", - tree: TreeNode{ - Value: "Workflow", - Children: []TreeNode{ - { - Value: "Setup", - Children: []TreeNode{ - {Value: "Install dependencies", Children: []TreeNode{}}, - {Value: "Configure environment", Children: []TreeNode{}}, - }, - }, - { - Value: "Build", - Children: []TreeNode{ - {Value: "Compile source", Children: []TreeNode{}}, - {Value: "Run tests", Children: []TreeNode{}}, - }, - }, - {Value: "Deploy", Children: []TreeNode{}}, - }, - }, - expected: []string{ - "Workflow", - "Setup", - "Install dependencies", - "Configure environment", - "Build", - "Compile source", - "Run tests", - "Deploy", - }, - }, - { - name: "tree with MCP server hierarchy", - tree: TreeNode{ - Value: "MCP Servers", - Children: []TreeNode{ - { - Value: "github", - Children: []TreeNode{ - {Value: "list_issues", Children: []TreeNode{}}, - {Value: "create_issue", Children: []TreeNode{}}, - {Value: "list_pull_requests", Children: []TreeNode{}}, - }, - }, - { - Value: "filesystem", - Children: []TreeNode{ - {Value: "read_file", Children: []TreeNode{}}, - {Value: "write_file", Children: []TreeNode{}}, - }, - }, - }, - }, - expected: []string{ - "MCP Servers", - "github", - "list_issues", - "create_issue", - "list_pull_requests", - "filesystem", - "read_file", - "write_file", - }, - }, - { - name: "deeply nested tree", - tree: TreeNode{ - Value: "Level 1", - Children: []TreeNode{ - { - Value: "Level 2", - Children: []TreeNode{ - { - Value: "Level 3", - Children: []TreeNode{ - {Value: "Level 4", Children: []TreeNode{}}, - }, - }, - }, - }, - }, - }, - expected: []string{ - "Level 1", - "Level 2", - "Level 3", - "Level 4", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - output := RenderTree(tt.tree) - - // Check that all expected strings are present - for _, expected := range tt.expected { - if !strings.Contains(output, expected) { - t.Errorf("RenderTree() output missing expected string '%s'\nGot:\n%s", expected, output) - } - } - - // Verify output is not empty - if output == "" { - t.Error("RenderTree() returned empty string") - } - }) - } -} - -func TestRenderTreeSimple(t *testing.T) { - tests := []struct { - name string - tree TreeNode - expected []string // Substrings that should be present - }{ - { - name: "simple tree structure", - tree: TreeNode{ - Value: "Root", - Children: []TreeNode{ - {Value: "Child1", Children: []TreeNode{}}, - {Value: "Child2", Children: []TreeNode{}}, - }, - }, - expected: []string{ - "Root", - "Child1", - "Child2", - }, - }, - { - name: "nested tree structure", - tree: TreeNode{ - Value: "Parent", - Children: []TreeNode{ - { - Value: "Child", - Children: []TreeNode{ - {Value: "Grandchild", Children: []TreeNode{}}, - }, - }, - }, - }, - expected: []string{ - "Parent", - "Child", - "Grandchild", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Use renderTreeSimple directly for testing - output := renderTreeSimple(tt.tree, "", true) - - for _, expected := range tt.expected { - if !strings.Contains(output, expected) { - t.Errorf("renderTreeSimple() output missing expected string '%s'\nGot:\n%s", expected, output) - } - } - }) - } -} - func TestRenderTitleBox(t *testing.T) { tests := []struct { name string diff --git a/pkg/console/console_wasm.go b/pkg/console/console_wasm.go index 8de124fbee..c2b1ea8525 100644 --- a/pkg/console/console_wasm.go +++ b/pkg/console/console_wasm.go @@ -127,5 +127,27 @@ func RenderComposedSections(sections []string) { } func RenderTree(root TreeNode) string { - return renderTreeSimple(root, "", true) + var render func(node TreeNode, prefix string, isLast bool) string + render = func(node TreeNode, prefix string, isLast bool) string { + var output strings.Builder + if prefix == "" { + output.WriteString(node.Value + "\n") + } else { + connector := "├── " + if isLast { + connector = "└── " + } + output.WriteString(prefix + connector + node.Value + "\n") + } + for i, child := range node.Children { + childIsLast := i == len(node.Children)-1 + childPrefix := prefix + "│ " + if isLast { + childPrefix = prefix + " " + } + output.WriteString(render(child, childPrefix, childIsLast)) + } + return output.String() + } + return render(root, "", true) } diff --git a/pkg/console/golden_test.go b/pkg/console/golden_test.go index 648da3cf65..7fe7ea52f4 100644 --- a/pkg/console/golden_test.go +++ b/pkg/console/golden_test.go @@ -131,118 +131,6 @@ func TestGolden_BoxRendering(t *testing.T) { // TestGolden_LayoutBoxRendering tests layout box rendering (returns string) -// TestGolden_TreeRendering tests tree rendering with different hierarchies -func TestGolden_TreeRendering(t *testing.T) { - tests := []struct { - name string - tree TreeNode - }{ - { - name: "single_node", - tree: TreeNode{ - Value: "Root", - Children: []TreeNode{}, - }, - }, - { - name: "flat_tree", - tree: TreeNode{ - Value: "Root", - Children: []TreeNode{ - {Value: "Child1", Children: []TreeNode{}}, - {Value: "Child2", Children: []TreeNode{}}, - {Value: "Child3", Children: []TreeNode{}}, - }, - }, - }, - { - name: "nested_tree", - tree: TreeNode{ - Value: "Workflow", - Children: []TreeNode{ - { - Value: "Setup", - Children: []TreeNode{ - {Value: "Install dependencies", Children: []TreeNode{}}, - {Value: "Configure environment", Children: []TreeNode{}}, - }, - }, - { - Value: "Build", - Children: []TreeNode{ - {Value: "Compile source", Children: []TreeNode{}}, - {Value: "Run tests", Children: []TreeNode{}}, - }, - }, - {Value: "Deploy", Children: []TreeNode{}}, - }, - }, - }, - { - name: "deep_hierarchy", - tree: TreeNode{ - Value: "Level 1", - Children: []TreeNode{ - { - Value: "Level 2", - Children: []TreeNode{ - { - Value: "Level 3", - Children: []TreeNode{ - { - Value: "Level 4", - Children: []TreeNode{ - {Value: "Level 5", Children: []TreeNode{}}, - }, - }, - }, - }, - }, - }, - }, - }, - }, - { - name: "mcp_server_tree", - tree: TreeNode{ - Value: "MCP Servers", - Children: []TreeNode{ - { - Value: "github", - Children: []TreeNode{ - {Value: "list_issues", Children: []TreeNode{}}, - {Value: "create_issue", Children: []TreeNode{}}, - {Value: "list_pull_requests", Children: []TreeNode{}}, - {Value: "create_pull_request", Children: []TreeNode{}}, - }, - }, - { - Value: "filesystem", - Children: []TreeNode{ - {Value: "read_file", Children: []TreeNode{}}, - {Value: "write_file", Children: []TreeNode{}}, - {Value: "list_directory", Children: []TreeNode{}}, - }, - }, - { - Value: "bash", - Children: []TreeNode{ - {Value: "execute", Children: []TreeNode{}}, - }, - }, - }, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - output := RenderTree(tt.tree) - golden.RequireEqual(t, []byte(output)) - }) - } -} - // TestGolden_ErrorFormatting tests error formatting with context func TestGolden_ErrorFormatting(t *testing.T) { tests := []struct { @@ -409,11 +297,6 @@ func TestGolden_MessageFormatting(t *testing.T) { message: "Failed to compile workflow", format: FormatErrorMessage, }, - { - name: "location_message", - message: "Downloaded to: /tmp/logs/workflow-123", - format: FormatLocationMessage, - }, { name: "command_message", message: "gh aw compile workflow.md", diff --git a/pkg/console/input.go b/pkg/console/input.go index c6e87ba2fe..9697c524f3 100644 --- a/pkg/console/input.go +++ b/pkg/console/input.go @@ -9,33 +9,6 @@ import ( "github.com/github/gh-aw/pkg/tty" ) -// PromptInput shows an interactive text input prompt using Bubble Tea (huh) -// Returns the entered text or an error -func PromptInput(title, description, placeholder string) (string, error) { - // Check if stdin is a TTY - if not, we can't show interactive forms - if !tty.IsStderrTerminal() { - return "", errors.New("interactive input not available (not a TTY)") - } - - var value string - - form := huh.NewForm( - huh.NewGroup( - huh.NewInput(). - Title(title). - Description(description). - Placeholder(placeholder). - Value(&value), - ), - ).WithAccessible(IsAccessibleMode()) - - if err := form.Run(); err != nil { - return "", err - } - - return value, nil -} - // PromptSecretInput shows an interactive password input prompt with masking // The input is masked for security and includes validation // Returns the entered secret value or an error @@ -69,31 +42,3 @@ func PromptSecretInput(title, description string) (string, error) { return value, nil } - -// PromptInputWithValidation shows an interactive text input with custom validation -// Returns the entered text or an error -func PromptInputWithValidation(title, description, placeholder string, validate func(string) error) (string, error) { - // Check if stdin is a TTY - if not, we can't show interactive forms - if !tty.IsStderrTerminal() { - return "", errors.New("interactive input not available (not a TTY)") - } - - var value string - - form := huh.NewForm( - huh.NewGroup( - huh.NewInput(). - Title(title). - Description(description). - Placeholder(placeholder). - Validate(validate). - Value(&value), - ), - ).WithAccessible(IsAccessibleMode()) - - if err := form.Run(); err != nil { - return "", err - } - - return value, nil -} diff --git a/pkg/console/input_test.go b/pkg/console/input_test.go index 6db522d6da..bd887c5c7f 100644 --- a/pkg/console/input_test.go +++ b/pkg/console/input_test.go @@ -3,37 +3,12 @@ package console import ( - "errors" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestPromptInput(t *testing.T) { - // Note: Interactive Huh forms cannot be fully tested without a mock terminal - // These tests verify function signatures and basic setup - - t.Run("function signature", func(t *testing.T) { - // Verify the function exists and has the right signature - _ = PromptInput - }) - - t.Run("validates parameters", func(t *testing.T) { - // Test that empty title and description don't cause panics - // In a real terminal, this would show a prompt with empty fields - title := "Test Title" - description := "Test Description" - placeholder := "Enter value" - - // Function exists and parameters are accepted - _, err := PromptInput(title, description, placeholder) - // Will error in test environment (no TTY), but that's expected - require.Error(t, err, "Should error when not in TTY") - assert.Contains(t, err.Error(), "not a TTY", "Error should mention TTY") - }) -} - func TestPromptSecretInput(t *testing.T) { t.Run("function signature", func(t *testing.T) { // Verify the function exists and has the right signature @@ -51,28 +26,3 @@ func TestPromptSecretInput(t *testing.T) { assert.Contains(t, err.Error(), "not a TTY", "Error should mention TTY") }) } - -func TestPromptInputWithValidation(t *testing.T) { - t.Run("function signature", func(t *testing.T) { - // Verify the function exists and has the right signature - _ = PromptInputWithValidation - }) - - t.Run("accepts custom validator", func(t *testing.T) { - title := "Test Title" - description := "Test Description" - placeholder := "Enter value" - validator := func(s string) error { - if len(s) < 3 { - return errors.New("must be at least 3 characters") - } - return nil - } - - // Function exists and parameters are accepted - _, err := PromptInputWithValidation(title, description, placeholder, validator) - // Will error in test environment (no TTY), but that's expected - require.Error(t, err, "Should error when not in TTY") - assert.Contains(t, err.Error(), "not a TTY", "Error should mention TTY") - }) -} diff --git a/pkg/console/progress.go b/pkg/console/progress.go index 482bac5ac4..96cd9a9206 100644 --- a/pkg/console/progress.go +++ b/pkg/console/progress.go @@ -66,31 +66,6 @@ func NewProgressBar(total int64) *ProgressBar { } } -// NewIndeterminateProgressBar creates a progress bar for when the total size is unknown -// This mode shows activity without a specific completion percentage, useful for: -// - Streaming downloads with unknown size -// - Processing unknown number of items -// - Operations where duration cannot be predicted -// -// The progress bar automatically adapts to TTY/non-TTY environments -func NewIndeterminateProgressBar() *ProgressBar { - progressLog.Print("Creating indeterminate progress bar") - prog := progress.New( - progress.WithScaledGradient("#BD93F9", "#8BE9FD"), - progress.WithWidth(40), - ) - - prog.EmptyColor = "#6272A4" // Muted purple-gray - - return &ProgressBar{ - progress: prog, - total: 0, - current: 0, - indeterminate: true, - updateCount: 0, - } -} - // Update updates the current progress and returns a formatted string // In determinate mode: // - TTY: Returns a visual progress bar with gradient and percentage diff --git a/pkg/console/progress_test.go b/pkg/console/progress_test.go index 2214bda14d..5f7f03e452 100644 --- a/pkg/console/progress_test.go +++ b/pkg/console/progress_test.go @@ -328,93 +328,10 @@ func TestProgressBarNonTTYFallback(t *testing.T) { }) } -func TestNewIndeterminateProgressBar(t *testing.T) { - t.Run("creates indeterminate progress bar successfully", func(t *testing.T) { - bar := NewIndeterminateProgressBar() - - require.NotNil(t, bar, "NewIndeterminateProgressBar should not return nil") - assert.Equal(t, int64(0), bar.total, "Total should be 0 for indeterminate mode") - assert.Equal(t, int64(0), bar.current, "Current should start at 0") - assert.True(t, bar.indeterminate, "Indeterminate flag should be true") - require.NotNil(t, bar.progress, "Progress model should be initialized") - }) -} - -func TestIndeterminateProgressBarUpdate(t *testing.T) { - t.Run("indeterminate mode with no data", func(t *testing.T) { - bar := NewIndeterminateProgressBar() - output := bar.Update(0) - - assert.NotEmpty(t, output, "Update should return non-empty string") - - // In non-TTY mode, should show "Processing..." - if !isTTY() { - assert.Equal(t, "Processing...", output, "Should show processing indicator") - } - }) - - t.Run("indeterminate mode with current value", func(t *testing.T) { - bar := NewIndeterminateProgressBar() - output := bar.Update(1024 * 1024) // 1MB processed - - assert.NotEmpty(t, output, "Update should return non-empty string") - - // In non-TTY mode, should show current value - if !isTTY() { - assert.Contains(t, output, "Processing...", "Should show processing text") - assert.Contains(t, output, "1.0MB", "Should show current size") - } - }) - - t.Run("indeterminate mode multiple updates", func(t *testing.T) { - bar := NewIndeterminateProgressBar() - - // Simulate progressive updates without known total - updates := []int64{0, 512 * 1024, 1024 * 1024, 2 * 1024 * 1024} - for _, value := range updates { - output := bar.Update(value) - assert.NotEmpty(t, output, "Each update should produce output") - assert.Equal(t, value, bar.current, "Current should track the latest update") - } - }) - - t.Run("indeterminate mode produces varying output", func(t *testing.T) { - // Skip if not in TTY mode as the pulsing effect is only visible in TTY - if !isTTY() { - t.Skip("Test requires TTY mode to validate pulsing effect") - } - - bar := NewIndeterminateProgressBar() - - // Update with different values to create pulse effect - outputs := make([]string, 8) - for i := range outputs { - outputs[i] = bar.Update(int64(i * 100)) - } - - // In TTY mode, outputs should vary (pulsing effect) - // We just verify they're all non-empty and at least some are different - allSame := true - for i := 1; i < len(outputs); i++ { - if outputs[i] != outputs[0] { - allSame = false - break - } - } - assert.False(t, allSame, "Indeterminate progress should produce varying visual output for pulsing effect") - }) -} - func TestProgressBarModeSelection(t *testing.T) { t.Run("determinate mode has total and not indeterminate", func(t *testing.T) { bar := NewProgressBar(1024) assert.Equal(t, int64(1024), bar.total, "Determinate mode should have total") assert.False(t, bar.indeterminate, "Determinate mode should not be indeterminate") }) - - t.Run("indeterminate mode has no total and is indeterminate", func(t *testing.T) { - bar := NewIndeterminateProgressBar() - assert.Equal(t, int64(0), bar.total, "Indeterminate mode should have zero total") - assert.True(t, bar.indeterminate, "Indeterminate mode should be indeterminate") - }) } diff --git a/pkg/console/render.go b/pkg/console/render.go index 47496ecacc..5faa8baeaa 100644 --- a/pkg/console/render.go +++ b/pkg/console/render.go @@ -1,7 +1,6 @@ package console import ( - "encoding/json" "fmt" "os" "path/filepath" @@ -604,32 +603,6 @@ func ToRelativePath(path string) string { return relPath } -// RenderTableAsJSON renders a table configuration as JSON -func RenderTableAsJSON(config TableConfig) (string, error) { - if len(config.Headers) == 0 { - return "[]", nil - } - - var result []map[string]string - for _, row := range config.Rows { - obj := make(map[string]string) - for i, cell := range row { - if i < len(config.Headers) { - key := strings.ToLower(strings.ReplaceAll(config.Headers[i], " ", "_")) - obj[key] = cell - } - } - result = append(result, obj) - } - - jsonBytes, err := json.Marshal(result) - if err != nil { - return "", fmt.Errorf("failed to marshal table to JSON: %w", err) - } - - return string(jsonBytes), nil -} - // FormatErrorWithSuggestions formats an error message with actionable suggestions func FormatErrorWithSuggestions(message string, suggestions []string) string { var output strings.Builder @@ -645,38 +618,6 @@ func FormatErrorWithSuggestions(message string, suggestions []string) string { return output.String() } -// renderTreeSimple renders a simple text-based tree without styling -func renderTreeSimple(node TreeNode, prefix string, isLast bool) string { - var output strings.Builder - - connector := "├── " - if isLast { - connector = "└── " - } - if prefix == "" { - output.WriteString(node.Value + "\n") - } else { - output.WriteString(prefix + connector + node.Value + "\n") - } - - for i, child := range node.Children { - childIsLast := i == len(node.Children)-1 - var childPrefix string - if prefix == "" { - childPrefix = "" - } else { - if isLast { - childPrefix = prefix + " " - } else { - childPrefix = prefix + "│ " - } - } - output.WriteString(renderTreeSimple(child, childPrefix, childIsLast)) - } - - return output.String() -} - // findWordEnd finds the end of a word starting at the given position func findWordEnd(line string, start int) int { if start >= len(line) { diff --git a/pkg/console/spinner.go b/pkg/console/spinner.go index d378478be2..1bea3b6181 100644 --- a/pkg/console/spinner.go +++ b/pkg/console/spinner.go @@ -177,5 +177,3 @@ func (s *SpinnerWrapper) UpdateMessage(message string) { } } } - -func (s *SpinnerWrapper) IsEnabled() bool { return s.enabled } diff --git a/pkg/console/spinner_test.go b/pkg/console/spinner_test.go index ab89311459..e719faecd1 100644 --- a/pkg/console/spinner_test.go +++ b/pkg/console/spinner_test.go @@ -38,12 +38,6 @@ func TestSpinnerAccessibilityMode(t *testing.T) { // Spinner should be disabled when ACCESSIBLE is set // Note: This may still be true if running in non-TTY environment - if spinner.IsEnabled() { - // Only check if we're actually in a TTY - // In CI/test environments, spinner will be disabled regardless - t.Log("Spinner enabled despite ACCESSIBLE=1 (may be expected in non-TTY)") - } - // Ensure no panic when starting/stopping disabled spinner spinner.Start() spinner.Stop() @@ -67,17 +61,6 @@ func TestSpinnerUpdateMessage(t *testing.T) { spinner.Stop() } -func TestSpinnerIsEnabled(t *testing.T) { - spinner := NewSpinner("Test message") - - // IsEnabled should return a boolean without panicking - enabled := spinner.IsEnabled() - - // The value depends on whether we're running in a TTY or not - // but the method should not panic - _ = enabled -} - func TestSpinnerStopWithMessage(t *testing.T) { spinner := NewSpinner("Processing...") @@ -185,9 +168,7 @@ func TestSpinnerDisabledOperations(t *testing.T) { spinner.StopWithMessage("Final message") // Check that spinner is disabled - if spinner.IsEnabled() && os.Getenv("ACCESSIBLE") != "" { - t.Error("Spinner should be disabled when ACCESSIBLE is set") - } + _ = spinner } func TestSpinnerRapidStartStop(t *testing.T) { diff --git a/pkg/console/terminal.go b/pkg/console/terminal.go index 1436b21985..91e4782542 100644 --- a/pkg/console/terminal.go +++ b/pkg/console/terminal.go @@ -35,22 +35,6 @@ func ClearLine() { } } -// MoveCursorUp moves cursor up n lines if stderr is a TTY. -// Uses ANSI escape code: \033[nA where n is the number of lines. -func MoveCursorUp(n int) { - if tty.IsStderrTerminal() { - fmt.Fprintf(os.Stderr, "\033[%dA", n) - } -} - -// MoveCursorDown moves cursor down n lines if stderr is a TTY. -// Uses ANSI escape code: \033[nB where n is the number of lines. -func MoveCursorDown(n int) { - if tty.IsStderrTerminal() { - fmt.Fprintf(os.Stderr, "\033[%dB", n) - } -} - // ShowWelcomeBanner clears the screen and displays the welcome banner for interactive commands. // Use this at the start of interactive commands (add, trial, init) for a consistent experience. func ShowWelcomeBanner(description string) { diff --git a/pkg/console/terminal_test.go b/pkg/console/terminal_test.go deleted file mode 100644 index 3b643d84b0..0000000000 --- a/pkg/console/terminal_test.go +++ /dev/null @@ -1,173 +0,0 @@ -//go:build !integration - -package console - -import ( - "bytes" - "io" - "os" - "testing" - - "github.com/stretchr/testify/assert" -) - -// captureStderr captures stderr output during function execution -func captureStderr(t *testing.T, fn func()) string { - t.Helper() - - // Save original stderr - oldStderr := os.Stderr - - // Create a pipe to capture stderr - r, w, err := os.Pipe() - if err != nil { - t.Fatalf("Failed to create pipe: %v", err) - } - - // Replace stderr with the write end of the pipe - os.Stderr = w - - // Create a channel to receive the captured output - outputChan := make(chan string, 1) - - // Read from the pipe in a goroutine - go func() { - var buf bytes.Buffer - io.Copy(&buf, r) - outputChan <- buf.String() - }() - - // Execute the function - fn() - - // Close the write end and restore stderr - w.Close() - os.Stderr = oldStderr - - // Get the captured output - output := <-outputChan - r.Close() - - return output -} - -func TestMoveCursorUp(t *testing.T) { - tests := []struct { - name string - lines int - }{ - { - name: "move up 1 line", - lines: 1, - }, - { - name: "move up 5 lines", - lines: 5, - }, - { - name: "move up 0 lines", - lines: 0, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - output := captureStderr(t, func() { - MoveCursorUp(tt.lines) - }) - - // In non-TTY environments, output should be empty - // We just ensure no panic occurs - assert.NotNil(t, output, "MoveCursorUp should not panic") - }) - } -} - -func TestMoveCursorDown(t *testing.T) { - tests := []struct { - name string - lines int - }{ - { - name: "move down 1 line", - lines: 1, - }, - { - name: "move down 5 lines", - lines: 5, - }, - { - name: "move down 0 lines", - lines: 0, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - output := captureStderr(t, func() { - MoveCursorDown(tt.lines) - }) - - // In non-TTY environments, output should be empty - // We just ensure no panic occurs - assert.NotNil(t, output, "MoveCursorDown should not panic") - }) - } -} - -func TestTerminalCursorFunctionsNoTTY(t *testing.T) { - // This test verifies that in non-TTY environments (like CI/tests), - // no ANSI codes are emitted for cursor movement functions - - tests := []struct { - name string - fn func() - }{ - { - name: "MoveCursorUp", - fn: func() { MoveCursorUp(5) }, - }, - { - name: "MoveCursorDown", - fn: func() { MoveCursorDown(3) }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - output := captureStderr(t, tt.fn) - - // Since tests typically run in non-TTY, verify output is empty - // This ensures we properly respect TTY detection - if os.Getenv("CI") != "" || !isRealTerminal() { - assert.Empty(t, output, "%s should not output ANSI codes in non-TTY", tt.name) - } - }) - } -} - -// isRealTerminal checks if we're actually running in a terminal -// This is a helper to distinguish between test environments and real terminals -func isRealTerminal() bool { - // In test environments, stderr is typically redirected - fileInfo, err := os.Stderr.Stat() - if err != nil { - return false - } - // Check if stderr is a character device (terminal) - return (fileInfo.Mode() & os.ModeCharDevice) != 0 -} - -func TestTerminalCursorFunctionsDoNotPanic(t *testing.T) { - // Ensure all cursor movement functions can be called safely without panicking - // even in edge cases - - t.Run("all cursor functions", func(t *testing.T) { - assert.NotPanics(t, func() { - MoveCursorUp(0) - MoveCursorUp(100) - MoveCursorDown(0) - MoveCursorDown(100) - }, "Cursor movement functions should never panic") - }) -}