detached 758670c

fix(sessions): address ephemeral session review findings

commit
758670c5fa86f351ab85e02cb5b2ac8954ebd55b
author
brice · · 2026-07-07 06:09 UTC
parent
16ed5a6
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
5 files · +43 -17
M internal/app/daemon_integration_test.go +2 -1
@@ -4,6 +4,7 @@ package app
44
55 import (
66 "context"
7+ "errors"
78 "io"
89 "log/slog"
910 "strings"
@@ -111,7 +112,7 @@ func killAll(dir string) error {
111112 return err
112113 }
113114 _, err = tr.Recv()
114- if err == io.EOF {
115+ if errors.Is(err, io.EOF) {
115116 return nil
116117 }
117118 return err
@@ -4,6 +4,7 @@ package app
4 4
5 import (5 import (
6 "context"6 "context"
7+ "errors"
7 "io"8 "io"
8 "log/slog"9 "log/slog"
9 "strings"10 "strings"
@@ -111,7 +112,7 @@ func killAll(dir string) error {
111 return err112 return err
112 }113 }
113 _, err = tr.Recv()114 _, err = tr.Recv()
114- if err == io.EOF {115+ if errors.Is(err, io.EOF) {
115 return nil116 return nil
116 }117 }
117 return err118 return err
M internal/usecase/client/client.go +18 -14
@@ -382,11 +382,14 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
382382 if err != nil {
383383 return attachResult{err: fmt.Errorf("vev: awaiting welcome: %w", err)}
384384 }
385+ result := func(welcomed bool, err error) attachResult {
386+ return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: welcomed, err: err}
387+ }
385388 switch reply.Type {
386389 case ports.MsgWelcome:
387390 welcome, derr := ports.UnmarshalWelcome(reply.Payload)
388391 if derr != nil {
389- return attachResult{err: fmt.Errorf("vev: decoding welcome: %w", derr)}
392+ return result(false, fmt.Errorf("vev: decoding welcome: %w", derr))
390393 }
391394 resumeToken = welcome.ResumeToken
392395 name = welcome.SessionName
@@ -395,16 +398,17 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
395398 case ports.MsgError:
396399 em, derr := ports.UnmarshalErrorMsg(reply.Payload)
397400 if derr != nil {
398- return attachResult{err: fmt.Errorf("vev: decoding error reply: %w", derr)}
401+ return result(false, fmt.Errorf("vev: decoding error reply: %w", derr))
399402 }
400- return attachResult{resumeToken: resumeToken, sessionName: name, err: &ProtocolError{Code: em.Code, Text: em.Text}}
403+ return result(false, &ProtocolError{Code: em.Code, Text: em.Text})
401404 default:
402- return attachResult{resumeToken: resumeToken, sessionName: name, err: fmt.Errorf("vev: unexpected reply type %d before welcome", reply.Type)}
405+ return result(false, fmt.Errorf("vev: unexpected reply type %d before welcome", reply.Type))
403406 }
407+ welcomedResult := func(err error) attachResult { return result(true, err) }
404408
405409 // 3. Enter raw mode after Welcome; Run owns restoration.
406410 if err := enterRaw(); err != nil {
407- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: err}
411+ return welcomedResult(err)
408412 }
409413 clearStatus()
410414
@@ -449,9 +453,9 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
449453 // context was cancelled. A queued sender error takes priority.
450454 select {
451455 case serr := <-sendErrCh:
452- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: sending to daemon: %w", serr)}
456+ return welcomedResult(fmt.Errorf("vev: sending to daemon: %w", serr))
453457 default:
454- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true}
458+ return welcomedResult(nil)
455459 }
456460 case <-colorQueryCh:
457461 if err := term.QueryColors(); err != nil {
@@ -460,21 +464,21 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
460464 case r := <-recvCh:
461465 if r.err != nil {
462466 if errors.Is(r.err, io.EOF) {
463- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: daemon vanished (missing: %v): %w", ms.missing(), r.err)}
467+ return welcomedResult(fmt.Errorf("vev: daemon vanished (missing: %v): %w", ms.missing(), r.err))
464468 }
465- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: receiving from daemon: %w", r.err)}
469+ return welcomedResult(fmt.Errorf("vev: receiving from daemon: %w", r.err))
466470 }
467471 switch r.frame.Type {
468472 case ports.MsgOutput:
469473 o, derr := ports.UnmarshalOutput(r.frame.Payload)
470474 if derr != nil {
471- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: decoding output: %w", derr)}
475+ return welcomedResult(fmt.Errorf("vev: decoding output: %w", derr))
472476 }
473477 if _, werr := term.Out().Write(o.Data); werr != nil {
474- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: writing terminal output: %w", werr)}
478+ return welcomedResult(fmt.Errorf("vev: writing terminal output: %w", werr))
475479 }
476480 if ferr := term.Flush(); ferr != nil {
477- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: flushing terminal: %w", ferr)}
481+ return welcomedResult(fmt.Errorf("vev: flushing terminal: %w", ferr))
478482 }
479483 if !ms.firstOutput {
480484 ms.firstOutput = true
@@ -483,7 +487,7 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
483487 case ports.MsgDetached:
484488 d, derr := ports.UnmarshalDetached(r.frame.Payload)
485489 if derr != nil {
486- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: decoding detached: %w", derr)}
490+ return welcomedResult(fmt.Errorf("vev: decoding detached: %w", derr))
487491 }
488492 ms.detached = true
489493 if d.Reason == ports.ReasonDetach {
@@ -491,7 +495,7 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
491495 } else {
492496 log.Warn("detached by daemon", "reason", d.Reason)
493497 }
494- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: detachedResult(d.Reason)}
498+ return welcomedResult(detachedResult(d.Reason))
495499 case ports.MsgPong:
496500 // Liveness reply; nothing to do.
497501 default:
@@ -382,11 +382,14 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
382 if err != nil {382 if err != nil {
383 return attachResult{err: fmt.Errorf("vev: awaiting welcome: %w", err)}383 return attachResult{err: fmt.Errorf("vev: awaiting welcome: %w", err)}
384 }384 }
385+ result := func(welcomed bool, err error) attachResult {
386+ return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: welcomed, err: err}
387+ }
385 switch reply.Type {388 switch reply.Type {
386 case ports.MsgWelcome:389 case ports.MsgWelcome:
387 welcome, derr := ports.UnmarshalWelcome(reply.Payload)390 welcome, derr := ports.UnmarshalWelcome(reply.Payload)
388 if derr != nil {391 if derr != nil {
389- return attachResult{err: fmt.Errorf("vev: decoding welcome: %w", derr)}392+ return result(false, fmt.Errorf("vev: decoding welcome: %w", derr))
390 }393 }
391 resumeToken = welcome.ResumeToken394 resumeToken = welcome.ResumeToken
392 name = welcome.SessionName395 name = welcome.SessionName
@@ -395,16 +398,17 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
395 case ports.MsgError:398 case ports.MsgError:
396 em, derr := ports.UnmarshalErrorMsg(reply.Payload)399 em, derr := ports.UnmarshalErrorMsg(reply.Payload)
397 if derr != nil {400 if derr != nil {
398- return attachResult{err: fmt.Errorf("vev: decoding error reply: %w", derr)}401+ return result(false, fmt.Errorf("vev: decoding error reply: %w", derr))
399 }402 }
400- return attachResult{resumeToken: resumeToken, sessionName: name, err: &ProtocolError{Code: em.Code, Text: em.Text}}403+ return result(false, &ProtocolError{Code: em.Code, Text: em.Text})
401 default:404 default:
402- return attachResult{resumeToken: resumeToken, sessionName: name, err: fmt.Errorf("vev: unexpected reply type %d before welcome", reply.Type)}405+ return result(false, fmt.Errorf("vev: unexpected reply type %d before welcome", reply.Type))
403 }406 }
407+ welcomedResult := func(err error) attachResult { return result(true, err) }
404 408
405 // 3. Enter raw mode after Welcome; Run owns restoration.409 // 3. Enter raw mode after Welcome; Run owns restoration.
406 if err := enterRaw(); err != nil {410 if err := enterRaw(); err != nil {
407- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: err}411+ return welcomedResult(err)
408 }412 }
409 clearStatus()413 clearStatus()
410 414
@@ -449,9 +453,9 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
449 // context was cancelled. A queued sender error takes priority.453 // context was cancelled. A queued sender error takes priority.
450 select {454 select {
451 case serr := <-sendErrCh:455 case serr := <-sendErrCh:
452- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: sending to daemon: %w", serr)}456+ return welcomedResult(fmt.Errorf("vev: sending to daemon: %w", serr))
453 default:457 default:
454- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true}458+ return welcomedResult(nil)
455 }459 }
456 case <-colorQueryCh:460 case <-colorQueryCh:
457 if err := term.QueryColors(); err != nil {461 if err := term.QueryColors(); err != nil {
@@ -460,21 +464,21 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
460 case r := <-recvCh:464 case r := <-recvCh:
461 if r.err != nil {465 if r.err != nil {
462 if errors.Is(r.err, io.EOF) {466 if errors.Is(r.err, io.EOF) {
463- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: daemon vanished (missing: %v): %w", ms.missing(), r.err)}467+ return welcomedResult(fmt.Errorf("vev: daemon vanished (missing: %v): %w", ms.missing(), r.err))
464 }468 }
465- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: receiving from daemon: %w", r.err)}469+ return welcomedResult(fmt.Errorf("vev: receiving from daemon: %w", r.err))
466 }470 }
467 switch r.frame.Type {471 switch r.frame.Type {
468 case ports.MsgOutput:472 case ports.MsgOutput:
469 o, derr := ports.UnmarshalOutput(r.frame.Payload)473 o, derr := ports.UnmarshalOutput(r.frame.Payload)
470 if derr != nil {474 if derr != nil {
471- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: decoding output: %w", derr)}475+ return welcomedResult(fmt.Errorf("vev: decoding output: %w", derr))
472 }476 }
473 if _, werr := term.Out().Write(o.Data); werr != nil {477 if _, werr := term.Out().Write(o.Data); werr != nil {
474- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: writing terminal output: %w", werr)}478+ return welcomedResult(fmt.Errorf("vev: writing terminal output: %w", werr))
475 }479 }
476 if ferr := term.Flush(); ferr != nil {480 if ferr := term.Flush(); ferr != nil {
477- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: flushing terminal: %w", ferr)}481+ return welcomedResult(fmt.Errorf("vev: flushing terminal: %w", ferr))
478 }482 }
479 if !ms.firstOutput {483 if !ms.firstOutput {
480 ms.firstOutput = true484 ms.firstOutput = true
@@ -483,7 +487,7 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
483 case ports.MsgDetached:487 case ports.MsgDetached:
484 d, derr := ports.UnmarshalDetached(r.frame.Payload)488 d, derr := ports.UnmarshalDetached(r.frame.Payload)
485 if derr != nil {489 if derr != nil {
486- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: fmt.Errorf("vev: decoding detached: %w", derr)}490+ return welcomedResult(fmt.Errorf("vev: decoding detached: %w", derr))
487 }491 }
488 ms.detached = true492 ms.detached = true
489 if d.Reason == ports.ReasonDetach {493 if d.Reason == ports.ReasonDetach {
@@ -491,7 +495,7 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
491 } else {495 } else {
492 log.Warn("detached by daemon", "reason", d.Reason)496 log.Warn("detached by daemon", "reason", d.Reason)
493 }497 }
494- return attachResult{resumeToken: resumeToken, sessionName: name, welcomed: true, err: detachedResult(d.Reason)}498+ return welcomedResult(detachedResult(d.Reason))
495 case ports.MsgPong:499 case ports.MsgPong:
496 // Liveness reply; nothing to do.500 // Liveness reply; nothing to do.
497 default:501 default:
M internal/usecase/daemon/client.go +1 -1
@@ -570,12 +570,12 @@ func (d *Daemon) detachOnSendError(sess *session, ac *attachedClient, failed por
570570 }
571571 if sess.detachIfCurrent(ac) {
572572 d.unregisterPreview(ac)
573- d.resetScreenDefaultColors(sess)
574573 if d.parkAttachment(sess, ac) {
575574 _ = ac.closeCapturedTransport(failed)
576575 d.log.Warn("parked client after send error", "session", sess.name)
577576 return
578577 }
578+ d.resetScreenDefaultColors(sess)
579579 _ = ac.closeCapturedTransport(failed)
580580 d.log.Warn("detached client after send error", "session", sess.name)
581581 }
@@ -570,12 +570,12 @@ func (d *Daemon) detachOnSendError(sess *session, ac *attachedClient, failed por
570 }570 }
571 if sess.detachIfCurrent(ac) {571 if sess.detachIfCurrent(ac) {
572 d.unregisterPreview(ac)572 d.unregisterPreview(ac)
573- d.resetScreenDefaultColors(sess)
574 if d.parkAttachment(sess, ac) {573 if d.parkAttachment(sess, ac) {
575 _ = ac.closeCapturedTransport(failed)574 _ = ac.closeCapturedTransport(failed)
576 d.log.Warn("parked client after send error", "session", sess.name)575 d.log.Warn("parked client after send error", "session", sess.name)
577 return576 return
578 }577 }
578+ d.resetScreenDefaultColors(sess)
579 _ = ac.closeCapturedTransport(failed)579 _ = ac.closeCapturedTransport(failed)
580 d.log.Warn("detached client after send error", "session", sess.name)580 d.log.Warn("detached client after send error", "session", sess.name)
581 }581 }
M internal/usecase/daemon/daemon.go +3 -1
@@ -15,7 +15,9 @@
1515 // scheduler).
1616 // - The daemon exits (Serve returns) when the last session is removed, or
1717 // when the parent context is cancelled (graceful shutdown notifies any
18-// attached clients with ReasonServerShutdown).
18+// attached clients with ReasonServerShutdown). Client detach does not end
19+// a session: ephemeral sessions stay in memory until explicitly killed or
20+// until the daemon exits, and named sessions can also persist on disk.
1921 //
2022 // Locking: a pane's screen/scrollback and per-client renderer shadow are
2123 // guarded by pane.mu/tab.mu as appropriate; the attached-client pointer by
@@ -15,7 +15,9 @@
15 // scheduler).15 // scheduler).
16 // - The daemon exits (Serve returns) when the last session is removed, or16 // - The daemon exits (Serve returns) when the last session is removed, or
17 // when the parent context is cancelled (graceful shutdown notifies any17 // when the parent context is cancelled (graceful shutdown notifies any
18-// attached clients with ReasonServerShutdown).18+// attached clients with ReasonServerShutdown). Client detach does not end
19+// a session: ephemeral sessions stay in memory until explicitly killed or
20+// until the daemon exits, and named sessions can also persist on disk.
19 //21 //
20 // Locking: a pane's screen/scrollback and per-client renderer shadow are22 // Locking: a pane's screen/scrollback and per-client renderer shadow are
21 // guarded by pane.mu/tab.mu as appropriate; the attached-client pointer by23 // guarded by pane.mu/tab.mu as appropriate; the attached-client pointer by
M internal/usecase/daemon/status_test.go +19 -0
@@ -363,6 +363,25 @@ func TestDetachOnSendErrorResetsScreenDefaultColors(t *testing.T) {
363363 require.Empty(t, got, "OSC 10/11 queries must be swallowed once the client that reported these colors is gone")
364364 }
365365
366+func TestDetachOnSendErrorParkPreservesScreenDefaultColors(t *testing.T) {
367+ p, release := newBlockingPTY(t)
368+ d, sess, ac, _ := newManualSessionWithPTYs(t, p)
369+ defer release()
370+ ac.resumeCapable = true
371+ d.applyTheme(sess, ac, ports.Theme{
372+ HasForeground: true, Foreground: renderer.RGB{R: 7, G: 8, B: 9},
373+ HasBackground: true, Background: renderer.RGB{R: 10, G: 11, B: 12},
374+ })
375+
376+ d.detachOnSendError(sess, ac, ac.transport())
377+
378+ tb := sess.activeTab()
379+ var got []byte
380+ tb.focusedPane().screen.OnResponse = func(b []byte) { got = append(got, b...) }
381+ tb.focusedPane().screen.Write([]byte("\x1b]10;?\a\x1b]11;?\a"))
382+ require.NotEmpty(t, got, "parked clients resume the same attachment, so screen default colors must be preserved")
383+}
384+
366385 func TestApplyThemeIgnoresReplacedClient(t *testing.T) {
367386 p, release := newBlockingPTY(t)
368387 d, sess, old, _ := newManualSessionWithPTYs(t, p)
@@ -363,6 +363,25 @@ func TestDetachOnSendErrorResetsScreenDefaultColors(t *testing.T) {
363 require.Empty(t, got, "OSC 10/11 queries must be swallowed once the client that reported these colors is gone")363 require.Empty(t, got, "OSC 10/11 queries must be swallowed once the client that reported these colors is gone")
364 }364 }
365 365
366+func TestDetachOnSendErrorParkPreservesScreenDefaultColors(t *testing.T) {
367+ p, release := newBlockingPTY(t)
368+ d, sess, ac, _ := newManualSessionWithPTYs(t, p)
369+ defer release()
370+ ac.resumeCapable = true
371+ d.applyTheme(sess, ac, ports.Theme{
372+ HasForeground: true, Foreground: renderer.RGB{R: 7, G: 8, B: 9},
373+ HasBackground: true, Background: renderer.RGB{R: 10, G: 11, B: 12},
374+ })
375+
376+ d.detachOnSendError(sess, ac, ac.transport())
377+
378+ tb := sess.activeTab()
379+ var got []byte
380+ tb.focusedPane().screen.OnResponse = func(b []byte) { got = append(got, b...) }
381+ tb.focusedPane().screen.Write([]byte("\x1b]10;?\a\x1b]11;?\a"))
382+ require.NotEmpty(t, got, "parked clients resume the same attachment, so screen default colors must be preserved")
383+}
384+
366 func TestApplyThemeIgnoresReplacedClient(t *testing.T) {385 func TestApplyThemeIgnoresReplacedClient(t *testing.T) {
367 p, release := newBlockingPTY(t)386 p, release := newBlockingPTY(t)
368 d, sess, old, _ := newManualSessionWithPTYs(t, p)387 d, sess, old, _ := newManualSessionWithPTYs(t, p)

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