diff --git a/.kiro-krew/specs/issue-258-footer-command-execution-all-tabs.md b/.kiro-krew/specs/issue-258-footer-command-execution-all-tabs.md new file mode 100644 index 0000000..45494cf --- /dev/null +++ b/.kiro-krew/specs/issue-258-footer-command-execution-all-tabs.md @@ -0,0 +1,247 @@ +# Design Specification: Footer Command Execution in Agent and Log Tabs + +**Issue**: #258 +**Title**: Footer command execution fails in agent and log tabs +**Closes**: #258 + +## Problem Summary + +When users type commands in the footer input field while viewing agent tabs or log tabs, the autocomplete suggestions display correctly, but pressing Enter does not execute the command. The command only executes when switching back to the main tab. This breaks user workflow and forces unnecessary tab switching. + +### Root Cause + +The enter key handler in `internal/tui/tui.go` (lines 611-647) uses hardcoded tab type logic that only executes footer commands for: +- **Main tab (TabTypeMain)**: Always executes commands +- **Planning tab (TabTypePlanning)**: Executes commands when footer is focused + +For agent tabs (TabTypeAgent) and log tabs (TabTypeLog), the enter key is forwarded to the tab's Update method but never reaches command execution logic, even when the footer input has focus. + +The current logic checks tab type first, then footer focus. It should check footer focus first, then handle tab-specific behavior only when footer is NOT focused. + +## Solution Approach + +Refactor the enter key handler to **prioritize footer focus state over tab type** when determining command execution. The logic should be: + +1. **If footer is focused**: Execute the command regardless of tab type +2. **If footer is NOT focused**: Forward the enter key to the active tab for tab-specific handling + +This approach ensures consistent command execution across all tab types while preserving tab-specific enter key behavior (e.g., sending messages in planning tabs when the tab content has focus, not the footer). + +## Architecture Overview + +### Current Flow (Problematic) +``` +Enter Key Pressed + ├─ Check: Is active tab Main? + │ └─ YES → Execute command + │ └─ NO → Check: Is active tab Planning AND footer focused? + │ └─ YES → Execute command + │ └─ NO → Forward to tab's Update method (command lost) + └─ Agent/Log tabs never execute commands +``` + +### New Flow (Correct) +``` +Enter Key Pressed + ├─ Check: Is footer input focused? + │ └─ YES → Execute command (all tab types) + │ └─ NO → Forward to active tab's Update method + └─ Tab-specific enter handling only when footer NOT focused +``` + +## Relevant Files + +### Files to Modify +- **`internal/tui/tui.go`** (lines 611-647): Refactor enter key handler logic + - Move footer focus check to top of conditional logic + - Remove tab-type-specific command execution branches + - Consolidate command execution into single path + +### Files Referenced (No Changes) +- **`internal/tui/autocomplete.go`**: Contains `AutocompleteInput.Focused()` method used for focus detection +- **`internal/tui/tabs.go`**: Defines `TabType` constants (TabTypeMain, TabTypeAgent, TabTypePlanning, TabTypeLog) +- **`internal/tui/planning_tab.go`**: Example of tab that needs enter forwarding when footer NOT focused +- **`internal/tui/footer.go`**: Footer management system + +## Team Orchestration + +This is a single-file refactoring with no dependencies: +- **Task 1**: Refactor enter key handler (can be completed independently) +- **Task 2**: Add test coverage (depends on Task 1) + +No parallel execution required - tasks must run sequentially. + +## Step-by-Step Task Breakdown + +### Task 1: Refactor Enter Key Handler Logic +**File**: `internal/tui/tui.go` (lines 611-647) +**Acceptance Criteria**: +1. Enter key handler checks `m.input.Focused()` as first condition +2. If footer focused: Execute command immediately (remove tab type checks) +3. If footer NOT focused: Forward to active tab's Update method +4. Preserve existing behavior for main tab (always has footer focused) +5. Preserve existing behavior for planning tab content interaction (enter forwarding when footer NOT focused) +6. All four tab types (Main, Planning, Agent, Log) execute commands when footer focused + +**Implementation Details**: +```go +case "enter": + // Refactored logic: Check footer focus FIRST + activeTab := m.tabManager.GetActiveTab() + + // If footer is focused, execute command regardless of tab type + if m.input.Focused() { + var cmd tea.Cmd + m.input, cmd = m.input.Update(msg) + + input := strings.TrimSpace(m.input.Value()) + m.input.SetValue("") + if input == "" { + return m, cmd + } + + return m.executeCommand(input) + } + + // Footer NOT focused: Forward to active tab for tab-specific handling + if activeTab != nil { + if cmd := m.tabManager.Update(msg); cmd != nil { + return m, cmd + } + } + return m, nil +``` + +**Key Changes**: +- Remove hardcoded `activeTab.Type() != TabTypeMain` check +- Remove nested `activeTab.Type() == TabTypePlanning && m.input.Focused()` check +- Single path for command execution when footer focused +- Single path for tab forwarding when footer NOT focused + +### Task 2: Add Test Coverage +**File**: Create `internal/tui/enter_key_test.go` +**Dependencies**: Task 1 +**Acceptance Criteria**: +1. Test command execution in main tab with footer focused +2. Test command execution in planning tab with footer focused +3. Test command execution in agent tab with footer focused +4. Test command execution in log tab with footer focused +5. Test planning tab message sending when footer NOT focused (forwarding behavior) +6. Verify autocomplete continues to work across all tab types + +**Implementation Details**: +```go +// Test structure for each tab type +func TestEnterKeyCommandExecutionInAllTabs(t *testing.T) { + tests := []struct { + name string + tabType TabType + footerFocused bool + inputValue string + expectCommand bool + }{ + {"main tab footer focused", TabTypeMain, true, "help", true}, + {"planning tab footer focused", TabTypePlanning, true, "status", true}, + {"agent tab footer focused", TabTypeAgent, true, "logs", true}, + {"log tab footer focused", TabTypeLog, true, "theme", true}, + {"planning tab content focused", TabTypePlanning, false, "message", false}, + } + // ... test implementation +} +``` + +## Validation Commands + +### Manual Testing +1. **Start the TUI**: `go run ./cmd/kiro-krew` +2. **Create an agent tab**: Execute `watch start` then `status` to spawn an agent +3. **Test agent tab**: Switch to agent tab, type `help` in footer, press Enter + - **Expected**: Help command executes and displays in main tab activity +4. **Create a log tab**: Execute `log view` +5. **Test log tab**: Type `status` in footer, press Enter + - **Expected**: Status display appears +6. **Test planning tab**: Execute `plan`, type command in footer, press Enter + - **Expected**: Command executes (existing behavior preserved) +7. **Test main tab**: Type `about` in footer, press Enter + - **Expected**: About dialog appears (existing behavior preserved) + +### Automated Testing +```bash +# Run tests +go test ./internal/tui -v -run TestEnterKey + +# Run all TUI tests +go test ./internal/tui -v + +# Check for regressions +go test ./... -v +``` + +### Acceptance Verification +All five acceptance criteria must pass: +1. ✅ Footer commands work in agent tabs +2. ✅ Footer commands work in log tabs +3. ✅ Main/planning tab behavior unchanged +4. ✅ Autocomplete still works (verified by existing autocomplete tests) +5. ✅ Enter forwarding for planning tab content still works (tab content sends messages, not commands) + +## Edge Cases and Considerations + +### 1. Autocomplete Menu Interaction +- Autocomplete menu should continue to intercept Enter key before command execution +- Current behavior: `m.input.Update(msg)` handles autocomplete selection +- Preserved in refactored code: autocomplete update happens before command execution check + +### 2. Empty Input Handling +- Pressing Enter with empty footer input should not execute command +- Current behavior: `if input == "" { return m, cmd }` +- Preserved in refactored code + +### 3. Tab Switching During Command Execution +- Commands execute in the context of the current model state +- Tab switching may occur as result of command (e.g., `status` switches to main tab) +- No special handling needed - existing behavior correct + +### 4. Planning Tab Message Sending +- Planning tab has dual enter behavior: + - Footer focused: Execute command + - Content focused: Send message to agent +- Footer focus state correctly distinguishes these cases +- Refactored logic preserves this by forwarding to tab when footer NOT focused + +### 5. Focus State Consistency +- Footer focus state managed by `AutocompleteInput.Focused()` method +- Focus changes handled by existing focus management system +- No changes needed to focus management + +## Non-Goals + +This specification does NOT include: +- Changes to command parsing or execution logic +- Changes to autocomplete behavior +- Changes to tab switching logic +- Changes to footer rendering or display +- Changes to focus management system +- New commands or command enhancements + +## Success Metrics + +1. **Functional**: All acceptance criteria pass manual and automated testing +2. **Behavioral**: No regressions in existing tab or command behavior +3. **Code Quality**: Refactored code is simpler and more maintainable than original +4. **User Experience**: Users can execute commands from any tab without tab switching + +## Implementation Notes + +### Why This Approach? +- **Minimal change**: Single file, ~30 lines of refactored code +- **Clear logic**: Footer focus check at top of conditional makes intent obvious +- **Maintainable**: Removes nested tab-type checks that obscured the logic +- **Extensible**: Future tab types automatically inherit correct command execution + +### Alternative Approaches Considered +1. **Add agent/log tab checks to existing logic**: Rejected because it perpetuates the wrong pattern (tab-type-first vs. focus-first) +2. **Modify tab Update methods**: Rejected because it scatters command execution logic across multiple files +3. **Create separate enter handler per tab type**: Rejected because it duplicates logic and increases maintenance burden + +The chosen approach (focus-first refactoring) is the simplest and most correct solution. diff --git a/internal/tui/enter_key_test.go b/internal/tui/enter_key_test.go new file mode 100644 index 0000000..8a97d4b --- /dev/null +++ b/internal/tui/enter_key_test.go @@ -0,0 +1,342 @@ +package tui + +import ( + "strings" + "testing" + + tea "charm.land/bubbletea/v2" + "github.com/jbrinkman/kiro-krew/internal/agent" + "github.com/jbrinkman/kiro-krew/internal/config" +) + +// TestEnterKeyCommandExecutionInAllTabs tests that command execution works +// when footer is focused, regardless of which tab is active +func TestEnterKeyCommandExecutionInAllTabs(t *testing.T) { + tests := []struct { + name string + tabType TabType + footerFocused bool + inputValue string + expectCommand bool + }{ + {"main tab footer focused", TabTypeMain, true, "help", true}, + {"planning tab footer focused", TabTypePlanning, true, "status", true}, + {"agent tab footer focused", TabTypeAgent, true, "theme", true}, + {"log tab footer focused", TabTypeLog, true, "about", true}, + {"planning tab content focused", TabTypePlanning, false, "message", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create test model with minimal configuration + m := createTestModelWithTab(t, tt.tabType) + + // Set footer focus state + if tt.footerFocused { + m.input.SetFocus(true) + } else { + m.input.SetFocus(false) + } + + // Set input value + m.input.SetValue(tt.inputValue) + + // Create enter key message + enterMsg := tea.KeyPressMsg(tea.Key{Code: 13}) + + // Process the enter key + updatedModel, _ := m.Update(enterMsg) + updated := updatedModel.(model) + + if tt.expectCommand { + // When footer is focused, input should be cleared after command execution + if updated.input.Value() != "" { + t.Errorf("Expected input to be cleared after command execution, got %q", updated.input.Value()) + } + + // The command should have been processed + // We can't directly verify command execution without exposing internals, + // but we verify that the input was cleared which indicates the command path was taken + } else { + // When footer is NOT focused, input should remain unchanged + // (enter was forwarded to tab for tab-specific handling) + if updated.input.Value() != tt.inputValue { + t.Errorf("Expected input value to remain %q when footer not focused, got %q", + tt.inputValue, updated.input.Value()) + } + } + }) + } +} + +// TestEnterKeyFooterFocusPriority verifies that footer focus state +// takes priority over tab type when determining command execution +func TestEnterKeyFooterFocusPriority(t *testing.T) { + tests := []struct { + name string + tabType TabType + focused bool + wantClear bool // Should input be cleared (command executed)? + }{ + {"main tab focused", TabTypeMain, true, true}, + {"agent tab focused", TabTypeAgent, true, true}, + {"planning tab focused", TabTypePlanning, true, true}, + {"log tab focused", TabTypeLog, true, true}, + {"planning tab unfocused", TabTypePlanning, false, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := createTestModelWithTab(t, tt.tabType) + m.input.SetFocus(tt.focused) + m.input.SetValue("test command") + + enterMsg := tea.KeyPressMsg(tea.Key{Code: 13}) + updatedModel, _ := m.Update(enterMsg) + updated := updatedModel.(model) + + inputCleared := updated.input.Value() == "" + if inputCleared != tt.wantClear { + if tt.wantClear { + t.Error("Expected input to be cleared (command executed)") + } else { + t.Error("Expected input to remain (enter forwarded to tab)") + } + } + }) + } +} + +// TestEnterKeyEmptyInput verifies that pressing enter with empty input +// does not execute a command but still clears autocomplete state +func TestEnterKeyEmptyInput(t *testing.T) { + m := createTestModelWithTab(t, TabTypeMain) + m.input.SetFocus(true) + m.input.SetValue("") // Empty input + + enterMsg := tea.KeyPressMsg(tea.Key{Code: 13}) + updatedModel, _ := m.Update(enterMsg) + updated := updatedModel.(model) + + // Input should remain empty + if updated.input.Value() != "" { + t.Errorf("Expected empty input to remain empty, got %q", updated.input.Value()) + } +} + +// TestEnterKeyWhitespaceOnly verifies that whitespace-only input +// is treated as empty and does not execute a command +func TestEnterKeyWhitespaceOnly(t *testing.T) { + whitespaceInputs := []string{ + " ", + "\t", + "\n", + " \t ", + } + + for _, input := range whitespaceInputs { + t.Run("whitespace: "+strings.ReplaceAll(input, " ", "·"), func(t *testing.T) { + m := createTestModelWithTab(t, TabTypeMain) + m.input.SetFocus(true) + m.input.SetValue(input) + + enterMsg := tea.KeyPressMsg(tea.Key{Code: 13}) + updatedModel, _ := m.Update(enterMsg) + updated := updatedModel.(model) + + // Input should be cleared (trimmed whitespace is empty) + if updated.input.Value() != "" { + t.Errorf("Expected whitespace-only input to be cleared, got %q", updated.input.Value()) + } + }) + } +} + +// TestEnterKeyAgentTabCommandExecution specifically tests the bug fix: +// commands should execute in agent tabs when footer is focused +func TestEnterKeyAgentTabCommandExecution(t *testing.T) { + m := createTestModelWithTab(t, TabTypeAgent) + m.input.SetFocus(true) + m.input.SetValue("help") + + enterMsg := tea.KeyPressMsg(tea.Key{Code: 13}) + updatedModel, _ := m.Update(enterMsg) + updated := updatedModel.(model) + + // Input should be cleared, indicating command was executed + if updated.input.Value() != "" { + t.Error("Agent tab should execute commands when footer is focused (bug #258)") + } +} + +// TestEnterKeyLogTabCommandExecution specifically tests the bug fix: +// commands should execute in log tabs when footer is focused +func TestEnterKeyLogTabCommandExecution(t *testing.T) { + m := createTestModelWithTab(t, TabTypeLog) + m.input.SetFocus(true) + m.input.SetValue("status") + + enterMsg := tea.KeyPressMsg(tea.Key{Code: 13}) + updatedModel, _ := m.Update(enterMsg) + updated := updatedModel.(model) + + // Input should be cleared, indicating command was executed + if updated.input.Value() != "" { + t.Error("Log tab should execute commands when footer is focused (bug #258)") + } +} + +// TestEnterKeyPlanningTabForwarding verifies that enter key is forwarded +// to planning tab when footer is NOT focused (for sending messages). +// Note: This test only verifies the negative case (command not executed). +// Full forwarding behavior is verified through integration tests. +func TestEnterKeyPlanningTabForwarding(t *testing.T) { + m := createTestModelWithTab(t, TabTypePlanning) + m.input.SetFocus(false) // Footer NOT focused + m.input.SetValue("Hello, planning agent") + + enterMsg := tea.KeyPressMsg(tea.Key{Code: 13}) + updatedModel, _ := m.Update(enterMsg) + updated := updatedModel.(model) + + // Input should NOT be cleared because enter was forwarded to planning tab + // for its internal message handling (not command execution) + if updated.input.Value() == "" { + t.Error("Planning tab should receive enter key for message sending when footer not focused") + } +} + +// TestEnterKeyCommandExecutionWithSequentialInput verifies that command execution +// works correctly when characters are typed sequentially before pressing enter +func TestEnterKeyCommandExecutionWithSequentialInput(t *testing.T) { + m := createTestModelWithTab(t, TabTypeMain) + m.input.SetFocus(true) + + // Type partial command to trigger autocomplete + m.input.SetValue("hel") + + // Update to trigger autocomplete suggestions + updatedModel, _ := m.Update(tea.KeyPressMsg(tea.Key{Text: "p", Code: 'p'})) + m = updatedModel.(model) + + // Now the input should be "help" and autocomplete may be showing suggestions + enterMsg := tea.KeyPressMsg(tea.Key{Code: 13}) + updatedModel, _ = m.Update(enterMsg) + updated := updatedModel.(model) + + // Command should execute, clearing the input + if updated.input.Value() != "" { + t.Error("Command should execute after sequential input") + } +} + +// TestEnterKeyAllTabTypes verifies the fix works across all four tab types +func TestEnterKeyAllTabTypes(t *testing.T) { + allTabTypes := []TabType{ + TabTypeMain, + TabTypeAgent, + TabTypePlanning, + TabTypeLog, + } + + for _, tabType := range allTabTypes { + t.Run(tabType.String(), func(t *testing.T) { + m := createTestModelWithTab(t, tabType) + m.input.SetFocus(true) + m.input.SetValue("test") + + enterMsg := tea.KeyPressMsg(tea.Key{Code: 13}) + updatedModel, _ := m.Update(enterMsg) + updated := updatedModel.(model) + + // All tab types should execute commands when footer is focused + if updated.input.Value() != "" { + t.Errorf("%s should execute commands when footer is focused", tabType) + } + }) + } +} + +// Helper function to create a test model with a specific active tab type +func createTestModelWithTab(t *testing.T, tabType TabType) model { + t.Helper() + + // Create minimal test configuration + cfg := &config.Config{ + LoadedTheme: createMinimalTheme(), + } + + // Create model using newModel - we need to provide nil for watcher and log files for testing + m := newModel(nil, agent.NewManager(cfg), cfg, nil, nil) + + // Create and add the appropriate tab type + switch tabType { + case TabTypeMain: + // Main tab already exists by default + case TabTypeAgent: + // Create and add an agent tab + agentTab := NewAgentTab("test-agent-123", m.manager, m.styles) + m.tabManager.AddTab(agentTab) + // Switch to the agent tab + tabs := m.tabManager.GetTabs() + for i, tab := range tabs { + if tab.Type() == TabTypeAgent { + m.tabManager.SetActiveTab(i) + break + } + } + case TabTypePlanning: + // Create and add a planning tab (without ACP connection for testing) + contextTracker := NewContextTracker() + planningTab := NewPlanningTabWithSession( + "test-plan-1", + "Test Plan", + m.styles, + contextTracker, + nil, // sessionManager not needed for this test + nil, // acpConn not needed for this test + ) + m.tabManager.AddTab(planningTab) + // Switch to the planning tab + tabs := m.tabManager.GetTabs() + for i, tab := range tabs { + if tab.Type() == TabTypePlanning { + m.tabManager.SetActiveTab(i) + break + } + } + case TabTypeLog: + // Create and add a log tab + logTab := NewLogTab("test-log", "info", 1000, m.styles) + m.tabManager.AddTab(logTab) + // Switch to the log tab + tabs := m.tabManager.GetTabs() + for i, tab := range tabs { + if tab.Type() == TabTypeLog { + m.tabManager.SetActiveTab(i) + break + } + } + } + + return m +} + +// Helper function to create a minimal theme for testing +func createMinimalTheme() *config.Theme { + theme := &config.Theme{} + theme.Colors.Primary = "#00AAFF" + theme.Colors.Secondary = "#0088CC" + theme.Colors.Success = "#00AA00" + theme.Colors.Warning = "#FFAA00" + theme.Colors.Error = "#FF0000" + theme.Colors.TextPrimary = "#FFFFFF" + theme.Colors.TextSecondary = "#CCCCCC" + theme.Colors.TextMuted = "#888888" + theme.Colors.Prompt = "#00AAFF" + theme.Colors.Separator = "#00AAFF" + theme.Colors.Activity = "#FFFFFF" + theme.Colors.Background = "#000000" + theme.Colors.Surface = "#111111" + return theme +} diff --git a/internal/tui/tabs.go b/internal/tui/tabs.go index 757610c..84a61cd 100644 --- a/internal/tui/tabs.go +++ b/internal/tui/tabs.go @@ -12,6 +12,22 @@ const ( TabTypeLog ) +// String returns the string representation of the TabType +func (t TabType) String() string { + switch t { + case TabTypeMain: + return "Main" + case TabTypeAgent: + return "Agent" + case TabTypePlanning: + return "Planning" + case TabTypeLog: + return "Log" + default: + return "Unknown" + } +} + // Tab interface defines the contract for all tab implementations type Tab interface { ID() string diff --git a/internal/tui/tui.go b/internal/tui/tui.go index a15f562..bcb119e 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -609,42 +609,32 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } return m, nil case "enter": - // Handle enter in main tab (console view), forward to other tabs + // Execute commands when footer is focused, forward enter to tabs otherwise. + // This allows command execution from any tab while preserving tab-specific + // enter handling (e.g., message sending in planning tabs). activeTab := m.tabManager.GetActiveTab() - if activeTab == nil || activeTab.Type() != TabTypeMain { - // If footer has focus in planning tab, route enter to footer for command execution - if activeTab != nil && activeTab.Type() == TabTypePlanning && m.input.Focused() { - var cmd tea.Cmd - m.input, cmd = m.input.Update(msg) - input := strings.TrimSpace(m.input.Value()) - m.input.SetValue("") - if input == "" { - return m, cmd - } + // If footer is focused, execute command regardless of tab type + if m.input.Focused() { + var cmd tea.Cmd + m.input, cmd = m.input.Update(msg) - return m.executeCommand(input) - } - // Forward to active tab (e.g., planning tab message sending) - if activeTab != nil { - if cmd := m.tabManager.Update(msg); cmd != nil { - return m, cmd - } + input := strings.TrimSpace(m.input.Value()) + m.input.SetValue("") + if input == "" { + return m, cmd } - return m, nil - } - - // Let autocomplete handle enter first (which will pass through) - var cmd tea.Cmd - m.input, cmd = m.input.Update(msg) - input := strings.TrimSpace(m.input.Value()) - m.input.SetValue("") - if input == "" { - return m, cmd + return m.executeCommand(input) } - return m.executeCommand(input) + // Footer NOT focused: Forward to active tab for tab-specific handling + if activeTab != nil { + if cmd := m.tabManager.Update(msg); cmd != nil { + return m, cmd + } + } + return m, nil default: // Forward key messages to active tab - removed TabTypeAgent restriction activeTab := m.tabManager.GetActiveTab()