detached 650884a

fix(daemon): stop discarding keybinding handler errors

focusDir now returns the errNoNeighbor sentinel instead of nil for the
"no pane in that direction" case, letting reportError's benign filter
keep edge-focus silent while genuine layout-tree errors propagate.
Action's four focus cases, ActionJumpAttention, and
ActionToggleFloatingPane now route their errors through
d.reportError instead of discarding them. jumpAttention returns an
error so a real switchToTarget failure (a target existed but the
commit failed) surfaces as a notice, while the no-target path stays a
silent nil.
commit
650884a14d52daab25241a295bfb9aaf9ae8c14f
author
brice · · 2026-07-20 18:19 UTC
parent
9453c6d
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
6 files · +97 -16
Unified
UnifiedSplit
M internal/usecase/daemon/attention.go +9 -5
@@ -42,23 +42,27 @@ func (d *Daemon) noteAttention(sess *session, tb *tab) {
4242 d.pokeAttentionTicker()
4343 }
4444
45-func (d *Daemon) jumpAttention(sess *session, ac *attachedClient) {
45+// jumpAttention switches to the oldest pending attention target, local tab
46+// first and then another session's. Returning nil never implies a switch
47+// happened — it also covers "no target exists", which is routine and not an
48+// error. Only a failure to reach a target that does exist is a genuine error.
49+func (d *Daemon) jumpAttention(sess *session, ac *attachedClient) error {
4650 if sess == nil || ac == nil {
47- return
51+ return nil
4852 }
4953 if idx, ok := oldestAttentionTab(sess); ok {
5054 if sess.switchTab(idx) {
5155 d.activateTab(sess, sess.activeTab())
5256 d.invalidateRender(sess, ac, true, "attention.go")
5357 }
54- return
58+ return nil
5559 }
5660
5761 target, ok := d.oldestOtherSessionAttention(sess)
5862 if !ok {
59- return
63+ return nil
6064 }
61- _ = d.switchToTarget(sess, ac, picker.Target{Session: target.sessionID, TabIndex: target.tabIndex})
65+ return d.switchToTarget(sess, ac, picker.Target{Session: target.sessionID, TabIndex: target.tabIndex})
6266 }
6367
6468 func oldestAttentionTab(sess *session) (int, bool) {
@@ -42,23 +42,27 @@ func (d *Daemon) noteAttention(sess *session, tb *tab) {
42 d.pokeAttentionTicker()42 d.pokeAttentionTicker()
43 }43 }
44 44
45-func (d *Daemon) jumpAttention(sess *session, ac *attachedClient) {45+// jumpAttention switches to the oldest pending attention target, local tab
46+// first and then another session's. Returning nil never implies a switch
47+// happened — it also covers "no target exists", which is routine and not an
48+// error. Only a failure to reach a target that does exist is a genuine error.
49+func (d *Daemon) jumpAttention(sess *session, ac *attachedClient) error {
46 if sess == nil || ac == nil {50 if sess == nil || ac == nil {
47- return51+ return nil
48 }52 }
49 if idx, ok := oldestAttentionTab(sess); ok {53 if idx, ok := oldestAttentionTab(sess); ok {
50 if sess.switchTab(idx) {54 if sess.switchTab(idx) {
51 d.activateTab(sess, sess.activeTab())55 d.activateTab(sess, sess.activeTab())
52 d.invalidateRender(sess, ac, true, "attention.go")56 d.invalidateRender(sess, ac, true, "attention.go")
53 }57 }
54- return58+ return nil
55 }59 }
56 60
57 target, ok := d.oldestOtherSessionAttention(sess)61 target, ok := d.oldestOtherSessionAttention(sess)
58 if !ok {62 if !ok {
59- return63+ return nil
60 }64 }
61- _ = d.switchToTarget(sess, ac, picker.Target{Session: target.sessionID, TabIndex: target.tabIndex})65+ return d.switchToTarget(sess, ac, picker.Target{Session: target.sessionID, TabIndex: target.tabIndex})
62 }66 }
63 67
64 func oldestAttentionTab(sess *session) (int, bool) {68 func oldestAttentionTab(sess *session) (int, bool) {
M internal/usecase/daemon/attention_test.go +36 -1
@@ -526,7 +526,7 @@ func TestJumpAttentionNoopsWithNoBells(t *testing.T) {
526526 defer releases[1]()
527527 sess.active = 0
528528
529- d.jumpAttention(sess, ac)
529+ require.NoError(t, d.jumpAttention(sess, ac), "no target to jump to is not a failure")
530530
531531 require.Same(t, sess, ac.currentSession())
532532 require.Equal(t, 0, activeTabIndex(sess))
@@ -537,6 +537,41 @@ func TestJumpAttentionNoopsWithNoBells(t *testing.T) {
537537 }
538538 }
539539
540+// TestJumpAttentionSwitchFailureReportsNotice drives jumpAttention's
541+// cross-session path once a target has already been chosen, then makes the
542+// commit itself fail (the origin session's client no longer matches ac —
543+// simulating a client that detached between target selection and commit).
544+// Unlike the no-target case, a target existed and the switch to it failed:
545+// that is a genuine error, not a benign no-op, and must reach the user.
546+func TestJumpAttentionSwitchFailureReportsNotice(t *testing.T) {
547+ p1, release1 := newBlockingPTY(t)
548+ p2, release2 := newBlockingPTY(t)
549+ defer release1()
550+ defer release2()
551+ d, sess1, ac, _ := newManualSessionWithPTYs(t, p1)
552+ sctx2, cancel2 := context.WithCancel(d.serveCtx)
553+ defer cancel2()
554+ tab2 := newTestTabWithContext(p2, sctx2, cancel2)
555+ tab2.attention = true
556+ tab2.attentionAt = time.Unix(9, 0)
557+ sess2 := &session{id: "other", name: "other", ctx: sctx2, cancel: cancel2, tabs: []*tab{tab2}}
558+ d.sessions[sess2.id] = sess2
559+
560+ sess1.mu.Lock()
561+ sess1.client = nil
562+ sess1.mu.Unlock()
563+
564+ err := d.jumpAttention(sess1, ac)
565+
566+ require.Error(t, err)
567+ require.Same(t, sess1, ac.currentSession(), "a failed switch must leave the client on its origin session")
568+
569+ d.reportError(sess1, err)
570+ history := d.notices.history()
571+ require.Len(t, history, 1)
572+ require.Equal(t, domain.NoticeSessionUnavailable, history[0].Code)
573+}
574+
540575 func TestAttentionAnimatorParksAdvancesAndResets(t *testing.T) {
541576 d, sess, _, sends, releases := newManualTabSession(t, 2)
542577 defer releases[0]()
@@ -526,7 +526,7 @@ func TestJumpAttentionNoopsWithNoBells(t *testing.T) {
526 defer releases[1]()526 defer releases[1]()
527 sess.active = 0527 sess.active = 0
528 528
529- d.jumpAttention(sess, ac)529+ require.NoError(t, d.jumpAttention(sess, ac), "no target to jump to is not a failure")
530 530
531 require.Same(t, sess, ac.currentSession())531 require.Same(t, sess, ac.currentSession())
532 require.Equal(t, 0, activeTabIndex(sess))532 require.Equal(t, 0, activeTabIndex(sess))
@@ -537,6 +537,41 @@ func TestJumpAttentionNoopsWithNoBells(t *testing.T) {
537 }537 }
538 }538 }
539 539
540+// TestJumpAttentionSwitchFailureReportsNotice drives jumpAttention's
541+// cross-session path once a target has already been chosen, then makes the
542+// commit itself fail (the origin session's client no longer matches ac —
543+// simulating a client that detached between target selection and commit).
544+// Unlike the no-target case, a target existed and the switch to it failed:
545+// that is a genuine error, not a benign no-op, and must reach the user.
546+func TestJumpAttentionSwitchFailureReportsNotice(t *testing.T) {
547+ p1, release1 := newBlockingPTY(t)
548+ p2, release2 := newBlockingPTY(t)
549+ defer release1()
550+ defer release2()
551+ d, sess1, ac, _ := newManualSessionWithPTYs(t, p1)
552+ sctx2, cancel2 := context.WithCancel(d.serveCtx)
553+ defer cancel2()
554+ tab2 := newTestTabWithContext(p2, sctx2, cancel2)
555+ tab2.attention = true
556+ tab2.attentionAt = time.Unix(9, 0)
557+ sess2 := &session{id: "other", name: "other", ctx: sctx2, cancel: cancel2, tabs: []*tab{tab2}}
558+ d.sessions[sess2.id] = sess2
559+
560+ sess1.mu.Lock()
561+ sess1.client = nil
562+ sess1.mu.Unlock()
563+
564+ err := d.jumpAttention(sess1, ac)
565+
566+ require.Error(t, err)
567+ require.Same(t, sess1, ac.currentSession(), "a failed switch must leave the client on its origin session")
568+
569+ d.reportError(sess1, err)
570+ history := d.notices.history()
571+ require.Len(t, history, 1)
572+ require.Equal(t, domain.NoticeSessionUnavailable, history[0].Code)
573+}
574+
540 func TestAttentionAnimatorParksAdvancesAndResets(t *testing.T) {575 func TestAttentionAnimatorParksAdvancesAndResets(t *testing.T) {
541 d, sess, _, sends, releases := newManualTabSession(t, 2)576 d, sess, _, sends, releases := newManualTabSession(t, 2)
542 defer releases[0]()577 defer releases[0]()
M internal/usecase/daemon/input.go +16 -5
@@ -286,19 +286,30 @@ func (h daemonKeyHandler) Action(action keys.Action) {
286286 case keys.ActionOpenPalette:
287287 h.d.enterPalette(sess, h.ac)
288288 case keys.ActionJumpAttention:
289- h.d.jumpAttention(sess, h.ac)
289+ if err := h.d.jumpAttention(sess, h.ac); err != nil {
290+ h.d.reportError(sess, err)
291+ }
290292 case keys.ActionToggleFloatingPane:
291293 if err := h.d.toggleFloating(sess, h.ac); err != nil {
292294 h.d.log.Warn("toggle floating pane failed", "err", err)
295+ h.d.reportError(sess, err)
293296 }
294297 case keys.ActionFocusPaneLeft:
295- _ = h.d.focusDir(sess, h.ac, layout.Left)
298+ if err := h.d.focusDir(sess, h.ac, layout.Left); err != nil {
299+ h.d.reportError(sess, err)
300+ }
296301 case keys.ActionFocusPaneRight:
297- _ = h.d.focusDir(sess, h.ac, layout.Right)
302+ if err := h.d.focusDir(sess, h.ac, layout.Right); err != nil {
303+ h.d.reportError(sess, err)
304+ }
298305 case keys.ActionFocusPaneUp:
299- _ = h.d.focusDir(sess, h.ac, layout.Up)
306+ if err := h.d.focusDir(sess, h.ac, layout.Up); err != nil {
307+ h.d.reportError(sess, err)
308+ }
300309 case keys.ActionFocusPaneDown:
301- _ = h.d.focusDir(sess, h.ac, layout.Down)
310+ if err := h.d.focusDir(sess, h.ac, layout.Down); err != nil {
311+ h.d.reportError(sess, err)
312+ }
302313 case keys.ActionSwitchTab1, keys.ActionSwitchTab2, keys.ActionSwitchTab3,
303314 keys.ActionSwitchTab4, keys.ActionSwitchTab5, keys.ActionSwitchTab6,
304315 keys.ActionSwitchTab7, keys.ActionSwitchTab8, keys.ActionSwitchTab9:
@@ -286,19 +286,30 @@ func (h daemonKeyHandler) Action(action keys.Action) {
286 case keys.ActionOpenPalette:286 case keys.ActionOpenPalette:
287 h.d.enterPalette(sess, h.ac)287 h.d.enterPalette(sess, h.ac)
288 case keys.ActionJumpAttention:288 case keys.ActionJumpAttention:
289- h.d.jumpAttention(sess, h.ac)289+ if err := h.d.jumpAttention(sess, h.ac); err != nil {
290+ h.d.reportError(sess, err)
291+ }
290 case keys.ActionToggleFloatingPane:292 case keys.ActionToggleFloatingPane:
291 if err := h.d.toggleFloating(sess, h.ac); err != nil {293 if err := h.d.toggleFloating(sess, h.ac); err != nil {
292 h.d.log.Warn("toggle floating pane failed", "err", err)294 h.d.log.Warn("toggle floating pane failed", "err", err)
295+ h.d.reportError(sess, err)
293 }296 }
294 case keys.ActionFocusPaneLeft:297 case keys.ActionFocusPaneLeft:
295- _ = h.d.focusDir(sess, h.ac, layout.Left)298+ if err := h.d.focusDir(sess, h.ac, layout.Left); err != nil {
299+ h.d.reportError(sess, err)
300+ }
296 case keys.ActionFocusPaneRight:301 case keys.ActionFocusPaneRight:
297- _ = h.d.focusDir(sess, h.ac, layout.Right)302+ if err := h.d.focusDir(sess, h.ac, layout.Right); err != nil {
303+ h.d.reportError(sess, err)
304+ }
298 case keys.ActionFocusPaneUp:305 case keys.ActionFocusPaneUp:
299- _ = h.d.focusDir(sess, h.ac, layout.Up)306+ if err := h.d.focusDir(sess, h.ac, layout.Up); err != nil {
307+ h.d.reportError(sess, err)
308+ }
300 case keys.ActionFocusPaneDown:309 case keys.ActionFocusPaneDown:
301- _ = h.d.focusDir(sess, h.ac, layout.Down)310+ if err := h.d.focusDir(sess, h.ac, layout.Down); err != nil {
311+ h.d.reportError(sess, err)
312+ }
302 case keys.ActionSwitchTab1, keys.ActionSwitchTab2, keys.ActionSwitchTab3,313 case keys.ActionSwitchTab1, keys.ActionSwitchTab2, keys.ActionSwitchTab3,
303 keys.ActionSwitchTab4, keys.ActionSwitchTab5, keys.ActionSwitchTab6,314 keys.ActionSwitchTab4, keys.ActionSwitchTab5, keys.ActionSwitchTab6,
304 keys.ActionSwitchTab7, keys.ActionSwitchTab8, keys.ActionSwitchTab9:315 keys.ActionSwitchTab7, keys.ActionSwitchTab8, keys.ActionSwitchTab9:
M internal/usecase/daemon/input_test.go +29 -0
@@ -329,6 +329,35 @@ func TestFloatingStaysTerminalTargetAfterDirectionalFocus(t *testing.T) {
329329 requirePTYWrite(t, floatingWrites, []byte("x"))
330330 }
331331
332+// TestActionFocusPaneAtEdgeStaysSilent drives a directional focus move with
333+// no pane on that side (the session has a single, unsplit pane). This is
334+// routine navigation, not a failure, and must never produce a notice.
335+func TestActionFocusPaneAtEdgeStaysSilent(t *testing.T) {
336+ d, _, ac, _ := newManualSessionWithPTYs(t, nil)
337+
338+ daemonKeyHandler{d: d, ac: ac}.Action(keys.ActionFocusPaneLeft)
339+
340+ require.Empty(t, d.notices.history(), "no-neighbor focus move must stay silent")
341+}
342+
343+// TestActionFocusPaneGenuineErrorReportsNoticeInternal drives a directional
344+// focus move against a tab with no layout tree at all — a genuine structural
345+// error distinct from "no pane in that direction" — and asserts it reaches
346+// the user as a notice instead of being silently discarded.
347+func TestActionFocusPaneGenuineErrorReportsNoticeInternal(t *testing.T) {
348+ d, sess, ac, _ := newManualSessionWithPTYs(t, nil)
349+ tb := sess.activeTab()
350+ tb.mu.Lock()
351+ tb.tree = nil
352+ tb.mu.Unlock()
353+
354+ daemonKeyHandler{d: d, ac: ac}.Action(keys.ActionFocusPaneLeft)
355+
356+ history := d.notices.history()
357+ require.Len(t, history, 1, "a genuine layout-tree error must surface exactly one notice")
358+ require.Equal(t, domain.NoticeInternal, history[0].Code)
359+}
360+
332361 func TestFloatingVisibilityRemainsIndependentAcrossTabSwitches(t *testing.T) {
333362 d, sess, ac, _ := newManualSessionWithPTYs(t, nil)
334363 first := sess.activeTab()
@@ -329,6 +329,35 @@ func TestFloatingStaysTerminalTargetAfterDirectionalFocus(t *testing.T) {
329 requirePTYWrite(t, floatingWrites, []byte("x"))329 requirePTYWrite(t, floatingWrites, []byte("x"))
330 }330 }
331 331
332+// TestActionFocusPaneAtEdgeStaysSilent drives a directional focus move with
333+// no pane on that side (the session has a single, unsplit pane). This is
334+// routine navigation, not a failure, and must never produce a notice.
335+func TestActionFocusPaneAtEdgeStaysSilent(t *testing.T) {
336+ d, _, ac, _ := newManualSessionWithPTYs(t, nil)
337+
338+ daemonKeyHandler{d: d, ac: ac}.Action(keys.ActionFocusPaneLeft)
339+
340+ require.Empty(t, d.notices.history(), "no-neighbor focus move must stay silent")
341+}
342+
343+// TestActionFocusPaneGenuineErrorReportsNoticeInternal drives a directional
344+// focus move against a tab with no layout tree at all — a genuine structural
345+// error distinct from "no pane in that direction" — and asserts it reaches
346+// the user as a notice instead of being silently discarded.
347+func TestActionFocusPaneGenuineErrorReportsNoticeInternal(t *testing.T) {
348+ d, sess, ac, _ := newManualSessionWithPTYs(t, nil)
349+ tb := sess.activeTab()
350+ tb.mu.Lock()
351+ tb.tree = nil
352+ tb.mu.Unlock()
353+
354+ daemonKeyHandler{d: d, ac: ac}.Action(keys.ActionFocusPaneLeft)
355+
356+ history := d.notices.history()
357+ require.Len(t, history, 1, "a genuine layout-tree error must surface exactly one notice")
358+ require.Equal(t, domain.NoticeInternal, history[0].Code)
359+}
360+
332 func TestFloatingVisibilityRemainsIndependentAcrossTabSwitches(t *testing.T) {361 func TestFloatingVisibilityRemainsIndependentAcrossTabSwitches(t *testing.T) {
333 d, sess, ac, _ := newManualSessionWithPTYs(t, nil)362 d, sess, ac, _ := newManualSessionWithPTYs(t, nil)
334 first := sess.activeTab()363 first := sess.activeTab()
M internal/usecase/daemon/palette_test.go +6 -4
@@ -938,10 +938,12 @@ func TestPaletteExecMethods(t *testing.T) {
938938 require.NoError(t, exec.StackPane())
939939 require.NoError(t, exec.ToggleStack())
940940 require.NoError(t, exec.ClosePane())
941- require.NoError(t, exec.FocusPaneLeft())
942- require.NoError(t, exec.FocusPaneRight())
943- require.NoError(t, exec.FocusPaneUp())
944- require.NoError(t, exec.FocusPaneDown())
941+ // A single pane remains: every direction is a benign no-neighbor move,
942+ // not an error the palette should ever surface (Task 10).
943+ require.ErrorIs(t, exec.FocusPaneLeft(), errNoNeighbor)
944+ require.ErrorIs(t, exec.FocusPaneRight(), errNoNeighbor)
945+ require.ErrorIs(t, exec.FocusPaneUp(), errNoNeighbor)
946+ require.ErrorIs(t, exec.FocusPaneDown(), errNoNeighbor)
945947 require.NoError(t, exec.CloseTab())
946948 require.Len(t, sess.tabs, 2)
947949 // Drain paints generated by methods above.
@@ -938,10 +938,12 @@ func TestPaletteExecMethods(t *testing.T) {
938 require.NoError(t, exec.StackPane())938 require.NoError(t, exec.StackPane())
939 require.NoError(t, exec.ToggleStack())939 require.NoError(t, exec.ToggleStack())
940 require.NoError(t, exec.ClosePane())940 require.NoError(t, exec.ClosePane())
941- require.NoError(t, exec.FocusPaneLeft())941+ // A single pane remains: every direction is a benign no-neighbor move,
942- require.NoError(t, exec.FocusPaneRight())942+ // not an error the palette should ever surface (Task 10).
943- require.NoError(t, exec.FocusPaneUp())943+ require.ErrorIs(t, exec.FocusPaneLeft(), errNoNeighbor)
944- require.NoError(t, exec.FocusPaneDown())944+ require.ErrorIs(t, exec.FocusPaneRight(), errNoNeighbor)
945+ require.ErrorIs(t, exec.FocusPaneUp(), errNoNeighbor)
946+ require.ErrorIs(t, exec.FocusPaneDown(), errNoNeighbor)
945 require.NoError(t, exec.CloseTab())947 require.NoError(t, exec.CloseTab())
946 require.Len(t, sess.tabs, 2)948 require.Len(t, sess.tabs, 2)
947 // Drain paints generated by methods above.949 // Drain paints generated by methods above.
M internal/usecase/daemon/pane_actions.go +1 -1
@@ -317,7 +317,7 @@ func (d *Daemon) focusDir(sess *session, ac *attachedClient, dir layout.Directio
317317 d.invalidateRender(sess, ac, true, "pane_actions.go")
318318 }
319319 if errors.Is(err, layout.ErrNoPane) {
320- return nil
320+ return errNoNeighbor
321321 }
322322 return err
323323 }
@@ -317,7 +317,7 @@ func (d *Daemon) focusDir(sess *session, ac *attachedClient, dir layout.Directio
317 d.invalidateRender(sess, ac, true, "pane_actions.go")317 d.invalidateRender(sess, ac, true, "pane_actions.go")
318 }318 }
319 if errors.Is(err, layout.ErrNoPane) {319 if errors.Is(err, layout.ErrNoPane) {
320- return nil320+ return errNoNeighbor
321 }321 }
322 return err322 return err
323 }323 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close