diff --git a/internal/tui/model.go b/internal/tui/model.go index 3c69814df..842567213 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -843,6 +843,11 @@ type permissionRequestMsg struct { type pendingPermissionPrompt struct { request agent.PermissionRequest decide func(agent.PermissionDecision) + // decideCmd is the Update-safe alternative to decide for prompts the TUI + // itself raises: decide forwards through runtimeMessageSink, which blocks on + // the program's unbuffered message channel when called from Update, so these + // prompts return a command that yields the follow-up message instead. + decideCmd func(agent.PermissionDecision) tea.Cmd // cursor is the highlighted option index (into permissionOptions): 0 is the // resting approval choice. Moved by ↑/↓/Tab; confirmed by Enter or a click. // Hotkeys resolve the matching request-provided option directly. @@ -4464,11 +4469,13 @@ func (m model) resolvePermissionWithReason(decision permissionDecision, reason s return m, nil } + resolved := agent.PermissionDecision{Action: decision, Reason: reason} if pending.decide != nil { - pending.decide(agent.PermissionDecision{ - Action: decision, - Reason: reason, - }) + pending.decide(resolved) + } + var decideCmd tea.Cmd + if pending.decideCmd != nil { + decideCmd = pending.decideCmd(resolved) } m.pendingPermission = nil // Time spent at the prompt is user wait, not provider silence. Restart the @@ -4478,9 +4485,10 @@ func (m model) resolvePermissionWithReason(decision permissionDecision, reason s if pending.request.ToolName == peerPermissionToolName { // Receipt delivery completes asynchronously. That completion advances // the peer queue after this prompt is fully settled. - return m, nil + return m, decideCmd } - return m.openNextPeerApproval() + next, cmd := m.openNextPeerApproval() + return next, tea.Batch(decideCmd, cmd) } func permissionDecisionReason(decision permissionDecision) string { diff --git a/internal/tui/peer_messages.go b/internal/tui/peer_messages.go index 8f88e997c..8906b1026 100644 --- a/internal/tui/peer_messages.go +++ b/internal/tui/peer_messages.go @@ -119,10 +119,11 @@ func (m model) openNextPeerApproval() (model, tea.Cmd) { } m.pendingPermission = &pendingPermissionPrompt{ request: request, - decide: func(decision agent.PermissionDecision) { - if m.runtimeMessageSink != nil { - m.runtimeMessageSink(peerDecisionMsg{message: message, allow: decision.Action == agent.PermissionDecisionAllow}) - } + // Resolved from Update, so the decision must come back as a command: + // runtimeMessageSink blocks on the event loop that is running Update. + decideCmd: func(decision agent.PermissionDecision) tea.Cmd { + decided := peerDecisionMsg{message: message, allow: decision.Action == agent.PermissionDecisionAllow} + return func() tea.Msg { return decided } }, } return m, nil diff --git a/internal/tui/peer_messages_test.go b/internal/tui/peer_messages_test.go index 75e187c57..fa34e4874 100644 --- a/internal/tui/peer_messages_test.go +++ b/internal/tui/peer_messages_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "strings" "testing" + "time" tea "charm.land/bubbletea/v2" "github.com/charmbracelet/x/ansi" @@ -125,19 +126,82 @@ func TestPermissionMismatchHoldsPeerMessageForExplicitDecision(t *testing.T) { } } - approvedModel, _ := next.resolvePermission(permissionDecisionAllow) + approvedModel, approveCmd := next.resolvePermission(permissionDecisionAllow) approved := approvedModel.(model) if approved.pendingPermission != nil { t.Fatal("approval prompt did not close") } + decision, ok := peerDecisionFromCmd(approveCmd) + if !ok || !decision.allow || decision.message.ID != "held-1" { + t.Fatalf("approval did not return a peer decision command: %#v", decision) + } select { case raw := <-messages: - decision, ok := raw.(peerDecisionMsg) - if !ok || !decision.allow || decision.message.ID != "held-1" { - t.Fatalf("decision = %#v", raw) - } + t.Fatalf("approval sent %#v through the runtime sink from Update", raw) default: - t.Fatal("approval did not enqueue a peer decision") + } +} + +// peerDecisionFromCmd runs cmd (flattening tea.Batch) and returns the first +// peerDecisionMsg it produces. +func peerDecisionFromCmd(cmd tea.Cmd) (peerDecisionMsg, bool) { + if cmd == nil { + return peerDecisionMsg{}, false + } + switch msg := cmd().(type) { + case peerDecisionMsg: + return msg, true + case tea.BatchMsg: + for _, inner := range msg { + if decision, ok := peerDecisionFromCmd(inner); ok { + return decision, true + } + } + } + return peerDecisionMsg{}, false +} + +// The runtime sink forwards to program.Send, which blocks on the unbuffered +// message channel the event loop reads between Update calls. Resolving a held +// peer prompt must therefore never call the sink from Update. +func TestPeerApprovalDecisionDoesNotCallBlockingSinkFromUpdate(t *testing.T) { + for _, tc := range []struct { + name string + decision permissionDecision + allow bool + }{ + {"allow", permissionDecisionAllow, true}, + {"deny", permissionDecisionDeny, false}, + } { + t.Run(tc.name, func(t *testing.T) { + release := make(chan struct{}) + t.Cleanup(func() { close(release) }) + m := newModel(context.Background(), Options{ + PermissionMode: agent.PermissionModeAsk, + RuntimeMessageSink: func(tea.Msg) { <-release }, + }) + m, _ = m.handlePeerMessage(peermsg.InboundMessage{ + ID: "held-blocking", From: peermsg.Peer{Ref: "11223344"}, Body: "hold me", RequiresApproval: true, + }) + + type result struct { + cmd tea.Cmd + } + done := make(chan result, 1) + go func() { + _, cmd := m.resolvePermission(tc.decision) + done <- result{cmd: cmd} + }() + select { + case res := <-done: + decision, ok := peerDecisionFromCmd(res.cmd) + if !ok || decision.allow != tc.allow || decision.message.ID != "held-blocking" { + t.Fatalf("decision = %#v ok=%v", decision, ok) + } + case <-time.After(5 * time.Second): + t.Fatal("resolving a held peer prompt blocked on the runtime sink") + } + }) } } @@ -170,7 +234,7 @@ func TestPeerApprovalDecisionWaitsForCompletionBeforeOpeningNext(t *testing.T) { second := peermsg.InboundMessage{ID: "second", From: peermsg.Peer{Ref: "22222222"}, Body: "second", RequiresApproval: true} m, _ = m.handlePeerMessage(first) m, _ = m.handlePeerMessage(second) - resolvedModel, _ := m.resolvePermission(permissionDecisionDeny) + resolvedModel, resolveCmd := m.resolvePermission(permissionDecisionDeny) resolved := resolvedModel.(model) if resolved.pendingPermission != nil { t.Fatal("next approval opened before peer decision completed") @@ -178,11 +242,8 @@ func TestPeerApprovalDecisionWaitsForCompletionBeforeOpeningNext(t *testing.T) { if len(resolved.peerApprovalQueue) != 1 { t.Fatalf("queue = %#v", resolved.peerApprovalQueue) } - var decision peerDecisionMsg - select { - case raw := <-messages: - decision = raw.(peerDecisionMsg) - default: + decision, ok := peerDecisionFromCmd(resolveCmd) + if !ok { t.Fatal("peer decision was not emitted") } next, _ := resolved.handlePeerDecision(decision.message, decision.allow)