diff --git a/README.md b/README.md index ee4160b..a18b0b6 100644 --- a/README.md +++ b/README.md @@ -231,7 +231,7 @@ The list nests three tiers — **day → project → session** — each folding The preview always shows **what that scope produced** — PRs, Jira issues, artifacts and plans, one row each. Selecting a date row shows the day's outputs; selecting a project row narrows to that project on that day. The sessions themselves are not listed in the pane: they are one row below in the list. -Every output row carries the session that produced it as an anchor (`a1b2c3 · ~/src/repo`). Focus the preview and press `Enter` on a row to land in that conversation **at the message where the output first appeared** — the digest tells you *what* came out, and the anchor is how you get to *how*. `o` opens the output itself (a PR, Jira issue or artifact in the browser), and `y` copies its URL or path. Outputs referenced from several sessions collapse to one row with a `+N` spread marker, anchored to the earliest session (where the work happened, not where it was later quoted) — and the jump lands in *that* session, at *its* first mention. +Every output row carries the session that produced it as an anchor (`a1b2c3 · ~/src/repo`). Focus the preview and press `Enter` on a row to land in that conversation **at the message where the output first appeared** — the digest tells you *what* came out, and the anchor is how you get to *how*. `o` opens the output itself (a PR, Jira issue or artifact in the browser), `y` copies its URL or path, and `x` lists every action that applies to the row (see [Output Row Actions](#output-row-actions-x)). Outputs referenced from several sessions collapse to one row with a `+N` spread marker, anchored to the earliest session (where the work happened, not where it was later quoted) — and the jump lands in *that* session, at *its* first mention. Sessions are bucketed by the calendar day of their **last** activity. A session that spans midnight appears once, under the day it was last active — it is never duplicated across dates. @@ -249,7 +249,25 @@ The per-session counterpart of the daily view: what this session produced, not w | Files Changed | `Edit`/`Write`/`MultiEdit`/`NotebookEdit` targets, collapsed per path with a write count (`Read` does not count) | | Scratchpad | Files in the session's scratchpad directory | -With the preview focused, `↑↓` moves the cursor, `y` copies the row's URL or path, and `Enter` opens it: external references go to the browser, and everything else jumps into the conversation at the entry that produced it. +With the preview focused, `↑↓` moves the cursor, `y` copies the row's URL or path, and `Enter` opens it: external references go to the browser, and everything else jumps into the conversation at the entry that produced it. `x` opens the row's full action menu — see below. + +#### Output Row Actions (`x`) + +Both output panes — the daily view's **Produced** list and the per-session **Outputs** digest — put every action for the row under the cursor behind `x`, the same modal-hint pattern the session browser uses. The menu lists **only what the row can actually do**, because a PR has no file to edit and a scratchpad file has no URL to open: + +| Key | Action | Offered when | +|-----|--------|--------------| +| `o` | Open in browser | The row has a URL (PR, Jira issue, artifact) | +| `↵` | Jump to first mention | The row records the transcript entry it came from | +| `↵` | Open the conversation | No entry was recorded (e.g. a plan slug inherited from a parent session), but the producing session is known | +| `e` | Open in `$EDITOR` | The row is a local file (Files Changed, Scratchpad, plan files, memory notes) | +| `y` | Copy | Always — the URL when there is one, the path otherwise | + +The hint box names the row it acts on (its section and title) so you can see what you are about to do it to. `Enter`, `o` and `y` keep working directly without the menu — `x` is the discoverable surface, not a replacement for the fast path. Letters follow your `actions` keymap (`edit`, `copy_path`), so rebinding those rebinds these. + +#### Switching the Preview (`p`) + +`p` opens the preview-mode menu — `v`:conv `s`:stats `m`:mem `x`:scratch `t`:tasks `a`:agents `w`:workflows `c`:contexts `r`:refs `o`:outputs `l`:live — from **either side**, whether the cursor is in the list or the preview has focus. It is the letter-based counterpart to the number keys (`0`-`9`), and follows the same rule about honesty: on a date row or a day-scoped project row the menu does not open, because those rows always render that scope's outputs and cannot honor any preview mode. The number keys already refuse there for the same reason. #### Subagent and Workflow Support @@ -353,7 +371,8 @@ Multi-select plugin components and press `t` to launch an isolated Claude sessio | `[` / `]` | Adjust split ratio | | `Space` | Multi-select toggle | | `1-9` | Number key shortcuts (configurable) | -| `x` | Actions menu (delete, move, resume, fork, URLs, files, ...) | +| `p` | Preview-mode menu (works from the list *and* the focused preview) | +| `x` | Actions menu (delete, move, resume, fork, URLs, files, ...) — on a focused outputs pane, the row's own actions | | `v` | Views menu (stats/config/plugins) | | `:` | Command mode | | `Ctrl+S` | Cross-session search | diff --git a/internal/tui/app.go b/internal/tui/app.go index 2f1d84f..1b7716c 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -1767,7 +1767,11 @@ func (a *App) View() string { if a.actionsMenu && a.state == viewSessions { hintBox := a.renderActionsHintBox() content = overlayCenteredModal(content, hintBox, a.width, ContentHeight(a.height), modalOptions{paddingX: 2, paddingY: 1, maxWidth: max(a.width-8, 28), maxHeight: max(ContentHeight(a.height)-4, 8)}) - help = formatHelp("x:actions — pick an action") + if a.outputsPreviewActionsActive() { + help = formatHelp("x:actions — pick an action for this output") + } else { + help = formatHelp("x:actions — pick an action") + } } if a.sessPageMenu && a.state == viewSessions { @@ -1979,6 +1983,13 @@ func (a *App) handleSessionKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return a.handleActionsMenu(key) } + // Preview-page menu ("p" prefix). Intercepted here with the other overlays + // rather than inside the focused-preview block, because `p` now opens it + // from the list side too. + if a.sessPageMenu { + return a.handleSessPageMenu(key) + } + // State-filter toggle menu ("s" prefix) if a.stateMenu { return a.handleStateMenu(key) @@ -2193,6 +2204,21 @@ func (a *App) handleSessionKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { } return a, nil case km.Session.Actions: + // A focused outputs digest owns `x` for the row under its cursor. This + // comes first: without it, a day-scoped project row would fall into the + // project arm below and silently multi-select the whole project — an + // action about the LIST while the user is looking at the PANE. + if a.outputsPreviewActionsActive() { + if _, ok := a.outputActionTarget(); !ok { + // Focused on a digest with nothing in it. Say so rather than + // falling through to the list's session actions, which would act + // on a row the user is not looking at. + a.copiedMsg = "Nothing produced to act on" + return a, nil + } + a.actionsMenu = true + return a, nil + } if a.hasMultiSelection() { a.actionsMenu = true return a, nil @@ -2394,18 +2420,26 @@ func (a *App) handleSessionKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { // Focused preview: custom conversation nav or simple scroll if sp.Focus && sp.Show { - if a.sessPageMenu { - return a.handleSessPageMenu(key) - } - if key == "p" { - a.sessPageMenu = true - return a, nil - } if m, cmd, handled := a.handleFocusedPreviewKeys(sp, key); handled { return m, cmd } } + // Preview-page menu. Reachable from BOTH sides: the common position is the + // cursor in the LIST wanting to change what the right pane shows, and + // gating this on preview focus made the combo look like it did not exist. + // Suppressed on rows that have no preview modes at all — see + // rowSupportsPreviewModes, which the number keys already honor. Two + // mechanisms for one thing must not disagree. + if key == "p" { + if !a.rowSupportsPreviewModes() { + a.copiedMsg = "This row always shows what it produced" + return a, nil + } + a.sessPageMenu = true + return a, nil + } + // List boundary (up/down always navigate list, scroll preview at edges) if !sp.Focus && sp.HandleListBoundary(key) { return a, a.schedulePreviewUpdate() @@ -3744,6 +3778,16 @@ func (a *App) sessionPreviewActionsActive() bool { func (a *App) handleActionsMenu(key string) (tea.Model, tea.Cmd) { a.actionsMenu = false a.copiedMsg = "" + // A focused outputs digest acts on the row under its cursor, not on a + // session in the list. Checked before every other arm for the same reason + // the `x` case is: the row may also be a project row, whose session actions + // would otherwise win. + if a.outputsPreviewActionsActive() { + if row, ok := a.outputActionTarget(); ok { + return a.runOutputAction(row, key) + } + return a, nil + } if a.sessionPreviewActionsActive() { switch key { case "c": @@ -5226,10 +5270,13 @@ func (a *App) renderSessionSplit() string { a.refreshConvPreview() } else if a.sessPreviewMode == sessPreviewOutputs && !isRemoteSetup { // Re-render the digest at the new width from already-collected rows. - // A day row keeps its own summary pane (selectedSession would hand us - // an arbitrary child), and this path must never dispatch — View - // cannot deliver a cmd, so arming a latch here would strand the pane. - if _, isDay := a.selectedDay(); !isDay { + // A row that owns the day pane keeps its own summary (selectedSession + // would hand us an arbitrary child), and this path must never + // dispatch — View cannot deliver a cmd, so arming a latch here would + // strand the pane. selectedOwnsDayPane, not selectedDay: a day-scoped + // PROJECT row owns that pane too, and checking only date rows let a + // resize overwrite its "Produced" list with a child session's digest. + if !a.selectedOwnsDayPane() { a.sessOutputsCacheKey = "" if sess, ok := a.selectedSession(); ok { a.refreshOutputsPreviewLayout(sess) @@ -7654,6 +7701,9 @@ func (a *App) renderActionsHintBox() string { akm := a.keymap.Actions var lines []string + if a.outputsPreviewActionsActive() { + return a.renderOutputActionsHintBox() + } if a.hasMultiSelection() && !a.sessionPreviewActionsActive() { header := fmt.Sprintf("%d selected", len(a.selectedSet)) lines = append(lines, lipgloss.NewStyle().Bold(true).Foreground(colorPrimary).Render(header)) diff --git a/internal/tui/daypane.go b/internal/tui/daypane.go index 8034b63..464ba2c 100644 --- a/internal/tui/daypane.go +++ b/internal/tui/daypane.go @@ -184,7 +184,7 @@ func (a *App) renderOutputsPane(title, subtitle, summary string, rows []dayOutpu heading := fmt.Sprintf("Produced (%d)", len(rows)) if len(rows) > 0 && a.sessSplit.Focus { - heading += " ↵:jump to first mention o:open y:copy" + heading += " ↵:jump to first mention o:open y:copy x:actions" } sb.WriteString(section.Render(heading) + "\n") @@ -215,7 +215,7 @@ func (a *App) renderOutputsPane(title, subtitle, summary string, rows []dayOutpu // belongs to the list (it folds the row); focused, the keys are this pane's, // and saying otherwise sent people to the wrong action. if a.sessSplit.Focus { - sb.WriteString(dimStyle.Render("↵ jumps to where it first appeared • o opens it • y copies • ↑↓ moves between outputs")) + sb.WriteString(dimStyle.Render("↵ jumps to where it first appeared • o opens it • y copies • x lists every action for the row • ↑↓ moves between outputs")) } else { sb.WriteString(dimStyle.Render("↵/o folds this row • tab focuses this pane")) } diff --git a/internal/tui/daypane_test.go b/internal/tui/daypane_test.go index 04f02d7..af0736c 100644 --- a/internal/tui/daypane_test.go +++ b/internal/tui/daypane_test.go @@ -298,3 +298,162 @@ func selectedConvEntryUUID(a *App) string { } return a.conv.messages[ci.merged.startIdx].UUID } + +// TestDayPaneActionsMenuActsOnTheRow guards the day pane's half of the x-actions +// work: with the pane focused, `x` must act on the OUTPUT under the cursor, not +// on the day's sessions. A day-scoped project row is the trap — the plain `x` +// path multi-selects the whole project, which is an action about the list while +// the user is looking at the pane. +func TestDayPaneActionsMenuActsOnTheRow(t *testing.T) { + sessions := []session.Session{{ + ID: "maker", ShortID: "maker", ProjectPath: "/tmp/repo-a", ProjectName: "repo-a", + ModTime: dayOf(0), + Refs: []session.SessionRef{{ + Kind: session.RefPR, Label: "sendbird/ccx#5", + URL: "https://github.com/sendbird/ccx/pull/5", Resolved: true, + }}, + }} + app := dayPaneApp(t, sessions) + + if !app.outputsPreviewActionsActive() { + t.Fatal("a focused day pane must own the x actions menu") + } + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'x'}}) + got := m.(*App) + if !got.actionsMenu { + t.Fatal("x did not open the actions menu on the focused day pane") + } + if len(got.selectedSet) > 0 { + t.Errorf("x multi-selected %d sessions instead of acting on the row", len(got.selectedSet)) + } + box := got.renderActionsHintBox() + if !strings.Contains(box, "sendbird/ccx#5") { + t.Errorf("the menu does not name the row it acts on:\n%s", box) + } + if strings.Contains(box, ":delete") { + t.Errorf("the session actions menu rendered over the day pane:\n%s", box) + } + + var opened string + got.openURL = func(u string) error { opened = u; return nil } + m, _ = got.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'o'}}) + if opened != "https://github.com/sendbird/ccx/pull/5" { + t.Errorf("x→o opened %q, want the row's PR", opened) + } + if m.(*App).actionsMenu { + t.Error("the menu should close after a key is picked") + } +} + +// TestDayPaneActionsAnchorToTheRowsSession pins the day pane's extra field: the +// row's anchor session, which the per-session digest does not need. Without it, +// the uuid-less fallback ("open the conversation") could not be offered. +func TestDayPaneActionsAnchorToTheRowsSession(t *testing.T) { + app := newTestApp(nil) + app.sessGroupMode = groupDaily + app.dayOutputRows = []dayOutputRow{{ + out: session.SessionOutput{Kind: session.OutputPlan, Title: "some-plan"}, + sessID: "planner", + }} + app.dayOutputsCursor = 0 + + acts := app.outputActionsFor(app.dayOutputRows[0].out, app.dayOutputRows[0].sessID != "") + var kinds []outputActionKind + for _, a := range acts { + kinds = append(kinds, a.kind) + } + if len(kinds) != 1 || kinds[0] != outputActionSession { + t.Fatalf("a uuid-less plan slug with an anchor should offer exactly the conversation, got %+v", acts) + } +} + +// TestDayPaneActionsOnProjectRow pins the day-scoped project row, which owns the +// same pane (selectedOwnsDayPane) but is a projectItem — the arm the plain `x` +// path would otherwise route into. +func TestDayPaneActionsOnProjectRow(t *testing.T) { + sessions := []session.Session{{ + ID: "a1", ShortID: "a1", ProjectPath: "/tmp/repo-a", ProjectName: "repo-a", + ModTime: dayOf(0), + Refs: []session.SessionRef{{ + Kind: session.RefPR, Label: "sendbird/ccx#1", + URL: "https://github.com/sendbird/ccx/pull/1", Resolved: true, + }}, + }} + app := newTestApp(sessions) + app.sessGroupMode = groupDaily + app.rebuildSessionList() + + idx := -1 + for i, item := range app.sessionList.VisibleItems() { + if pi, ok := item.(projectItem); ok && pi.dayKey != "" { + idx = i + break + } + } + if idx < 0 { + t.Fatal("expected a day-scoped project row in the daily tree") + } + app.sessionList.Select(idx) + app.sessSplit.Show = true + app.sessSplit.Focus = true + _ = app.updateSessionPreview() + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'x'}}) + got := m.(*App) + if len(got.selectedSet) > 0 { + t.Errorf("x on a focused day-project pane multi-selected %d sessions instead of acting on the row", len(got.selectedSet)) + } + if box := got.renderActionsHintBox(); !strings.Contains(box, "sendbird/ccx#1") { + t.Errorf("the menu does not name the row it acts on:\n%s", box) + } +} + +// TestDayProjectPaneSurvivesResize guards a bug found while wiring the actions +// menu: the resize branch in renderSessionSplit guarded only selectedDay(), so +// on a day-scoped PROJECT row — which owns the same pane — a window resize fell +// through to the per-session digest path and overwrote the "Produced" list with +// an arbitrary child session's outputs. +func TestDayProjectPaneSurvivesResize(t *testing.T) { + sessions := []session.Session{{ + ID: "a1", ShortID: "a1", ProjectPath: "/tmp/repo-a", ProjectName: "repo-a", + ModTime: dayOf(0), + Refs: []session.SessionRef{{ + Kind: session.RefPR, Label: "sendbird/ccx#1", + URL: "https://github.com/sendbird/ccx/pull/1", Resolved: true, + }}, + }} + app := newTestApp(sessions) + app.sessGroupMode = groupDaily + // The digest mode is what makes the resize branch reachable; a day-scoped + // row renders the day pane regardless, which is exactly the mismatch. + app.sessPreviewMode = sessPreviewOutputs + app.rebuildSessionList() + + idx := -1 + for i, item := range app.sessionList.VisibleItems() { + if pi, ok := item.(projectItem); ok && pi.dayKey != "" { + idx = i + break + } + } + if idx < 0 { + t.Fatal("expected a day-scoped project row in the daily tree") + } + app.sessionList.Select(idx) + app.sessSplit.Show = true + app.sessSplit.Focus = true + _ = app.updateSessionPreview() + + if before := app.sessSplit.Preview.View(); !strings.Contains(before, "Produced") { + t.Fatalf("fixture should start on the day-project pane, got:\n%s", before) + } + + m, _ := app.Update(tea.WindowSizeMsg{Width: 140, Height: 44}) + got := m.(*App) + _ = got.View() // the resize branch lives in the render path + + if after := got.sessSplit.Preview.View(); !strings.Contains(after, "Produced") { + t.Errorf("resize replaced the day-project pane with a session digest:\n%s", after) + } +} diff --git a/internal/tui/help.go b/internal/tui/help.go index c6ea0f6..5fb9e5d 100644 --- a/internal/tui/help.go +++ b/internal/tui/help.go @@ -74,24 +74,27 @@ func (a *App) sessHelpLine() string { sk := a.keymap.Session var h string if !a.sessSplit.Show { - h = fmtKey(sk.Open, "open") + " " + fmtKey(sk.Actions, "actions") + " →:preview " + fmtKey(sk.Search, "search") + h = fmtKey(sk.Open, "open") + " " + fmtKey(sk.Actions, "actions") + " →:preview p:page " + fmtKey(sk.Search, "search") } else if a.sessSplit.Focus { - switch a.sessPreviewMode { - case sessPreviewConversation: + switch { + case a.outputsPreviewActionsActive(): + // Both digests: the row under the cursor is what the keys act on. + h = "↑↓:nav ↵:jump o:open " + fmtKey(sk.Actions, "actions") + " ←:unfocus p:page" + case a.sessPreviewMode == sessPreviewConversation: h = "↑↓:nav c:full " + fmtKey(sk.Open, "jump") + " ←:unfocus tab:mode" - case sessPreviewAgents: + case a.sessPreviewMode == sessPreviewAgents: h = "↑↓:nav " + fmtKey(sk.Open, "jump") + " ←:unfocus tab:mode" - case sessPreviewWorkflows: + case a.sessPreviewMode == sessPreviewWorkflows: h = "↑↓:agent ↵:transcript ←:unfocus tab:mode" - case sessPreviewRefs: + case a.sessPreviewMode == sessPreviewRefs: h = "↑↓:nav ↵:open sp:select ←:unfocus tab:mode" - case sessPreviewContexts: + case a.sessPreviewMode == sessPreviewContexts: h = "↑↓:node ↵:open ←:unfocus tab:mode" default: h = "↑↓:scroll ←:unfocus tab:mode" } } else { - h = "↑↓:nav →:focus tab:mode ←:close" + h = "↑↓:nav →:focus tab:mode p:page ←:close" } h += " " + a.helpSuffix() return formatHelp(h) diff --git a/internal/tui/outputs.go b/internal/tui/outputs.go index e71469e..74ab5ef 100644 --- a/internal/tui/outputs.go +++ b/internal/tui/outputs.go @@ -186,7 +186,7 @@ func (a *App) renderOutputs(outs []session.SessionOutput, width int, collecting, var sb strings.Builder title := "── Outputs ──" if len(outs) > 0 && a.sessSplit.Focus { - title = "── Outputs ↵:open y:copy ──" + title = "── Outputs ↵:open y:copy x:actions ──" } sb.WriteString(statTitleStyle.Render(title) + "\n") if collecting { @@ -252,8 +252,187 @@ func outputLine(o session.SessionOutput, width int, selected bool) string { return head + " " + dimStyle.Render(truncate(o.Detail, avail)) + suffix } +// --- Row actions (shared by both output digests) --- + +// outputActionKind names one thing you can do with an output row. The row's own +// fields decide which of these apply — a plan slug with no file has nothing to +// edit, a scratchpad file has no URL — and only the applicable ones are ever +// offered. Advertising an action a row cannot perform is the confusion this +// menu exists to remove. +type outputActionKind int + +const ( + outputActionOpenURL outputActionKind = iota + outputActionJump // to the transcript entry that produced it + outputActionSession // to the producing conversation, no entry recorded + outputActionEdit // open the local file in $EDITOR + outputActionCopy // URL when there is one, else the path +) + +// outputAction is one applicable entry: the key that picks it, the label the +// hint box shows, and what it does. +type outputAction struct { + kind outputActionKind + key string + label string +} + +// outputActionsFor returns the actions that actually apply to o, in the order +// they are offered. hasSession says whether the caller knows which session +// produced the row (the day pane carries an anchor id; the per-session digest +// tracks one session), which is what makes the uuid-less "open the conversation" +// fallback possible. +func (a *App) outputActionsFor(o session.SessionOutput, hasSession bool) []outputAction { + akm := a.keymap.Actions + var acts []outputAction + if o.URL != "" { + acts = append(acts, outputAction{outputActionOpenURL, "o", "open in browser"}) + } + switch { + case o.MessageUUID != "": + acts = append(acts, outputAction{outputActionJump, "enter", "jump to first mention"}) + case hasSession: + // A plan slug inherited from a parent session records no entry, so there + // is nothing to jump to — but the conversation itself is still reachable + // and is the honest phrasing of what Enter will do. + acts = append(acts, outputAction{outputActionSession, "enter", "open the conversation"}) + } + if o.Path != "" { + acts = append(acts, outputAction{outputActionEdit, akm.Edit, "open in $EDITOR"}) + } + if o.URL != "" { + acts = append(acts, outputAction{outputActionCopy, akm.CopyPath, "copy URL"}) + } else if o.Path != "" { + acts = append(acts, outputAction{outputActionCopy, akm.CopyPath, "copy path"}) + } + return acts +} + +// outputActionRow is whichever digest row the actions menu is acting on, +// flattened to what the actions need: the output plus the session that produced +// it. The day pane fills sessID from the row's anchor, the per-session digest +// from the session the digest is tracking. +type outputActionRow struct { + out session.SessionOutput + sessID string +} + +// outputActionTarget returns the row under the cursor in whichever digest owns +// the focused pane, or false when neither does. +func (a *App) outputActionTarget() (outputActionRow, bool) { + if a.selectedOwnsDayPane() { + r, ok := a.selectedDayOutput() + if !ok { + return outputActionRow{}, false + } + return outputActionRow{out: r.out, sessID: r.sessID}, true + } + o, ok := a.selectedOutput() + if !ok { + return outputActionRow{}, false + } + // The digest tracks exactly one session (sessOutputsCacheID); the list + // cursor can resolve to a different one, and jumping against that would + // open the wrong transcript. + return outputActionRow{out: o, sessID: a.sessOutputsCacheID}, true +} + +// runOutputAction performs the action the key picked, if it applies to the row. +// An inapplicable key does nothing — the menu never offered it. +func (a *App) runOutputAction(row outputActionRow, key string) (tea.Model, tea.Cmd) { + for _, act := range a.outputActionsFor(row.out, row.sessID != "") { + if act.key != key { + continue + } + switch act.kind { + case outputActionOpenURL: + if err := a.openInBrowser(row.out.URL); err != nil { + a.copiedMsg = "Open failed: " + err.Error() + } else { + a.copiedMsg = "Opened " + row.out.Title + } + return a, nil + case outputActionJump: + return a.jumpToSessionEntry(row.sessID, row.out.MessageUUID) + case outputActionSession: + sess, ok := a.sessionByIDFromStore(row.sessID) + if !ok { + return a, nil + } + a.currentSess = sess + return a, a.openConversation(sess) + case outputActionEdit: + // tea.ExecProcess (inside openInEditor) is mandatory here: shelling + // out from a running Bubble Tea program without releasing the + // terminal leaves both the editor and the TUI writing to it. + return a.openInEditor(row.out.Path) + case outputActionCopy: + target := row.out.URL + if target == "" { + target = row.out.Path + } + copyToClipboard(target) + a.copiedMsg = "Copied " + row.out.Title + return a, nil + } + } + return a, nil +} + +// renderOutputActionsHintBox renders the actions menu for the digest row under +// the cursor. It names the row (kind and title) so the user can see what they +// are about to act on, and lists ONLY the actions that apply to it — an entry +// for something the row cannot do is the confusion this menu replaces. +func (a *App) renderOutputActionsHintBox() string { + row, ok := a.outputActionTarget() + if !ok { + return "" + } + hl := lipgloss.NewStyle().Foreground(colorAccent).Bold(true) + d := dimStyle + + kind := outputSection(row.out.Kind) + header := lipgloss.NewStyle().Bold(true).Foreground(colorPrimary).Render(truncate(row.out.Title, 48)) + if kind != "" { + header = d.Render(kind+" ") + header + } + + lines := []string{header} + acts := a.outputActionsFor(row.out, row.sessID != "") + if len(acts) == 0 { + // Reachable in principle (a row with no url, no path and no anchor); say + // so rather than showing an empty box. + lines = append(lines, d.Render("no actions for this row")) + } + for _, act := range acts { + lines = append(lines, hl.Render(displayKey(act.key))+d.Render(":"+act.label)) + } + lines = append(lines, d.Render("esc:cancel")) + + boxStyle := lipgloss.NewStyle(). + Border(lipgloss.RoundedBorder()). + BorderForeground(colorDim). + Padding(0, 1) + return boxStyle.Render(strings.Join(lines, "\n")) +} + +// outputsPreviewActionsActive reports whether the focused preview is an outputs +// digest, and so owns the `x` actions menu. Both digests qualify: the day pane +// (a date row or a day-scoped project row, which updateSessionPreview routes +// there regardless of the preview mode) and the per-session digest. The day +// check comes first for the same reason handleFocusedPreviewKeys puts it first +// — those rows own the pane whatever sessPreviewMode says. +func (a *App) outputsPreviewActionsActive() bool { + if !a.sessSplit.Focus || !a.sessSplit.Show { + return false + } + return a.selectedOwnsDayPane() || a.sessPreviewMode == sessPreviewOutputs +} + // handleOutputsPreviewKeys drives the Outputs digest when the preview pane has // focus: cursor movement plus Enter/o to open and y to copy the row's target. +// `x` opens the fuller actions menu (see outputsPreviewActionsActive) — these +// direct keys stay as the fast path. func (a *App) handleOutputsPreviewKeys(sp *SplitPane, key string) (tea.Model, tea.Cmd, bool) { switch key { case "enter", "o": diff --git a/internal/tui/outputs_actions_test.go b/internal/tui/outputs_actions_test.go new file mode 100644 index 0000000..9e4240d --- /dev/null +++ b/internal/tui/outputs_actions_test.go @@ -0,0 +1,291 @@ +package tui + +import ( + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/sendbird/ccx/internal/session" +) + +// The `x` actions menu on an output row must be HONEST: it lists exactly the +// actions the row can perform and nothing else. A PR has a URL and no file; a +// changed file has a path and no URL; a plan slug inherited from a parent +// session has neither a URL nor a recorded entry to jump to. Offering the same +// fixed menu for all three is the confusion these tests pin down. + +// outputsDigestApp builds a session-row App with the per-session Outputs digest +// focused and the given rows already collected. +func outputsDigestApp(t *testing.T, rows []session.SessionOutput) *App { + t.Helper() + sessions := []session.Session{{ + ID: "maker", ShortID: "maker", ProjectPath: "/tmp/repo-a", ProjectName: "repo-a", + ModTime: dayOf(0), + }} + app := newTestApp(sessions) + app.sessPreviewMode = sessPreviewOutputs + app.sessSplit.Show = true + app.sessSplit.Focus = true + app.sessOutputsCacheID = "maker" + app.sessOutputsRows = rows + app.sessOutputsCursor = 0 + return app +} + +// TestOutputActionsOnlyOfferWhatTheRowCanDo is the heart of the request: each +// row kind gets exactly its applicable actions. +func TestOutputActionsOnlyOfferWhatTheRowCanDo(t *testing.T) { + app := outputsDigestApp(t, nil) + + cases := []struct { + name string + out session.SessionOutput + anchor bool + want []outputActionKind + notWant []outputActionKind + }{ + { + name: "PR with a URL and a first mention", + out: session.SessionOutput{ + Kind: session.OutputPR, Title: "sendbird/ccx#5", + URL: "https://github.com/sendbird/ccx/pull/5", MessageUUID: "u2", + }, + anchor: true, + want: []outputActionKind{outputActionOpenURL, outputActionJump, outputActionCopy}, + notWant: []outputActionKind{outputActionEdit}, + }, + { + name: "changed file — a local path, no URL", + out: session.SessionOutput{ + Kind: session.OutputChange, Title: "app.go", + Path: "/tmp/repo-a/app.go", MessageUUID: "u7", + }, + anchor: true, + want: []outputActionKind{outputActionEdit, outputActionJump, outputActionCopy}, + notWant: []outputActionKind{outputActionOpenURL}, + }, + { + name: "scratchpad file found on disk — no transcript entry", + out: session.SessionOutput{ + Kind: session.OutputScratchpad, Title: "notes.md", + Path: "/tmp/scratch/notes.md", + }, + anchor: true, + want: []outputActionKind{outputActionEdit, outputActionSession, outputActionCopy}, + notWant: []outputActionKind{outputActionOpenURL, outputActionJump}, + }, + { + name: "plan slug inherited from a parent session — no uuid, no file", + out: session.SessionOutput{Kind: session.OutputPlan, Title: "some-plan"}, + // No anchor session either: nothing at all applies. + anchor: false, + want: nil, + notWant: []outputActionKind{outputActionOpenURL, outputActionJump, outputActionEdit, outputActionCopy, outputActionSession}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := app.outputActionsFor(tc.out, tc.anchor) + have := map[outputActionKind]bool{} + for _, act := range got { + have[act.kind] = true + } + for _, k := range tc.want { + if !have[k] { + t.Errorf("action %v should apply to this row but was not offered (got %+v)", k, got) + } + } + for _, k := range tc.notWant { + if have[k] { + t.Errorf("action %v cannot apply to this row but was offered anyway (got %+v)", k, got) + } + } + }) + } +} + +// TestOutputActionsHintBoxHidesInapplicableEntries guards the rendering half: +// the menu the user actually sees must not advertise an action the row cannot +// perform, and must name the row so they know what they are acting on. +func TestOutputActionsHintBoxHidesInapplicableEntries(t *testing.T) { + app := outputsDigestApp(t, []session.SessionOutput{{ + Kind: session.OutputChange, Title: "app.go", + Path: "/tmp/repo-a/app.go", MessageUUID: "u7", + }}) + + box := app.renderOutputActionsHintBox() + if !strings.Contains(box, "app.go") { + t.Errorf("hint box does not say which row it acts on:\n%s", box) + } + if !strings.Contains(box, "$EDITOR") { + t.Errorf("a file row must offer the editor, got:\n%s", box) + } + if strings.Contains(box, "browser") { + t.Errorf("a row with no URL must not advertise the browser, got:\n%s", box) + } + + // A PR is the mirror image. + app.sessOutputsRows = []session.SessionOutput{{ + Kind: session.OutputPR, Title: "sendbird/ccx#5", + URL: "https://github.com/sendbird/ccx/pull/5", MessageUUID: "u2", + }} + box = app.renderOutputActionsHintBox() + if !strings.Contains(box, "browser") { + t.Errorf("a PR row must offer the browser, got:\n%s", box) + } + if strings.Contains(box, "$EDITOR") { + t.Errorf("a row with no local file must not advertise the editor, got:\n%s", box) + } +} + +// TestOutputsActionsMenuOpensAndOpensURL walks the full key path: `x` opens the +// menu for the focused digest, and the next key runs the row's action. +func TestOutputsActionsMenuOpensAndOpensURL(t *testing.T) { + app := outputsDigestApp(t, []session.SessionOutput{{ + Kind: session.OutputPR, Title: "sendbird/ccx#5", + URL: "https://github.com/sendbird/ccx/pull/5", + }}) + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'x'}}) + got := m.(*App) + if !got.actionsMenu { + t.Fatal("x did not open the actions menu on a focused outputs digest") + } + if !strings.Contains(got.renderActionsHintBox(), "sendbird/ccx#5") { + t.Errorf("the session actions menu rendered instead of the output one:\n%s", got.renderActionsHintBox()) + } + + var opened string + got.openURL = func(u string) error { opened = u; return nil } + m, _ = got.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'o'}}) + got = m.(*App) + + if opened != "https://github.com/sendbird/ccx/pull/5" { + t.Errorf("x→o should open the PR, opened %q", opened) + } + if got.actionsMenu { + t.Error("the menu should close after a key is picked") + } +} + +// TestOutputActionsEditGoesThroughExecProcess pins the terminal-safety +// requirement: shelling out to $EDITOR from inside a running Bubble Tea program +// must hand the terminal over via tea.ExecProcess. Asserting the command is +// CONSTRUCTED (not running it — that would spawn a real editor) is as far as a +// test can go, so this checks the action routes to openInEditor at all. +func TestOutputActionsEditGoesThroughExecProcess(t *testing.T) { + app := outputsDigestApp(t, []session.SessionOutput{{ + Kind: session.OutputChange, Title: "app.go", Path: "/tmp/repo-a/app.go", + }}) + + row, ok := app.outputActionTarget() + if !ok { + t.Fatal("expected the digest row to be the action target") + } + _, cmd := app.runOutputAction(row, app.keymap.Actions.Edit) + if cmd == nil { + t.Fatal("e on a file row produced no command — $EDITOR was never launched") + } + // tea.ExecProcess returns an execMsg-producing cmd, which is a distinct + // type from a plain tick/nil. Running it would spawn the editor, so only + // its existence is asserted here; the terminal-handover contract lives in + // openInEditor, which every other edit path in the app also uses. +} + +// TestOutputActionsCopyFallsBackToPath pins the copy target: URL when there is +// one, path otherwise. +func TestOutputActionsCopyFallsBackToPath(t *testing.T) { + app := outputsDigestApp(t, []session.SessionOutput{{ + Kind: session.OutputScratchpad, Title: "notes.md", Path: "/tmp/scratch/notes.md", + }}) + + var copied string + orig := clipboardWrite + clipboardWrite = func(text string) error { copied = text; return nil } + t.Cleanup(func() { clipboardWrite = orig }) + + row, _ := app.outputActionTarget() + app.runOutputAction(row, app.keymap.Actions.CopyPath) + if copied != "/tmp/scratch/notes.md" { + t.Errorf("copied %q, want the row's path", copied) + } +} + +// TestOutputActionTargetUsesTheDigestSession guards the jump anchor: the digest +// tracks exactly one session (sessOutputsCacheID), which is NOT necessarily the +// one the list cursor resolves to. Jumping against the cursor's session would +// open the wrong transcript. +func TestOutputActionTargetUsesTheDigestSession(t *testing.T) { + app := outputsDigestApp(t, []session.SessionOutput{{ + Kind: session.OutputPR, Title: "sendbird/ccx#5", MessageUUID: "u2", + }}) + // The digest is still showing the session it was built for while the list + // cursor has moved on. + app.sessOutputsCacheID = "previous-session" + + row, ok := app.outputActionTarget() + if !ok { + t.Fatal("expected a target row") + } + if row.sessID != "previous-session" { + t.Errorf("action anchored to %q, want the session the digest tracks", row.sessID) + } +} + +// TestOutputsActionsDoNotHijackTheSessionMenu pins ownership the other way: with +// the preview unfocused, `x` is still the session actions menu. +func TestOutputsActionsDoNotHijackTheSessionMenu(t *testing.T) { + app := outputsDigestApp(t, []session.SessionOutput{{ + Kind: session.OutputPR, Title: "sendbird/ccx#5", URL: "https://example.com/1", + }}) + app.sessSplit.Focus = false + + if app.outputsPreviewActionsActive() { + t.Fatal("an unfocused pane must not own the actions menu") + } + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'x'}}) + got := m.(*App) + if !got.actionsMenu { + t.Fatal("x should still open the session actions menu from the list") + } + if box := got.renderActionsHintBox(); strings.Contains(box, "open in browser") { + t.Errorf("the output menu leaked into the list context:\n%s", box) + } +} + +// TestOutputsDirectKeysStillWork guards the fast path: `x` is additive, so +// Enter/o/y must keep working without going through the menu. +func TestOutputsDirectKeysStillWork(t *testing.T) { + app := outputsDigestApp(t, []session.SessionOutput{{ + Kind: session.OutputPR, Title: "sendbird/ccx#5", + URL: "https://github.com/sendbird/ccx/pull/5", + }}) + + var opened string + app.openURL = func(u string) error { opened = u; return nil } + if _, _, handled := app.handleOutputsPreviewKeys(&app.sessSplit, "o"); !handled { + t.Fatal("the digest stopped handling the direct o key") + } + if opened != "https://github.com/sendbird/ccx/pull/5" { + t.Errorf("direct o opened %q", opened) + } +} + +// TestOutputsActionsOnAnEmptyDigestSaysSo pins the empty case: a focused digest +// with nothing in it must not fall through to the list's session actions, which +// would act on a row the user is not looking at. +func TestOutputsActionsOnAnEmptyDigestSaysSo(t *testing.T) { + app := outputsDigestApp(t, nil) + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'x'}}) + got := m.(*App) + if got.actionsMenu { + t.Error("x opened a menu for a digest with no rows") + } + if got.copiedMsg == "" { + t.Error("x was swallowed silently on an empty digest") + } +} diff --git a/internal/tui/pagemenu_test.go b/internal/tui/pagemenu_test.go new file mode 100644 index 0000000..04f9b5b --- /dev/null +++ b/internal/tui/pagemenu_test.go @@ -0,0 +1,157 @@ +package tui + +import ( + "testing" + "time" + + "github.com/charmbracelet/bubbles/list" + tea "github.com/charmbracelet/bubbletea" + + "github.com/sendbird/ccx/internal/session" +) + +// The `p` preview-page combo existed, but only while the PREVIEW pane had +// focus. The normal position is the cursor in the LIST wanting to change what +// the right pane shows — there `p` did nothing at all, which is why it read as +// "the combo does not exist". + +// TestPageMenuOpensFromTheList is the reported gap. +func TestPageMenuOpensFromTheList(t *testing.T) { + sessions := []session.Session{{ + ID: "a1", ShortID: "a1", ProjectPath: "/tmp/repo-a", ProjectName: "repo-a", + ModTime: dayOf(0), + }} + app := newTestApp(sessions) + app.sessSplit.Show = true + app.sessSplit.Focus = false // cursor in the list — the normal position + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'p'}}) + got := m.(*App) + if !got.sessPageMenu { + t.Fatal("p from the list did not open the preview-page menu") + } + + // The next key picks a mode. + m, _ = got.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'o'}}) + got = m.(*App) + if got.sessPageMenu { + t.Error("the menu should close after a mode is picked") + } + if got.sessPreviewMode != sessPreviewOutputs { + t.Errorf("p→o left the preview mode at %v, want outputs", got.sessPreviewMode) + } +} + +// TestPageMenuStillOpensFromTheFocusedPreview pins the behavior that already +// worked — moving the binding out of the focus block must not lose it. +func TestPageMenuStillOpensFromTheFocusedPreview(t *testing.T) { + sessions := []session.Session{{ + ID: "a1", ShortID: "a1", ProjectPath: "/tmp/repo-a", ProjectName: "repo-a", + ModTime: dayOf(0), + }} + app := newTestApp(sessions) + app.sessSplit.Show = true + app.sessSplit.Focus = true + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'p'}}) + if !m.(*App).sessPageMenu { + t.Fatal("p from the focused preview stopped opening the page menu") + } +} + +// TestPageMenuSuppressedOnDayRows keeps the two mechanisms for one thing in +// agreement: a date row (and a day-scoped project row) always renders that +// scope's outputs, so it has no preview modes — the number keys already refuse +// there (rowSupportsPreviewModes), and the p menu must refuse identically +// rather than offering eleven entries that all no-op. +func TestPageMenuSuppressedOnDayRows(t *testing.T) { + sessions := []session.Session{{ + ID: "a1", ShortID: "a1", ProjectPath: "/tmp/repo-a", ProjectName: "repo-a", + ModTime: dayOf(0), + }} + app := newTestApp(sessions) + app.sessGroupMode = groupDaily + app.rebuildSessionList() + app.sessionList.Select(0) // the date row + app.sessSplit.Show = true + + if !app.selectedOwnsDayPane() { + t.Fatal("fixture should put the cursor on a day row") + } + before := app.sessPreviewMode + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'p'}}) + got := m.(*App) + if got.sessPageMenu { + t.Error("the page menu opened on a row that cannot honor any preview mode") + } + if got.sessPreviewMode != before { + t.Errorf("preview mode changed to %v on a day row", got.sessPreviewMode) + } + if got.copiedMsg == "" { + t.Error("p was swallowed silently — the user gets no reason why") + } +} + +// TestPageMenuDoesNotSwallowSearchInput pins the letter's other consumer: while +// the "/" filter is active, p is text, not a command. +func TestPageMenuDoesNotSwallowSearchInput(t *testing.T) { + sessions := []session.Session{{ + ID: "a1", ShortID: "a1", ProjectPath: "/tmp/repo-a", ProjectName: "repo-a", + ModTime: dayOf(0), + }} + app := newTestApp(sessions) + app.sessSplit.Show = true + app.sessionList.SetFilterState(list.Filtering) + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'p'}}) + got := m.(*App) + if got.sessPageMenu { + t.Fatal("p opened the page menu while the search filter was taking input") + } + if v := got.sessionList.FilterValue(); v != "p" { + t.Errorf("the filter input received %q, want the typed p", v) + } +} + +// TestPageMenuCountsAsAnOverlay guards the digit shortcuts: with the page menu +// open, a number key belongs to the menu's dismissal, not to the top-level +// preview-mode shortcuts firing behind it. +func TestPageMenuCountsAsAnOverlay(t *testing.T) { + app := newTestApp(nil) + app.sessPageMenu = true + if !app.isInOverlay() { + t.Error("an open page menu must count as an overlay") + } +} + +func TestPageMenuOpensWithThePreviewClosed(t *testing.T) { + // ccx can start with the split hidden, and `p` is exactly the key you reach + // for to say "show me something in the preview". Before the fix the binding + // sat behind `sp.Focus && sp.Show`, so a closed preview swallowed it — the + // pane did not even open. + sessions := []session.Session{ + {ID: "a1", ShortID: "a1", ProjectPath: "/tmp/repo-a", ProjectName: "repo-a", ModTime: time.Now(), MsgCount: 5}, + } + app := newTestApp(sessions) + app.sessSplit.Show = false + app.sessSplit.Focus = false + app.rebuildSessionList() + + m, _ := app.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("p")}) + a := m.(*App) + if !a.sessPageMenu { + t.Fatal("expected p to open the page menu with the preview closed") + } + + m, _ = a.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("s")}) + b := m.(*App) + if b.sessPreviewMode != sessPreviewStats { + t.Fatalf("expected the stats preview, got %d", b.sessPreviewMode) + } + // Picking a mode from a closed preview has to open the pane too — otherwise + // the mode changes behind a hidden pane and nothing visible happens. + if !b.sessSplit.Show { + t.Fatal("expected picking a mode to open the preview pane") + } +} diff --git a/internal/tui/shortcuts.go b/internal/tui/shortcuts.go index f92e433..78209bc 100644 --- a/internal/tui/shortcuts.go +++ b/internal/tui/shortcuts.go @@ -285,7 +285,7 @@ func (a *App) isInOverlay() bool { return a.actionsMenu || a.editMenu || a.convActionsMenu || a.executionContextMenu || a.conv.execution.Focused || a.viewsMenu || a.statsPageMenu || a.inspectorMenu || a.showHelp || - a.stateMenu + a.stateMenu || a.sessPageMenu } // shortcutHint returns a compact hint string showing active shortcuts