v0.14.2

[yimsk]

Summary

• fix(app): route keys to view when filter input is active (App terminates on q character in filter #158)

Details

Global keybindings (q quit, ? help, R region, P profile, A AI, Ctrl+E compact) intercepted keypresses before views could handle them in filter mode. Added InputCapture.HasActiveInput() guard before the global key switch block, matching the existing pattern used for Esc/Backspace handling.

Closes #158

[claude]

Code Review: v0.14.2 - Fix Global Keybindings in Filter Mode

Summary

This PR successfully fixes issue #158 where global keybindings (q, ?, R, P, A, Ctrl+E) were intercepting keypresses before views could handle them in filter mode. The fix adds an InputCapture.HasActiveInput() guard before the global key switch block, matching the existing pattern used for Esc/Backspace handling.

Code Quality ✅

Strengths:

• Consistent Pattern: The fix follows the exact same pattern already established for Esc/Backspace handling (lines 286-298), which is excellent for maintainability
• Minimal Scope: The change is focused and surgical - only 8 lines added with zero deletions
• Code Structure: The implementation correctly checks for InputCapture interface, validates HasActiveInput(), and delegates to the view's Update method
• Type Safety: Proper type assertion with ok pattern for interface checks

Correctness ✅

Issue Fixed:

• Global keybindings will no longer intercept input when a filter is active
• The guard at lines 301-307 ensures views with active input receive ALL keypresses first
• This fixes the reported bug where typing 'q' in a filter would quit the app

Flow Analysis:

1. When filter is active, HasActiveInput() returns true
2. The new guard routes the keypress to the view
3. The view handles the key (adding it to the filter input)
4. Global keybindings are never reached

Testing ✅

Existing Test Coverage:
The codebase has excellent test coverage for this pattern:

• TestEscInFilterMode (app_test.go:102-140) tests the exact same guard pattern for Esc
• TestResourceBrowserHasActiveInput tests the InputCapture interface implementation
• Multiple views implement InputCapture with proper tests

Recommendation:
Consider adding a specific test case for this fix:

func TestGlobalKeysInFilterMode(t *testing.T) {
	app := newTestApp(t)
	
	mockView := &MockView{name: "ResourceBrowser", hasInput: true}
	app.currentView = mockView
	
	// Test 'q' key (quit) doesn't quit when filter is active
	qMsg := tea.KeyPressMsg{Code: 0, Text: "q"}
	_, cmd := app.Update(qMsg)
	
	// Should delegate to view, not quit
	if cmd != nil && cmd() != nil {
		if _, isQuit := cmd().(tea.QuitMsg); isQuit {
			t.Error("Expected 'q' to be handled by view in filter mode, not quit app")
		}
	}
	
	// Similar tests for ?, R, P, A, Ctrl+E...
}

Performance ✅

Impact: Negligible

• Single interface type assertion per keypress
• No allocations or additional overhead
• The check short-circuits early when input is active

Security ✅

No Security Concerns:

• No new attack surface introduced
• No user input sanitization issues
• Follows principle of least privilege (views handle their own input)

Architecture & Best Practices ✅

Strengths:

1. Consistency: Duplicates existing pattern rather than creating new abstractions
2. Separation of Concerns: Views remain responsible for their own input handling
3. Interface Design: Good use of optional InputCapture interface
4. Guard Clause Pattern: Clean early-return style

Code Location:
The placement at line 301 (after Esc/Backspace handling, before global keybindings) is perfect. This creates a clear priority order:

1. Warnings screen (if active)
2. Modals (if active)
3. Command mode (if active)
4. Esc/Backspace navigation
5. Filter/input mode ← New guard
6. Global keybindings
7. View delegation

Minor Observations

Comment Opportunity (optional):
Consider adding a brief comment to match the Esc handling block:

// Route all keypresses to view when it has active input (filter, search, etc.)
if ic, ok := a.currentView.(view.InputCapture); ok && ic.HasActiveInput() {

Consistency Note:
The modal handler (line 630) has similar logic for InputCapture. The pattern is consistent across the codebase.

Verdict: ✅ Approved

This is a high-quality bug fix that:

• Solves the reported issue completely
• Follows established patterns
• Maintains code consistency
• Has minimal risk
• Requires no breaking changes

Recommendation: Merge after adding a test case (suggested above) to prevent regression.

---

Great work! This fix demonstrates good understanding of the codebase architecture and follows excellent software engineering practices.