diff --git a/internal/ui/dialog/dialog.go b/internal/ui/dialog/dialog.go index 15e0fa52f5ab7b181e5659e7526213d254f01931..dc5ce3913dc2813b960247975e653274a61f62c5 100644 --- a/internal/ui/dialog/dialog.go +++ b/internal/ui/dialog/dialog.go @@ -1,6 +1,8 @@ package dialog import ( + "time" + "charm.land/bubbles/v2/key" tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" @@ -47,9 +49,26 @@ type LoadingDialog interface { StopLoading() } +// Grace period constants for dialogs that open asynchronously and may +// receive in-flight keystrokes from a previously focused component. +const ( + // graceQuietPeriod is how long input must be quiet before the dialog + // arms. Each absorbed keystroke resets this timer. + graceQuietPeriod = 200 * time.Millisecond + // graceMaxDelay is the absolute ceiling: the dialog always arms after + // this duration regardless of input activity. Prevents auto-repeat + // from keeping the dialog disarmed indefinitely. + graceMaxDelay = 1500 * time.Millisecond +) + // Overlay manages multiple dialogs as an overlay. type Overlay struct { dialogs []Dialog + + // Grace period state for the front dialog. Only active when the + // dialog was opened via OpenDialogWithGrace. + graceOpenedAt time.Time + graceLastInputAt time.Time } // NewOverlay creates a new [Overlay] instance. @@ -77,6 +96,36 @@ func (d *Overlay) ContainsDialog(dialogID string) bool { // OpenDialog opens a new dialog to the stack. func (d *Overlay) OpenDialog(dialog Dialog) { d.dialogs = append(d.dialogs, dialog) + d.graceOpenedAt = time.Time{} + d.graceLastInputAt = time.Time{} +} + +// OpenDialogWithGrace opens a dialog with an input grace period. All +// keystrokes are absorbed until either the input has been quiet for +// graceQuietPeriod or graceMaxDelay has elapsed since opening, whichever +// comes first. Use this for dialogs that open asynchronously (e.g. +// permission prompts) where in-flight keystrokes from a previously +// focused component could act on the dialog before the user sees it. +func (d *Overlay) OpenDialogWithGrace(dialog Dialog) { + now := time.Now() + d.dialogs = append(d.dialogs, dialog) + d.graceOpenedAt = now + d.graceLastInputAt = now +} + +// inGracePeriod reports whether the front dialog is still within its +// input grace period. Returns false if no grace period is active. +func (d *Overlay) inGracePeriod() bool { + if d.graceOpenedAt.IsZero() { + return false + } + if time.Since(d.graceOpenedAt) >= graceMaxDelay { + return false + } + if time.Since(d.graceLastInputAt) >= graceQuietPeriod { + return false + } + return true } // CloseDialog closes the dialog with the specified ID from the stack. @@ -97,6 +146,15 @@ func (d *Overlay) CloseFrontDialog() { d.removeDialog(len(d.dialogs) - 1) } +func (d *Overlay) removeDialog(idx int) { + d.dialogs = append(d.dialogs[:idx], d.dialogs[idx+1:]...) + // Clear grace state when the front dialog changes. + if idx == len(d.dialogs) { + d.graceOpenedAt = time.Time{} + d.graceLastInputAt = time.Time{} + } +} + // Dialog returns the dialog with the specified ID, or nil if not found. func (d *Overlay) Dialog(dialogID string) Dialog { for _, dialog := range d.dialogs { @@ -133,6 +191,12 @@ func (d *Overlay) Update(msg tea.Msg) tea.Msg { return nil } + // Absorb keystrokes during the grace period for async dialogs. + if _, ok := msg.(tea.KeyPressMsg); ok && d.inGracePeriod() { + d.graceLastInputAt = time.Now() + return nil + } + idx := len(d.dialogs) - 1 // active dialog is the last one dialog := d.dialogs[idx] if dialog == nil { @@ -203,11 +267,3 @@ func (d *Overlay) Draw(scr uv.Screen, area uv.Rectangle) *tea.Cursor { } return cur } - -// removeDialog removes a dialog from the stack. -func (d *Overlay) removeDialog(idx int) { - if idx < 0 || idx >= len(d.dialogs) { - return - } - d.dialogs = append(d.dialogs[:idx], d.dialogs[idx+1:]...) -} diff --git a/internal/ui/dialog/overlay_test.go b/internal/ui/dialog/overlay_test.go new file mode 100644 index 0000000000000000000000000000000000000000..a610a112201faacf86697ac07eb463bf4a7a090e --- /dev/null +++ b/internal/ui/dialog/overlay_test.go @@ -0,0 +1,131 @@ +package dialog + +import ( + "testing" + "time" + + tea "charm.land/bubbletea/v2" + uv "github.com/charmbracelet/ultraviolet" + "github.com/stretchr/testify/require" +) + +// stubDialog is a minimal Dialog for testing Overlay behavior. +type stubDialog struct { + id string + received []tea.Msg +} + +func (s *stubDialog) ID() string { return s.id } +func (s *stubDialog) HandleMsg(msg tea.Msg) Action { + s.received = append(s.received, msg) + return nil +} +func (s *stubDialog) Draw(_ uv.Screen, _ uv.Rectangle) *tea.Cursor { return nil } + +func keyMsg(r rune) tea.KeyPressMsg { + return tea.KeyPressMsg{Code: r, Text: string(r)} +} + +// TestOverlay_GracePeriodSwallowsKeys verifies that all keystrokes +// arriving within the grace period after OpenDialogWithGrace are absorbed +// and never forwarded to the dialog. +func TestOverlay_GracePeriodSwallowsKeys(t *testing.T) { + t.Parallel() + + d := &stubDialog{id: "test"} + o := NewOverlay() + o.OpenDialogWithGrace(d) + + for _, r := range []rune{'a', 's', 'd', 'x', 'z'} { + o.Update(keyMsg(r)) + } + require.Empty(t, d.received, "no keys should reach the dialog during grace period") +} + +// TestOverlay_GracePeriodArmsAfterQuiet verifies that once input has been +// quiet for graceQuietPeriod, subsequent keys are forwarded normally. +func TestOverlay_GracePeriodArmsAfterQuiet(t *testing.T) { + t.Parallel() + + d := &stubDialog{id: "test"} + o := NewOverlay() + o.OpenDialogWithGrace(d) + + // Backdate so both deadlines have elapsed. + o.graceOpenedAt = time.Now().Add(-graceMaxDelay - time.Millisecond) + o.graceLastInputAt = time.Now().Add(-graceQuietPeriod - time.Millisecond) + + o.Update(keyMsg('a')) + require.Len(t, d.received, 1, "key after grace period should reach the dialog") +} + +// TestOverlay_GracePeriodBurstExtendsQuietWindow verifies that a sustained +// burst of keystrokes keeps resetting the quiet timer, but the fixed +// ceiling (graceMaxDelay) eventually arms the dialog regardless. +func TestOverlay_GracePeriodBurstExtendsQuietWindow(t *testing.T) { + t.Parallel() + + d := &stubDialog{id: "test"} + o := NewOverlay() + o.OpenDialogWithGrace(d) + + // Simulate keys arriving every 40ms (within graceQuietPeriod). + // Each one resets the quiet timer, but we stay under graceMaxDelay. + for range 5 { + o.graceLastInputAt = time.Now().Add(-40 * time.Millisecond) + o.Update(keyMsg('a')) + } + require.Empty(t, d.received, "burst within max delay should be absorbed") + + // Now exceed the max delay ceiling. Even though lastInputAt is recent, + // the fixed deadline forces arming. + o.graceOpenedAt = time.Now().Add(-graceMaxDelay - time.Millisecond) + o.graceLastInputAt = time.Now() // just typed, but max delay wins + o.Update(keyMsg('b')) + require.Len(t, d.received, 1, "key after max delay should reach dialog even during burst") +} + +// TestOverlay_OpenDialogWithoutGraceHasNoGuard verifies that dialogs +// opened via OpenDialog (without grace) receive keys immediately. +func TestOverlay_OpenDialogWithoutGraceHasNoGuard(t *testing.T) { + t.Parallel() + + d := &stubDialog{id: "test"} + o := NewOverlay() + o.OpenDialog(d) + + o.Update(keyMsg('a')) + require.Len(t, d.received, 1, "dialog without grace should receive keys immediately") +} + +// TestOverlay_GraceClearedOnClose verifies that closing the front dialog +// clears grace state so a subsequently opened dialog without grace is +// not affected. +func TestOverlay_GraceClearedOnClose(t *testing.T) { + t.Parallel() + + d1 := &stubDialog{id: "first"} + d2 := &stubDialog{id: "second"} + o := NewOverlay() + o.OpenDialogWithGrace(d1) + + // Close the grace dialog and open a normal one. + o.CloseFrontDialog() + o.OpenDialog(d2) + + o.Update(keyMsg('a')) + require.Len(t, d2.received, 1, "new dialog after grace close should receive keys immediately") +} + +// TestOverlay_NonKeyMessagesPassDuringGrace verifies that non-keypress +// messages (e.g. mouse, tick) are forwarded even during the grace period. +func TestOverlay_NonKeyMessagesPassDuringGrace(t *testing.T) { + t.Parallel() + + d := &stubDialog{id: "test"} + o := NewOverlay() + o.OpenDialogWithGrace(d) + + o.Update(tea.MouseWheelMsg{}) + require.Len(t, d.received, 1, "non-key messages should pass through during grace") +} diff --git a/internal/ui/dialog/permissions_test.go b/internal/ui/dialog/permissions_test.go new file mode 100644 index 0000000000000000000000000000000000000000..d72d8654ff31e606b84e5263b83fac168ac9d06d --- /dev/null +++ b/internal/ui/dialog/permissions_test.go @@ -0,0 +1,98 @@ +package dialog + +import ( + "testing" + + tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/crush/internal/permission" + "github.com/charmbracelet/crush/internal/ui/common" + "github.com/charmbracelet/crush/internal/ui/styles" + "github.com/stretchr/testify/require" +) + +func newTestPermissions(t *testing.T) *Permissions { + t.Helper() + s := styles.CharmtonePantera() + com := &common.Common{Styles: &s} + perm := permission.PermissionRequest{ + ID: "perm-test", + ToolCallID: "tool-call-test", + ToolName: "bash", + } + return NewPermissions(com, perm) +} + +// TestPermissions_ActionKeysResolve verifies that action keys produce the +// correct permission response. +func TestPermissions_ActionKeysResolve(t *testing.T) { + t.Parallel() + + tests := []struct { + key tea.KeyPressMsg + action PermissionAction + }{ + {keyMsg('a'), PermissionAllow}, + {keyMsg('A'), PermissionAllow}, + {keyMsg('d'), PermissionDeny}, + {keyMsg('D'), PermissionDeny}, + {keyMsg('s'), PermissionAllowForSession}, + {keyMsg('S'), PermissionAllowForSession}, + } + + for _, tc := range tests { + p := newTestPermissions(t) + action := p.HandleMsg(tc.key) + resp, ok := action.(ActionPermissionResponse) + require.Truef(t, ok, "key %q should produce ActionPermissionResponse", tc.key.Text) + require.Equal(t, tc.action, resp.Action) + } +} + +// TestPermissions_NavigationCyclesOptions verifies that tab and arrow keys +// cycle through the three permission options. +func TestPermissions_NavigationCyclesOptions(t *testing.T) { + t.Parallel() + + p := newTestPermissions(t) + require.Equal(t, 0, p.selectedOption) + + // Tab cycles forward. + p.HandleMsg(tea.KeyPressMsg{Code: tea.KeyTab}) + require.Equal(t, 1, p.selectedOption) + + p.HandleMsg(tea.KeyPressMsg{Code: tea.KeyTab}) + require.Equal(t, 2, p.selectedOption) + + // Wrap around. + p.HandleMsg(tea.KeyPressMsg{Code: tea.KeyTab}) + require.Equal(t, 0, p.selectedOption) + + // Left cycles backward. + p.HandleMsg(keyMsg('h')) + require.Equal(t, 2, p.selectedOption) +} + +// TestPermissions_EnterConfirmsSelection verifies that enter confirms the +// currently selected option. +func TestPermissions_EnterConfirmsSelection(t *testing.T) { + t.Parallel() + + p := newTestPermissions(t) + p.selectedOption = 1 // Allow for session. + + action := p.HandleMsg(tea.KeyPressMsg{Code: tea.KeyEnter}) + resp, ok := action.(ActionPermissionResponse) + require.True(t, ok) + require.Equal(t, PermissionAllowForSession, resp.Action) +} + +// TestPermissions_EscapeDenies verifies that escape denies the request. +func TestPermissions_EscapeDenies(t *testing.T) { + t.Parallel() + + p := newTestPermissions(t) + action := p.HandleMsg(tea.KeyPressMsg{Code: tea.KeyEscape}) + resp, ok := action.(ActionPermissionResponse) + require.True(t, ok) + require.Equal(t, PermissionDeny, resp.Action) +} diff --git a/internal/ui/model/permission_test.go b/internal/ui/model/permission_test.go index c209e211223bf9e09f26eaeff9098b54206456a2..58ddcf43977a1c84ab72661dc775d3b774b5ef73 100644 --- a/internal/ui/model/permission_test.go +++ b/internal/ui/model/permission_test.go @@ -25,7 +25,7 @@ func TestHandlePermissionNotification_RemoteGrantClosesDialog(t *testing.T) { ToolCallID: "tool-call-X", ToolName: "bash", } - u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm)) + u.dialog.OpenDialogWithGrace(dialog.NewPermissions(u.com, perm)) require.True(t, u.dialog.ContainsDialog(dialog.PermissionsID)) u.handlePermissionNotification(permission.PermissionNotification{ @@ -45,7 +45,7 @@ func TestHandlePermissionNotification_RemoteDenyClosesDialog(t *testing.T) { ID: "perm-2", ToolCallID: "tool-call-Y", } - u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm)) + u.dialog.OpenDialogWithGrace(dialog.NewPermissions(u.com, perm)) u.handlePermissionNotification(permission.PermissionNotification{ ToolCallID: "tool-call-Y", @@ -64,7 +64,7 @@ func TestHandlePermissionNotification_InitialPendingDoesNotClose(t *testing.T) { ID: "perm-3", ToolCallID: "tool-call-Z", } - u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm)) + u.dialog.OpenDialogWithGrace(dialog.NewPermissions(u.com, perm)) // The initial notification published by permission.Request is // neither granted nor denied; it must not dismiss the dialog. @@ -84,7 +84,7 @@ func TestHandlePermissionNotification_DifferentToolCallIDDoesNotClose(t *testing ID: "perm-4", ToolCallID: "tool-call-A", } - u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm)) + u.dialog.OpenDialogWithGrace(dialog.NewPermissions(u.com, perm)) u.handlePermissionNotification(permission.PermissionNotification{ ToolCallID: "tool-call-B", diff --git a/internal/ui/model/ui.go b/internal/ui/model/ui.go index 0c6102572cdc768ce0ebdf061a816eacb66af974..9dfe7227961247369c78ffc6c9636013bce6a704 100644 --- a/internal/ui/model/ui.go +++ b/internal/ui/model/ui.go @@ -1808,7 +1808,7 @@ func (m *UI) openAuthenticationDialog(provider catwalk.Provider, model config.Se return nil } - m.dialog.OpenDialog(dlg) + m.dialog.OpenDialogWithGrace(dlg) return cmd } @@ -3519,7 +3519,7 @@ func (m *UI) openPermissionsDialog(perm permission.PermissionRequest) tea.Cmd { } permDialog := dialog.NewPermissions(m.com, perm, opts...) - m.dialog.OpenDialog(permDialog) + m.dialog.OpenDialogWithGrace(permDialog) return nil }