diff --git a/internal/tui/model.go b/internal/tui/model.go index 14486764d..070da5fa9 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -308,6 +308,10 @@ type permissionRequestMsg struct { type pendingPermissionPrompt struct { request agent.PermissionRequest decide func(agent.PermissionDecision) + // cursor is the highlighted option index (into permissionOptions): 0=allow + // once (the resting default), 1=always, 2=deny. Moved by ↑/↓/Tab; confirmed + // by Enter or a click. The a/y/d hotkeys resolve directly and ignore it. + cursor int } // askUserRequestMsg is the TUI-loop equivalent of permissionRequestMsg: the @@ -696,7 +700,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } if m.pendingPermission != nil { - return m, nil + // Enter confirms the highlighted option (default: allow once); the + // a/y/d hotkeys and a click still resolve directly. + return m.confirmPermissionCursor() } if m.pendingAskUser != nil { return m.submitAskUserAnswer() @@ -733,6 +739,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { if m.transcriptDetailed { return m, nil } + if m.pendingPermission != nil { + return m.movePermissionCursor(-1), nil + } // shift+tab toggles the permission mode between Auto and Ask (Unsafe // is intentionally not reachable by a casual keypress — see // nextPermissionMode), but only when nothing modal is up: a permission @@ -761,6 +770,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { if m.transcriptDetailed { return m, nil } + if m.pendingPermission != nil { + return m.movePermissionCursor(1), nil + } if m.providerWizard != nil { return m.handleProviderWizardKey(msg) } @@ -788,6 +800,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { if m.transcriptDetailed { return m, nil } + if m.pendingPermission != nil { + return m.movePermissionCursor(1), nil + } if m.providerWizard != nil { return m.handleProviderWizardKey(msg) } @@ -818,6 +833,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { if m.transcriptDetailed { return m, nil } + if m.pendingPermission != nil { + return m.movePermissionCursor(-1), nil + } if m.providerWizard != nil { return m.handleProviderWizardKey(msg) } diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 8b1e33790..9f4b158c3 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -1079,7 +1079,7 @@ func TestPermissionRequestShowsFocusedPrompt(t *testing.T) { t.Fatalf("expected permission request to append one permission row, got %#v", next.transcript) } view := next.View() - for _, want := range []string{"write_file", "[a] allow", "[d] deny", "[y] always", "risk:high", "Creates or overwrites files."} { + for _, want := range []string{"write_file", "allow once", "[a]", "deny", "[d]", "always", "[y]", "risk:high", "Creates or overwrites files."} { assertContains(t, view, want) } } @@ -1145,16 +1145,18 @@ func TestPermissionPromptBlocksNormalSubmit(t *testing.T) { next = updated.(model) if cmd != nil { - t.Fatal("expected Enter to be ignored while permission prompt is active") + t.Fatal("expected permission confirm to resolve synchronously (no cmd)") } - if len(decisions) != 0 { - t.Fatalf("expected Enter not to choose a permission decision, got %#v", decisions) + // Enter confirms the highlighted option (default: allow once) — it must NOT + // submit the composer's pending text as a new prompt. + if len(decisions) != 1 || decisions[0] != permissionDecisionAllow { + t.Fatalf("expected Enter to confirm the default option (allow once), got %#v", decisions) } if transcriptContains(next.transcript, "second prompt") { t.Fatalf("permission prompt should block normal prompt submit, got %#v", next.transcript) } - if next.pendingPermission == nil { - t.Fatal("expected permission prompt to remain pending after Enter") + if next.pendingPermission != nil { + t.Fatalf("expected permission prompt to clear after confirm, got %#v", next.pendingPermission) } } diff --git a/internal/tui/permission_prompt.go b/internal/tui/permission_prompt.go new file mode 100644 index 000000000..2c5f61b8f --- /dev/null +++ b/internal/tui/permission_prompt.go @@ -0,0 +1,63 @@ +package tui + +import ( + tea "charm.land/bubbletea/v2" +) + +// permissionOption is one selectable choice in the permission popup. The slice +// order is both the on-screen order and the cursor index space; index 0 is the +// resting default highlight. +type permissionOption struct { + label string + hotkey string + choice permissionDecision +} + +// permissionOptions returns the ordered choices the popup offers. "Allow once" +// is first so it is the resting highlight (the common response a user attends +// to a prompt to give); "deny" is last. The hotkeys mirror handlePermissionKey. +func permissionOptions() []permissionOption { + return []permissionOption{ + {label: "allow once", hotkey: "a", choice: permissionDecisionAllow}, + {label: "always", hotkey: "y", choice: permissionDecisionAlwaysAllow}, + {label: "deny", hotkey: "d", choice: permissionDecisionDeny}, + } +} + +// clampPermissionCursor keeps a cursor index within the option range. +func clampPermissionCursor(cursor int) int { + n := len(permissionOptions()) + if cursor < 0 { + return 0 + } + if cursor >= n { + return n - 1 + } + return cursor +} + +// movePermissionCursor advances the highlighted option by delta, wrapping around +// the ends. A no-op when no permission prompt is pending. The cursor lives on the +// pending prompt (a pointer), mirroring how the picker's selection moves. +func (m model) movePermissionCursor(delta int) model { + if m.pendingPermission == nil { + return m + } + n := len(permissionOptions()) + cursor := (clampPermissionCursor(m.pendingPermission.cursor) + delta) % n + if cursor < 0 { + cursor += n + } + m.pendingPermission.cursor = cursor + return m +} + +// confirmPermissionCursor resolves the currently highlighted option. It is the +// Enter-key counterpart to the a/y/d hotkeys and a mouse click. +func (m model) confirmPermissionCursor() (tea.Model, tea.Cmd) { + if m.pendingPermission == nil { + return m, nil + } + option := permissionOptions()[clampPermissionCursor(m.pendingPermission.cursor)] + return m.resolvePermission(option.choice) +} diff --git a/internal/tui/permission_prompt_test.go b/internal/tui/permission_prompt_test.go new file mode 100644 index 000000000..9efb83d16 --- /dev/null +++ b/internal/tui/permission_prompt_test.go @@ -0,0 +1,96 @@ +package tui + +import ( + "context" + "strings" + "testing" + + tea "charm.land/bubbletea/v2" + + "github.com/Gitlawb/zero/internal/agent" +) + +func pendingPermissionModel(t *testing.T, decide func(agent.PermissionDecision)) model { + t.Helper() + m := newModel(context.Background(), Options{}) + m.pending = true + m.activeRunID = 7 + updated, _ := m.Update(permissionRequestMsg{ + runID: 7, + request: testPromptPermissionRequest(), + decide: decide, + }) + next := updated.(model) + if next.pendingPermission == nil { + t.Fatal("setup: expected a pending permission prompt") + } + return next +} + +func TestPermissionCursorDefaultsToAllowOnce(t *testing.T) { + m := pendingPermissionModel(t, func(agent.PermissionDecision) {}) + if m.pendingPermission.cursor != 0 { + t.Fatalf("default cursor = %d, want 0 (allow once)", m.pendingPermission.cursor) + } +} + +func TestPermissionCursorMovesAndEnterConfirms(t *testing.T) { + decisions := []permissionDecision{} + m := pendingPermissionModel(t, func(d agent.PermissionDecision) { + decisions = append(decisions, permissionDecision(d.Action)) + }) + // 0 →down 1 →down 2 →up 1 (always). + for _, key := range []rune{tea.KeyDown, tea.KeyDown, tea.KeyUp} { + updated, _ := m.Update(testKey(key)) + m = updated.(model) + } + if m.pendingPermission == nil || m.pendingPermission.cursor != 1 { + t.Fatalf("cursor after down,down,up = %v, want 1 (always)", m.pendingPermission) + } + updated, _ := m.Update(testKey(tea.KeyEnter)) + m = updated.(model) + if len(decisions) != 1 || decisions[0] != permissionDecisionAlwaysAllow { + t.Fatalf("enter on cursor 1 should resolve 'always', got %#v", decisions) + } + if m.pendingPermission != nil { + t.Fatal("prompt should clear after confirm") + } +} + +func TestPermissionCursorWrapsWithUp(t *testing.T) { + m := pendingPermissionModel(t, func(agent.PermissionDecision) {}) + updated, _ := m.Update(testKey(tea.KeyUp)) // 0 wraps to last (deny) + m = updated.(model) + if want := len(permissionOptions()) - 1; m.pendingPermission.cursor != want { + t.Fatalf("Up from 0 should wrap to %d, got %d", want, m.pendingPermission.cursor) + } +} + +func TestPermissionHotkeysStillResolveDirectly(t *testing.T) { + got := []permissionDecision{} + m := pendingPermissionModel(t, func(d agent.PermissionDecision) { + got = append(got, permissionDecision(d.Action)) + }) + if _, cmd := m.Update(testKeyText("d")); cmd != nil { // hotkey ignores the cursor + t.Fatal("'d' should resolve synchronously") + } + if len(got) != 1 || got[0] != permissionDecisionDeny { + t.Fatalf("'d' should resolve deny directly, got %#v", got) + } +} + +func TestPermissionRenderEmitsHighlightedClickableOffsets(t *testing.T) { + request := agent.PermissionRequest{ToolName: "bash"} + card, offsets := renderFocusedPermissionPrompt(request, 2, 60) // cursor on deny + if len(offsets) != len(permissionOptions()) { + t.Fatalf("offsets = %d, want %d", len(offsets), len(permissionOptions())) + } + lines := strings.Split(plainRender(t, card), "\n") + deny := offsets[2] + if deny < 0 || deny >= len(lines) || !strings.Contains(lines[deny], "deny") { + t.Fatalf("offset[2] (%d) should point at the deny line; lines=%#v", deny, lines) + } + if !strings.Contains(lines[deny], "▸") { + t.Fatalf("the highlighted (cursor) option line should carry ▸, got %q", lines[deny]) + } +} diff --git a/internal/tui/rendering.go b/internal/tui/rendering.go index 673949cf2..9fde8fb67 100644 --- a/internal/tui/rendering.go +++ b/internal/tui/rendering.go @@ -815,10 +815,11 @@ func wrapDetailBlock(detail string, width int) string { return strings.Join(lines, "\n") } -// renderFocusedPermissionPrompt draws the modal permission card: PERMISSION -// badge + risk on top, tool + reason body, then the key-chip action row. The -// keys themselves are handled in handlePermissionKey, unchanged. -func renderFocusedPermissionPrompt(request agent.PermissionRequest, width int) string { +// renderFocusedPermissionPrompt draws the modal permission card and reports the +// card-relative Y offset of each option line (in permissionOptions order) so the +// caller can register those lines as clickable. cursor is the highlighted option +// (default 0 = allow once); the a/y/d hotkeys still resolve directly. +func renderFocusedPermissionPrompt(request agent.PermissionRequest, cursor int, width int) (string, []int) { name := strings.TrimSpace(request.ToolName) if name == "" { name = "tool" @@ -844,14 +845,33 @@ func renderFocusedPermissionPrompt(request agent.PermissionRequest, width int) s lines = append(lines, fill(zeroTheme.muted).Render("scope: "+scope)) } - actions := zeroTheme.badge.Render(" [a] allow once ") + - fill(zeroTheme.ink).Render(" ") + - fill(zeroTheme.accent).Render("[y]") + fill(zeroTheme.ink).Render(" always ") + - fill(zeroTheme.red).Render("[d]") + fill(zeroTheme.ink).Render(" deny ") + - fill(zeroTheme.faint).Render("[esc] cancel run") - lines = append(lines, actions) + lines = append(lines, "") + + // Each option is its own line so a click anywhere on that row selects it (no + // per-column hit-testing). The highlighted row gets a ▸ marker and a reverse + // label; the rest stay quiet. styledBlockFill prepends exactly one top-border + // line, so an option at content index i renders at card line i+1 — the offset + // returned for click registration. + options := permissionOptions() + cursor = clampPermissionCursor(cursor) + offsets := make([]int, len(options)) + for index, option := range options { + offsets[index] = 1 + len(lines) + hotkey := fill(zeroTheme.faint).Render(" [" + option.hotkey + "]") + if index == cursor { + marker := fill(zeroTheme.accent).Render("▸ ") + label := zeroTheme.badge.Render(" " + option.label + " ") + lines = append(lines, marker+label+hotkey) + } else { + label := fill(zeroTheme.ink).Render(option.label) + lines = append(lines, " "+label+hotkey) + } + } + + lines = append(lines, "") + lines = append(lines, fill(zeroTheme.faint).Render("↑↓ move · enter or click to confirm · [esc] cancel run")) - return styledBlockFill(width, lines, zeroTheme.permBorder, zeroTheme.permBg) + return styledBlockFill(width, lines, zeroTheme.permBorder, zeroTheme.permBg), offsets } // renderFocusedAskUserPrompt draws the ask-user questionnaire in the same diff --git a/internal/tui/rendering_lime_test.go b/internal/tui/rendering_lime_test.go index a8652ce48..17fce30bb 100644 --- a/internal/tui/rendering_lime_test.go +++ b/internal/tui/rendering_lime_test.go @@ -1166,12 +1166,16 @@ func TestFocusedPermissionCardShowsBadgeRiskAndKeys(t *testing.T) { SideEffect: "write", Risk: sandbox.Risk{Level: sandbox.RiskMedium}, } - got := plainRender(t, renderFocusedPermissionPrompt(request, 80)) - for _, want := range []string{"PERMISSION", "risk: medium", "edit_file", "writes internal/agent/exec.go", "[a] allow once", "[y] always", "[d] deny", "[esc]"} { + card, offsets := renderFocusedPermissionPrompt(request, 0, 80) + got := plainRender(t, card) + for _, want := range []string{"PERMISSION", "risk: medium", "edit_file", "writes internal/agent/exec.go", "allow once", "[a]", "always", "[y]", "deny", "[d]", "[esc]"} { if !strings.Contains(got, want) { t.Fatalf("permission card = %q, missing %q", got, want) } } + if len(offsets) != len(permissionOptions()) { + t.Fatalf("offsets = %d, want one per option (%d)", len(offsets), len(permissionOptions())) + } } func TestPermissionPromptCollapsesAfterDecision(t *testing.T) { diff --git a/internal/tui/transcript_selection.go b/internal/tui/transcript_selection.go index e2f1b53e9..d5ef7a9ca 100644 --- a/internal/tui/transcript_selection.go +++ b/internal/tui/transcript_selection.go @@ -31,6 +31,11 @@ type transcriptSelectableLine struct { text string toggle bool live bool + // permOption marks a clickable permission-popup choice; permChoice is the + // decision a left-click on this row resolves. These rows carry no selectable + // text (they are buttons, not content). + permOption bool + permChoice permissionDecision } type transcriptCopiedMsg struct { @@ -165,7 +170,29 @@ func (m model) transcriptBodyItems(width int, emptyOverlay string) []transcriptB items = append(items, transcriptBlankBodyItem()) switch { case m.pendingPermission != nil: - items = append(items, transcriptBlockBodyItem(transcriptBodyItemPendingPrompt, -1, renderFocusedPermissionPrompt(m.pendingPermission.request, width))) + perm := m.pendingPermission + items = append(items, transcriptBodyItem{ + kind: transcriptBodyItemPendingPrompt, + rowIndex: -1, + heightCacheStable: false, // the highlight changes with the cursor + render: func(startBodyY int) transcriptBodyRenderedItem { + block, offsets := renderFocusedPermissionPrompt(perm.request, perm.cursor, width) + options := permissionOptions() + selectable := make([]transcriptSelectableLine, 0, len(offsets)) + for index, offset := range offsets { + if index >= len(options) { + break + } + selectable = append(selectable, transcriptSelectableLine{ + bodyY: startBodyY + offset, + rowIndex: -1, + permOption: true, + permChoice: options[index].choice, + }) + } + return transcriptBodyRenderedItem{lines: viewLines(block), selectable: selectable} + }, + }) case m.pendingAskUser != nil: items = append(items, transcriptBlockBodyItem(transcriptBodyItemPendingPrompt, -1, renderFocusedAskUserPrompt(*m.pendingAskUser, m.input.Value(), width))) default: @@ -579,6 +606,11 @@ func (m model) handleTranscriptSelectionMouse(msg tea.MouseMsg) (model, tea.Cmd, } return m, nil, false } + if line.permOption { + // A left-click on a permission-popup option resolves it directly. + next, cmd := m.resolvePermission(line.permChoice) + return next.(model), cmd, true + } if line.toggle { if line.live { m.streamingReasoningExpanded = !m.streamingReasoningExpanded