From 8f9a697c0373e5975521d1b163ae4475a0719132 Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Tue, 19 May 2026 23:20:23 -0400 Subject: [PATCH] feat(tui): auto close permission prompt when another client responds When two clients share a session and one of them answers a permission prompt, the other client now sees its modal close automatically as the resolution arrives, instead of being left holding a stale dialog over an already decided request. The initial pending notification is ignored so the modal it just opened is not immediately dismissed. Co-Authored-By: Charm Crush --- internal/ui/dialog/permissions.go | 6 ++ internal/ui/model/permission_test.go | 96 ++++++++++++++++++++++++++++ internal/ui/model/ui.go | 25 +++++--- 3 files changed, 119 insertions(+), 8 deletions(-) create mode 100644 internal/ui/model/permission_test.go diff --git a/internal/ui/dialog/permissions.go b/internal/ui/dialog/permissions.go index 4f158211514bfab5ac0ee4e857686b8105c359f3..4dfb9ba6655a863468a87de5cd1ce90faada6c70 100644 --- a/internal/ui/dialog/permissions.go +++ b/internal/ui/dialog/permissions.go @@ -224,6 +224,12 @@ func (*Permissions) ID() string { return PermissionsID } +// ToolCallID returns the tool call ID associated with this dialog's +// permission request. +func (p *Permissions) ToolCallID() string { + return p.permission.ToolCallID +} + // HandleMsg implements [Dialog]. func (p *Permissions) HandleMsg(msg tea.Msg) Action { switch msg := msg.(type) { diff --git a/internal/ui/model/permission_test.go b/internal/ui/model/permission_test.go new file mode 100644 index 0000000000000000000000000000000000000000..c209e211223bf9e09f26eaeff9098b54206456a2 --- /dev/null +++ b/internal/ui/model/permission_test.go @@ -0,0 +1,96 @@ +package model + +import ( + "testing" + + "github.com/charmbracelet/crush/internal/permission" + "github.com/charmbracelet/crush/internal/ui/dialog" + "github.com/stretchr/testify/require" +) + +// newTestUIForPermissions builds a UI with a chat, dialog overlay, and +// common context sufficient to exercise handlePermissionNotification. +func newTestUIForPermissions() *UI { + u := newTestUI() + u.dialog = dialog.NewOverlay() + return u +} + +func TestHandlePermissionNotification_RemoteGrantClosesDialog(t *testing.T) { + t.Parallel() + + u := newTestUIForPermissions() + perm := permission.PermissionRequest{ + ID: "perm-1", + ToolCallID: "tool-call-X", + ToolName: "bash", + } + u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm)) + require.True(t, u.dialog.ContainsDialog(dialog.PermissionsID)) + + u.handlePermissionNotification(permission.PermissionNotification{ + ToolCallID: "tool-call-X", + Granted: true, + }) + + require.False(t, u.dialog.ContainsDialog(dialog.PermissionsID), + "granted notification should close matching permissions dialog") +} + +func TestHandlePermissionNotification_RemoteDenyClosesDialog(t *testing.T) { + t.Parallel() + + u := newTestUIForPermissions() + perm := permission.PermissionRequest{ + ID: "perm-2", + ToolCallID: "tool-call-Y", + } + u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm)) + + u.handlePermissionNotification(permission.PermissionNotification{ + ToolCallID: "tool-call-Y", + Denied: true, + }) + + require.False(t, u.dialog.ContainsDialog(dialog.PermissionsID), + "denied notification should close matching permissions dialog") +} + +func TestHandlePermissionNotification_InitialPendingDoesNotClose(t *testing.T) { + t.Parallel() + + u := newTestUIForPermissions() + perm := permission.PermissionRequest{ + ID: "perm-3", + ToolCallID: "tool-call-Z", + } + u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm)) + + // The initial notification published by permission.Request is + // neither granted nor denied; it must not dismiss the dialog. + u.handlePermissionNotification(permission.PermissionNotification{ + ToolCallID: "tool-call-Z", + }) + + require.True(t, u.dialog.ContainsDialog(dialog.PermissionsID), + "initial pending notification must not close the dialog") +} + +func TestHandlePermissionNotification_DifferentToolCallIDDoesNotClose(t *testing.T) { + t.Parallel() + + u := newTestUIForPermissions() + perm := permission.PermissionRequest{ + ID: "perm-4", + ToolCallID: "tool-call-A", + } + u.dialog.OpenDialog(dialog.NewPermissions(u.com, perm)) + + u.handlePermissionNotification(permission.PermissionNotification{ + ToolCallID: "tool-call-B", + Granted: true, + }) + + require.True(t, u.dialog.ContainsDialog(dialog.PermissionsID), + "notification for unrelated tool call must not close the dialog") +} diff --git a/internal/ui/model/ui.go b/internal/ui/model/ui.go index 6b60b503715307e23cb68282b8976a52136e27d7..eea0d76b514f7421120120efc713bc556495af1e 100644 --- a/internal/ui/model/ui.go +++ b/internal/ui/model/ui.go @@ -3392,16 +3392,25 @@ func (m *UI) openPermissionsDialog(perm permission.PermissionRequest) tea.Cmd { // handlePermissionNotification updates tool items when permission state changes. func (m *UI) handlePermissionNotification(notification permission.PermissionNotification) { - toolItem := m.chat.MessageItem(notification.ToolCallID) - if toolItem == nil { - return + if toolItem := m.chat.MessageItem(notification.ToolCallID); toolItem != nil { + if permItem, ok := toolItem.(chat.ToolMessageItem); ok { + if notification.Granted { + permItem.SetStatus(chat.ToolStatusRunning) + } else { + permItem.SetStatus(chat.ToolStatusAwaitingPermission) + } + } } - if permItem, ok := toolItem.(chat.ToolMessageItem); ok { - if notification.Granted { - permItem.SetStatus(chat.ToolStatusRunning) - } else { - permItem.SetStatus(chat.ToolStatusAwaitingPermission) + // If this notification reflects a final resolution (granted or denied), + // dismiss any open permissions dialog whose tool call ID matches. This + // covers the case where another client resolved the request remotely. + if !notification.Granted && !notification.Denied { + return + } + if d := m.dialog.Dialog(dialog.PermissionsID); d != nil { + if perm, ok := d.(*dialog.Permissions); ok && perm.ToolCallID() == notification.ToolCallID { + m.dialog.CloseDialog(dialog.PermissionsID) } } }