feat(tui): UX hardening for approvals, thinking, tool steps, and errors

README.md

@@ -178,13 +178,16 @@ untrusted-content boundary), so attachments go through the same security model
 as any other external input — bodek doesn't special-case them. (Saved sessions
 are resumed via `/sessions` or `^R`, not `@`.)
 
-When the agent requests approval for a dangerous operation, answer inline:
+When the agent requests approval for a dangerous operation, pick an outcome
+from the panel and confirm — typing never answers by accident:
 
 | Key | Action |
 |-----|--------|
-| `a` | Approve once |
-| `d` | Deny |
-| `t` | Trust this risk class for the session (when offered) |
+| `↑` / `↓` (or `←` / `→`) | Move the highlight (Approve / Deny / Trust class when offered) |
+| `⏎` | Confirm the highlighted option |
+| `Esc` | Deny (abort) |
+| `Tab` | Expand/collapse the full command & description text |
+| `PgUp` / `PgDn` / `^U` / `^D` | Scroll the transcript while the panel is open |
 
 ---
 

internal/tui/approval.go

@@ -4,18 +4,54 @@ import (
 	tea "github.com/charmbracelet/bubbletea"
 )
 
-// handleApprovalKey answers the pending approval (or quits); any other key
-// is swallowed while the panel waits for a decision.
+// approvalOption is one selectable outcome in the approval panel; action is
+// the protocol reply sent back to the server.
+type approvalOption struct {
+	label  string
+	action string
+}
+
+// approvalOptions lists the panel's outcomes in display order; trust is only
+// offered when the server allows it.
+func (m *Model) approvalOptions() []approvalOption {
+	opts := []approvalOption{
+		{"approve", "approve"},
+		{"deny", "deny"},
+	}
+	if m.approval.AllowTrust {
+		opts = append(opts, approvalOption{"trust class", "trust"})
+	}
+	return opts
+}
+
+// handleApprovalKey drives the pending approval: arrows move the highlight,
+// enter confirms it, esc denies, tab expands the full command/description,
+// and the transcript scroll keys keep working. Bare letters never decide — a
+// prompt typed mid-approval must not leak into a decision.
 func (m *Model) handleApprovalKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 	switch msg.String() {
-	case "a", "y":
-		return m, m.answer("approve")
-	case "d", "n":
-		return m, m.answer("deny")
-	case "t":
-		if m.approval.AllowTrust {
-			return m, m.answer("trust")
+	case "up", "left":
+		if m.apprSel > 0 {
+			m.apprSel--
 		}
+	case "down", "right":
+		if m.apprSel < len(m.approvalOptions())-1 {
+			m.apprSel++
+		}
+	case "enter":
+		return m, m.answer(m.approvalOptions()[m.apprSel].action)
+	case "esc":
+		return m, m.answer("deny")
+	case "tab":
+		m.apprExpanded = !m.apprExpanded
+		m.relayout() // the panel grows/shrinks with the full text
+	case "pgup", "pgdown", "ctrl+u", "ctrl+d":
+		var cmd tea.Cmd
+		m.vp, cmd = m.vp.Update(msg)
+		return m, cmd
+	case "ctrl+g":
+		m.vp.GotoBottom()
+		return m, nil
 	case "ctrl+c":
 		m.quitting = true
 		return m, tea.Quit
@@ -26,6 +62,8 @@ func (m *Model) handleApprovalKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 func (m *Model) answer(action string) tea.Cmd {
 	id := m.approval.ID
 	m.approval = nil
+	m.apprSel = 0
+	m.apprExpanded = false
 	m.status = "thinking"
 	m.relayout()
 	m.refresh()

internal/tui/approval_test.go

@@ -0,0 +1,187 @@
+package tui
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	tea "github.com/charmbracelet/bubbletea"
+	ws "golang.org/x/net/websocket"
+
+	"github.com/BackendStack21/bodek/internal/client"
+)
+
+// approvalRecorder builds a Model against a stand-in that records every
+// approval_response action it receives, so tests can assert the exact
+// protocol reply a key sequence produced.
+func approvalRecorder(t *testing.T) (*Model, chan string) {
+	t.Helper()
+	t.Setenv("HOME", t.TempDir())
+
+	actions := make(chan string, 4)
+	mux := http.NewServeMux()
+	mux.Handle("/ws", ws.Handler(func(c *ws.Conn) {
+		for {
+			var d []byte
+			if err := ws.Message.Receive(c, &d); err != nil {
+				return
+			}
+			var msg struct {
+				Type   string `json:"type"`
+				Action string `json:"action"`
+			}
+			if json.Unmarshal(d, &msg) == nil && msg.Type == "approval_response" {
+				actions <- msg.Action
+			}
+		}
+	}))
+	srv := httptest.NewServer(mux)
+	t.Cleanup(srv.Close)
+	cl, err := client.Dial("ws"+strings.TrimPrefix(srv.URL, "http")+"/ws", srv.URL, srv.URL, "")
+	if err != nil {
+		t.Fatalf("Dial: %v", err)
+	}
+	t.Cleanup(func() { cl.Close() })
+
+	m := New(cl, Options{Model: "m"})
+	m.Update(tea.WindowSizeMsg{Width: 100, Height: 30})
+	return m, actions
+}
+
+// awaitAction reads the next recorded approval_response action.
+func awaitAction(t *testing.T, actions chan string) string {
+	t.Helper()
+	select {
+	case a := <-actions:
+		return a
+	case <-time.After(2 * time.Second):
+		t.Fatal("no approval_response received")
+		return ""
+	}
+}
+
+// TestApprovalEnterConfirmsHighlight verifies that only enter on the
+// highlighted option answers the approval, with the same protocol replies as
+// before (approve / deny / trust).
+func TestApprovalEnterConfirmsHighlight(t *testing.T) {
+	m, actions := approvalRecorder(t)
+	cases := []struct {
+		name       string
+		allowTrust bool
+		keys       []string
+		want       string
+	}{
+		{"approve is the default highlight", false, []string{"enter"}, "approve"},
+		{"deny one down", false, []string{"down", "enter"}, "deny"},
+		{"trust at the bottom when offered", true, []string{"down", "down", "enter"}, "trust"},
+		{"left/right also navigate", true, []string{"right", "right", "left", "enter"}, "deny"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			m.handleEvent(client.Event{Type: "approval_request", ID: "apr", AllowTrust: tc.allowTrust})
+			var cmd tea.Cmd
+			for _, k := range tc.keys {
+				_, cmd = m.Update(key(k))
+			}
+			exec(cmd)
+			if m.approval != nil {
+				t.Fatal("approval still pending after enter")
+			}
+			if got := awaitAction(t, actions); got != tc.want {
+				t.Errorf("action = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
+// TestApprovalEscDenies verifies esc is the abort path: it denies even when
+// the highlight sits on another option.
+func TestApprovalEscDenies(t *testing.T) {
+	m, actions := approvalRecorder(t)
+	m.handleEvent(client.Event{Type: "approval_request", ID: "apr", AllowTrust: true})
+	m.Update(key("down")) // highlight elsewhere — esc must not confirm it
+	_, cmd := m.Update(key("esc"))
+	exec(cmd)
+	if m.approval != nil {
+		t.Fatal("approval still pending after esc")
+	}
+	if got := awaitAction(t, actions); got != "deny" {
+		t.Errorf("action = %q, want deny", got)
+	}
+}
+
+// TestApprovalExpandToggle verifies the panel starts collapsed to one
+// truncated line and tab reveals the full command/description text without
+// changing the total screen height.
+func TestApprovalExpandToggle(t *testing.T) {
+	m := newTestModel()
+	height := func() int { return strings.Count(m.View(), "\n") + 1 }
+	cmd := "git push origin " + strings.Repeat("some/really/long/path/", 8) + "end-marker"
+	m.handleEvent(client.Event{Type: "approval_request", ID: "apr",
+		Name: "shell", Command: cmd, Description: "push it"})
+
+	collapsed := plain(m.approvalPanel())
+	if !strings.Contains(collapsed, "…") {
+		t.Error("collapsed panel should truncate the command")
+	}
+	if strings.Contains(collapsed, "end-marker") {
+		t.Error("collapsed panel leaked the full command tail")
+	}
+	base := height()
+
+	m.Update(key("tab"))
+	if out := plain(m.approvalPanel()); !strings.Contains(out, "end-marker") {
+		t.Errorf("expanded panel should show the full command:\n%s", out)
+	}
+	if got := height(); got != base {
+		t.Errorf("view height changed when panel expanded: %d → %d rows", base, got)
+	}
+
+	m.Update(key("tab"))
+	if plain(m.approvalPanel()) != collapsed {
+		t.Error("second tab should restore the collapsed panel")
+	}
+}
+
+// TestApprovalScrollWhilePending verifies the transcript scroll keys keep
+// working while the approval panel waits for a decision.
+func TestApprovalScrollWhilePending(t *testing.T) {
+	m := newTestModel()
+	md := strings.Repeat("transcript line\n", 60)
+	// Pre-rendered verbatim (finalized messages use msg.rendered as-is).
+	m.msgs = append(m.msgs, message{role: roleAsst, content: md, rendered: md})
+	m.refresh()
+	if m.vp.TotalLineCount() <= m.vp.Height {
+		t.Fatal("test transcript should be taller than the viewport")
+	}
+	m.handleEvent(client.Event{Type: "approval_request", ID: "apr", Command: "rm x"})
+	if m.approval == nil {
+		t.Fatal("approval not set")
+	}
+
+	bottom := m.vp.YOffset
+	m.Update(key("pgup"))
+	if m.vp.YOffset >= bottom {
+		t.Errorf("pgup did not scroll while approval pending: yoffset=%d, was=%d", m.vp.YOffset, bottom)
+	}
+	m.Update(key("ctrl+g"))
+	if !m.vp.AtBottom() {
+		t.Errorf("ctrl+g did not jump to the latest while approval pending: yoffset=%d", m.vp.YOffset)
+	}
+	m.Update(key("pgup"))
+	up := m.vp.YOffset
+	m.Update(key("ctrl+d"))
+	if m.vp.YOffset <= up {
+		t.Errorf("ctrl+d did not scroll down while approval pending: yoffset=%d, was=%d", m.vp.YOffset, up)
+	}
+	m.Update(key("pgdown"))
+	if !m.vp.AtBottom() {
+		t.Errorf("pgdown did not return to the bottom: yoffset=%d", m.vp.YOffset)
+	}
+	if m.approval == nil {
+		t.Error("scrolling must not answer the approval")
+	}
+}

internal/tui/banner.go

@@ -27,8 +27,15 @@ var (
 // bindings — left-aligned with a gentle margin.
 func welcome(th theme, width int, cwd string) string {
 	var b strings.Builder
-	for _, line := range bannerArt {
-		b.WriteString(gradient(line, gradFrom, gradTo))
+	// The block art needs its own width plus the left padding below; narrower
+	// terminals get a one-line wordmark instead of wrapped garbage.
+	if artW := lipgloss.Width(bannerArt[0]); width >= artW+2 {
+		for _, line := range bannerArt {
+			b.WriteString(gradient(line, gradFrom, gradTo))
+			b.WriteByte('\n')
+		}
+	} else {
+		b.WriteString(th.logo.Render(gradient("⬡ bodek", gradFrom, gradTo)))
 		b.WriteByte('\n')
 	}
 	b.WriteByte('\n')
@@ -49,7 +56,7 @@ func welcome(th theme, width int, cwd string) string {
 		{"@ to attach", "attach files, e.g. @main.go"},
 		{"⏎ send", "^J newline  ·  ^T toggle thinking"},
 		{"^L clear", "↑/↓ scroll  ·  PgUp/PgDn page  ·  ^C quit"},
-		{"approvals", "a approve  ·  d deny  ·  t trust"},
+		{"approvals", "↑↓ select  ·  ⏎ confirm  ·  esc deny"},
 		{"tool steps", "^E toggle tool details  ·  --mouse to click-expand"},
 	}
 	const keyW = 11

internal/tui/banner_test.go

@@ -0,0 +1,37 @@
+package tui
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/charmbracelet/lipgloss"
+)
+
+// TestWelcomeNarrowFallback verifies the welcome splash drops the block art
+// for a one-line wordmark when the terminal is too narrow for it, and that
+// the compact render stays within the terminal width.
+func TestWelcomeNarrowFallback(t *testing.T) {
+	th := newTheme()
+	artW := lipgloss.Width(bannerArt[0])
+
+	wide := plain(welcome(th, artW+2, "/tmp"))
+	if !strings.Contains(wide, "██████") {
+		t.Error("banner at art width should show the block art")
+	}
+
+	narrow := plain(welcome(th, artW, "/tmp")) // one column short of art+padding
+	if strings.Contains(narrow, "██████") {
+		t.Error("narrow banner should drop the block art")
+	}
+	if !strings.Contains(narrow, "bodek") || !strings.Contains(narrow, "terminal interface") {
+		t.Errorf("compact banner should keep wordmark and tagline:\n%s", narrow)
+	}
+	// The box wraps its content at `width` and then adds its 2-column left
+	// padding (pre-existing at every width) — the fallback's job is that no
+	// line exceeds that.
+	for i, ln := range strings.Split(narrow, "\n") {
+		if w := lipgloss.Width(ln); w > artW+2 {
+			t.Errorf("line %d wraps past the rendered width (%d > %d): %q", i, w, artW+2, ln)
+		}
+	}
+}

internal/tui/coverage_test.go

@@ -244,19 +244,38 @@ func TestSubmitGuards(t *testing.T) {
 	}
 	m.disconn = true
 	m.ta.SetValue("hi")
-	cmd := m.submit()
-	if cmd == nil {
-		t.Error("submit while disconnected should arm the notice expiry")
+	if cmd := m.submit(); cmd != nil {
+		t.Error("submit while disconnected should be nil (the warning is sticky, no expiry to arm)")
 	}
 	if m.ta.Value() != "hi" {
 		t.Error("submit while disconnected must keep the draft")
 	}
-	exec(cmd) // fires the notice-expiry tick safely
 	if len(m.notices) == 0 {
 		t.Error("submit while disconnected should explain why nothing was sent")
 	}
 }
 
+// TestWrapText covers the approval panel's hard-wrap helper: degenerate
+// widths, empty and blank lines, exact fits, and unbreakable words.
+func TestWrapText(t *testing.T) {
+	cases := []struct {
+		in   string
+		n    int
+		want []string
+	}{
+		{"", 5, []string{""}},                             // empty input still claims its row
+		{"hello", 5, []string{"hello"}},                   // exact width
+		{"hello", 0, []string{"h", "e", "l", "l", "o"}},   // width floors at 1
+		{"abcdefghij", 4, []string{"abcd", "efgh", "ij"}}, // unbreakable word chunks
+		{"a\n\nb", 10, []string{"a", "", "b"}},            // blank lines survive
+	}
+	for _, tc := range cases {
+		if got := wrapText(tc.in, tc.n); strings.Join(got, "\n") != strings.Join(tc.want, "\n") {
+			t.Errorf("wrapText(%q, %d) = %q, want %q", tc.in, tc.n, got, tc.want)
+		}
+	}
+}
+
 func TestTinyHelpers(t *testing.T) {
 	if orDash("") != "—" || orDash("x") != "x" {
 		t.Error("orDash")