From cfa426883096e8fbba8c35fc5c300f0aabf49d59 Mon Sep 17 00:00:00 2001 From: Gavin Jeong Date: Thu, 9 Jul 2026 16:37:52 +0900 Subject: [PATCH] fix: open PR/Jira URL on Enter in the References preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JIRA: https://sendbird.atlassian.net/browse/CPLAT-10756 With the References preview focused, Enter on a PR/Jira entry did nothing useful — it fell through to km.Session.Open and opened the conversation instead of the URL. handleRefsPreviewKeys mapped both "enter" and "o" to openRefUnderCursor, but the Session.Open case runs first for Enter and had branches for the conversation/agents/workflows previews only, not refs. So `o` opened the URL while Enter leaked to openConversation. Add a refs branch to Session.Open mirroring the others, so a focused refs preview opens the URL under the cursor on Enter. Also make the browser opener injectable (a.openURL, defaulting to `open`) so the behavior is testable without spawning a real browser. TestRefsPreviewEnterOpensURL asserts both Enter and `o` open the ref URL and leave the view on viewSessions. --- internal/tui/app.go | 19 ++++++++++++++- internal/tui/refs_preview_test.go | 39 +++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/internal/tui/app.go b/internal/tui/app.go index c033ca2..af21868 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -342,6 +342,7 @@ type App struct { sessRefsCursor int // cursor within the References preview list sessRefsResolved bool // whether the currently-previewed session's refs have been resolved refsInFlight map[string]bool // session IDs with a resolve pass currently running (prevents re-targeting every tick) + openURL func(string) error // opens a URL in the browser; overridable in tests (defaults to `open`) // Conversation preview state sessConvEntries []mergedMsg // merged conversation messages @@ -1807,6 +1808,13 @@ func (a *App) handleSessionKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m, cmd, _ := a.drillIntoWorkflowAgent() return m, cmd } + // If References preview is focused, open the PR/Jira URL under the cursor + // in the browser (mirrors `o`; without this Enter would fall through to + // openConversation). + if sp.Focus && sp.Show && a.sessPreviewMode == sessPreviewRefs && len(a.sessPreviewRefs) > 0 { + m, cmd, _ := a.openRefUnderCursor() + return m, cmd + } sess, ok := a.selectedSession() if !ok { return a, nil @@ -2278,7 +2286,7 @@ func (a *App) openRefUnderCursor() (tea.Model, tea.Cmd, bool) { a.copiedMsg = "No URL for this reference" return a, nil, true } - if err := exec.Command("open", ref.URL).Start(); err != nil { + if err := a.openInBrowser(ref.URL); err != nil { a.copiedMsg = "Error: " + err.Error() return a, nil, true } @@ -2286,6 +2294,15 @@ func (a *App) openRefUnderCursor() (tea.Model, tea.Cmd, bool) { return a, nil, true } +// openInBrowser opens a URL in the default browser. The opener is overridable +// (a.openURL) so tests can intercept it instead of spawning `open`. +func (a *App) openInBrowser(url string) error { + if a.openURL != nil { + return a.openURL(url) + } + return exec.Command("open", url).Start() +} + // drillIntoWorkflowAgent opens the conversation view for the current session and // navigates into the workflow agent under the cursor — its full transcript, // which the workflow run summary only previews. diff --git a/internal/tui/refs_preview_test.go b/internal/tui/refs_preview_test.go index dbcf185..02818d9 100644 --- a/internal/tui/refs_preview_test.go +++ b/internal/tui/refs_preview_test.go @@ -90,3 +90,42 @@ func TestRenderSessionRefs(t *testing.T) { t.Error("focused render missing open hint") } } + +// TestRefsPreviewEnterOpensURL guards the reported bug: with the References +// preview focused, Enter on a PR/Jira entry must open its URL in the browser — +// NOT fall through to km.Session.Open and open the conversation. `o` already +// worked; Enter was intercepted by the Session.Open case, which had no refs +// branch. Both Enter and `o` must open the URL and leave the view unchanged. +func TestRefsPreviewEnterOpensURL(t *testing.T) { + for _, key := range []tea.KeyMsg{ + {Type: tea.KeyEnter}, + {Type: tea.KeyRunes, Runes: []rune{'o'}}, + } { + sessions := fakeSessions() + a := newTestApp(sessions) + a.sessionList.Select(0) + + // Simulate a focused, populated References preview. + a.sessPreviewMode = sessPreviewRefs + a.sessSplit.Show = true + a.sessSplit.Focus = true + a.sessPreviewRefs = []session.SessionRef{ + {Kind: session.RefPR, URL: "https://github.com/sendbird/ccx/pull/62", + Label: "sendbird/ccx#62", State: session.RefStateOpen, Resolved: true}, + } + a.sessRefsCursor = 0 + + var opened string + a.openURL = func(u string) error { opened = u; return nil } + + m, _ := a.Update(key) + got := m.(*App) + + if opened != "https://github.com/sendbird/ccx/pull/62" { + t.Errorf("key %v: expected the ref URL to be opened, got %q", key, opened) + } + if got.state != viewSessions { + t.Errorf("key %v: view changed to %v — Enter leaked to openConversation", key, got.state) + } + } +}