fix(tui): stop held peer message Allow/Deny from deadlocking Update

[PierrunoYT]

Summary

Answering Allow or Deny on a held cross-session (peer) message hung the TUI. The prompt's decide closure called m.runtimeMessageSink(peerDecisionMsg{...}) from inside Update (via resolvePermissionWithReason). The sink ends in program.Send, which in bubbletea v2.0.9 does p.msgs <- msg on an unbuffered channel whose only reader is the event loop currently running Update, so it never returned (and Ctrl+C queued behind it).

This change adds a decideCmd field to pendingPermissionPrompt. The peer prompt uses it to return a tea.Cmd that yields peerDecisionMsg, and resolvePermissionWithReason returns that command instead of calling the sink. The existing peerDecisionMsg handling in Update is unchanged. Agent permission prompts keep using decide, which is called from the agent goroutine, not from Update.

Not included: the issue's optional "debug guard against sink calls on the Update goroutine".

Linked issue

Fixes #1099

Note: #1099 does not currently carry the issue-approved label. Opening this anyway at the author's request; it can be held until the issue is approved.

Verification

• New regression test TestPeerApprovalDecisionDoesNotCallBlockingSinkFromUpdate installs a sink that blocks and resolves the held prompt with Allow and Deny. Without the fix it fails after the 5 s guard ("resolving a held peer prompt blocked on the runtime sink"); with the fix it passes.
• TestPermissionMismatchHoldsPeerMessageForExplicitDecision and TestPeerApprovalDecisionWaitsForCompletionBeforeOpeningNext previously read the decision from a capturing sink, which is why they could not see the bug. They now take the decision from the returned command, and the first also asserts the sink is not called. They fail on the old code and pass on the new.
• go build ./..., go vet ./internal/tui/, gofmt -l internal/tui, git diff HEAD --check clean; go test ./internal/tui/ passes.
• Not run locally: go test -race (cgo unavailable here), go test ./..., smoke, make targets. Relying on CI for those.

Checklist

• The linked issue already has the issue-approved label. (not yet)
• go build ./..., go vet ./..., and go test ./... pass locally. (build passes; vet and tests only for ./internal/tui)
• gofmt clean.
• Tests added/updated for the change (and run under -race where relevant). (tests added; -race not run locally)
• UI changes include screenshots or a short recording where possible. (no visual change)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Peer permission decisions now complete without waiting for the message sink, so allowing or denying a request won’t stall when message delivery is blocked.
  • After a decision, the next queued peer approval can be presented.

[Copilot]

Copilot review overview

🟡 Changes recommended

Asynchronous decisions can act after expiration or release, causing stale or duplicate delivery.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Reject stale peer decisions after approval expiry or release · New
• Clarify decideCmd is for TUI-owned prompt follow-up messages · New

What changed in this PR

Prevents held peer-message decisions from deadlocking the TUI event loop.

Changes:

• Returns peer decisions through a Bubble Tea command.
• Updates permission resolution to propagate decision commands.
• Adds Allow/Deny regression tests.

File	Description
internal/​tui/​peer_messages.go	Emits peer decisions through commands.
internal/​tui/​model.go	Supports command-based permission decisions.
internal/​tui/​peer_messages_test.go	Tests nonblocking decision handling.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: Twigpine/zero/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 21af12f8-4971-4053-8278-9c89ac161884

📥 Commits

Reviewing files that changed from the base of the PR and between 99721c7 and a72684b.

📒 Files selected for processing (3)

• internal/tui/model.go
• internal/tui/peer_messages.go
• internal/tui/peer_messages_test.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

Walkthrough

Peer permission decisions now return Bubble Tea commands instead of being sent synchronously through the runtime message sink. Permission resolution returns or batches the command, and tests check the resulting decision and message ID.

Changes

Peer permission resolution

Layer / File(s)	Summary
Return and verify peer decision commands internal/tui/model.go, internal/tui/peer_messages.go, internal/tui/peer_messages_test.go	Permission resolution captures and returns the decision command. The peer callback creates a command that emits the decision message. Tests inspect allow and deny commands, verify resolution completes with a blocked sink, and retain queued-approval checks.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Update as TUI Update
  participant Resolve as resolvePermissionWithReason
  participant BubbleTea as Bubble Tea runtime
  Update->>Resolve: Resolve peer permission
  Resolve-->>Update: Return decision command
  Update->>BubbleTea: Execute command
  BubbleTea-->>Update: Emit peerDecisionMsg

Loading

Suggested reviewers: anandh8x

Merge Risk: ⚪ Minimal · up to a7268

Peer decisions can be delivered without calling the blocking runtime sink during permission resolution. No issue requiring a change before merge was established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a7268

The fix preserves explicit approval and existing permission controls while removing the blocking delivery path. However, a deferred decision can arrive after its approval has expired or been released, potentially causing stale or duplicate work in the receiving session.

Retained concerns

• Medium · reliability · inferred: A deferred peer decision can survive expiry or release of its approval. If the terminal event is handled first, the later decision still clears peerPendingApproval without checking message identity and can deliver the old message. Release can therefore produce duplicate actionable work; expiry can be followed by delivery despite terminal cleanup. If another approval has opened meanwhile, the stale decision can also erase its active tracking. This weakens terminal-state consistency and failure containment for cross-session requests.

Security review details

Security Blast Radius

• inferred — The supported stale-decision outcome is confined to processing peer requests in the receiving session, including queued or repeated agent work. The inspected delivery change does not grant additional tool permissions; exposure beyond this session was not established.

Security Findings and Attack Paths

• inferred — No verified Security finding was supplied. A lifecycle failure remains plausible: resolve an approval, process expiry or release before its returned decision command, then consume the stale decision. The consumer neither checks current ownership nor rejects an already-terminal message before delivery. Successful exploitation of event timing was not demonstrated.

Trust Boundaries and Controls

• observed — The production path retains explicit approval for held messages and the same Allow/Deny enforcement handler. Denial does not call delivery. The routing signals identified test ranges, not a new external trust boundary.

Resilience and Maintainability Implications

• observed — ResolveHeld removes held records under a lock and returns without another receipt when the record is absent. This limits repeated service receipts, but it is not an atomic gate on TUI execution: delivery precedes the separately scheduled receipt command. Automatic release also removes held records before invoking the release callback.

Hardening Proposals

• proposed — Treat approval, expiry and release as competing terminal transitions. Correlate deferred decisions with active approval identity or generation, and reject stale decisions before clearing current state or delivering work.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing a TUI deadlock when Allow or Deny resolves a held peer message.
Linked Issues check	✅ Passed	Direct issue [#1099] requires that Allow and Deny for a held peer message do not call the blocking runtime sink from Update. openNextPeerApproval now supplies decideCmd, and the peer callback re…
Out of Scope Changes check	✅ Passed	The changes stay within [#1099]. They modify permission resolution and peer approval flow, and update peer approval regression tests. The test changes directly verify the deadlock fix and queue behavi…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

_{Comment @coderabbitai help to get the list of available commands.}