fix(daemon): surface floating pane spawn failures
Thread the owning *session into failFloatingLaunch so a user-initiated floating-pane open that fails asynchronously (PTY open on the launch worker) reports a NoticeFloatingSpawn toast instead of only a log line. Background prewarm failures stay silent -- the user never asked for that launch, so a toast would be noise; if it matters, the user's later explicit open attempt will fail and surface then.
- commit
- 778ed2e02f5e7c9f5cf2ad83b44b6204480be37b
- author
- brice · · 2026-07-20 18:43 UTC
- parent
- 601c376
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +61 -4
M internal/usecase/daemon/floating.go +13 -4
| @@ -292,12 +292,12 @@ type floatingLaunchSpec struct { | |||
| 292 | 292 | func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfig, generation uint64, userOpen bool) { | |
| 293 | 293 | spec, err := d.newFloatingLaunchSpec(sess, tb, cfg, userOpen) | |
| 294 | 294 | if err != nil { | |
| 295 | - | d.failFloatingLaunch(tb, generation, userOpen, spec.sessionName, err) | |
| 295 | + | d.failFloatingLaunch(sess, tb, generation, userOpen, spec.sessionName, err) | |
| 296 | 296 | return | |
| 297 | 297 | } | |
| 298 | 298 | launch, ok := sess.registerFloatingLaunch() | |
| 299 | 299 | if !ok { | |
| 300 | - | d.failFloatingLaunch(tb, generation, userOpen, spec.sessionName, context.Canceled) | |
| 300 | + | d.failFloatingLaunch(sess, tb, generation, userOpen, spec.sessionName, context.Canceled) | |
| 301 | 301 | return | |
| 302 | 302 | } | |
| 303 | 303 | // Count the launch before its goroutine starts. It remains counted through | |
| @@ -381,7 +381,7 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||
| 381 | 381 | }() | |
| 382 | 382 | pty, err := d.ptys.Open(openCtx, spec.command, spec.args, spec.env, spec.cwd, spec.size) | |
| 383 | 383 | if err != nil { | |
| 384 | - | d.failFloatingLaunch(tb, generation, spec.userOpen, spec.sessionName, err) | |
| 384 | + | d.failFloatingLaunch(sess, tb, generation, spec.userOpen, spec.sessionName, err) | |
| 385 | 385 | return | |
| 386 | 386 | } | |
| 387 | 387 | p := newPaneWithStableID(layout.PaneID("floating"), spec.paneStableID, pty, spec.size) | |
| @@ -402,7 +402,7 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||
| 402 | 402 | d.installFloating(sess, tb, p, generation) | |
| 403 | 403 | } | |
| 404 | 404 | ||
| 405 | - | func (d *Daemon) failFloatingLaunch(tb *tab, generation uint64, userOpen bool, sessionName string, err error) { | |
| 405 | + | func (d *Daemon) failFloatingLaunch(sess *session, tb *tab, generation uint64, userOpen bool, sessionName string, err error) { | |
| 406 | 406 | tb.mu.Lock() | |
| 407 | 407 | current := tb.failFloatingLocked(generation) | |
| 408 | 408 | tb.mu.Unlock() | |
| @@ -412,6 +412,15 @@ func (d *Daemon) failFloatingLaunch(tb *tab, generation uint64, userOpen bool, s | |||
| 412 | 412 | kind = "user-open" | |
| 413 | 413 | } | |
| 414 | 414 | d.log.Warn("floating pty spawn failed", "err", err, "session", sessionName, "kind", kind) | |
| 415 | + | // A background prewarm is speculative: the user never asked for it, so | |
| 416 | + | // a toast here would be confusing noise. Only a user-initiated open | |
| 417 | + | // (the user pressed the toggle-floating key) earns a toast; if a | |
| 418 | + | // prewarm failure matters, the user's later open attempt will fail | |
| 419 | + | // too and surface then. | |
| 420 | + | if userOpen { | |
| 421 | + | d.reportError(sess, domain.UserErr(domain.NoticeFloatingSpawn, | |
| 422 | + | "couldn't open floating pane: command failed to start", err)) | |
| 423 | + | } | |
| 415 | 424 | } | |
| 416 | 425 | } | |
| 417 | 426 | ||
| @@ -292,12 +292,12 @@ type floatingLaunchSpec struct { | |||||
| 292 | func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfig, generation uint64, userOpen bool) { | 292 | func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfig, generation uint64, userOpen bool) { | ||
| 293 | spec, err := d.newFloatingLaunchSpec(sess, tb, cfg, userOpen) | 293 | spec, err := d.newFloatingLaunchSpec(sess, tb, cfg, userOpen) | ||
| 294 | if err != nil { | 294 | if err != nil { | ||
| 295 | - | d.failFloatingLaunch(tb, generation, userOpen, spec.sessionName, err) | 295 | + | d.failFloatingLaunch(sess, tb, generation, userOpen, spec.sessionName, err) |
| 296 | return | 296 | return | ||
| 297 | } | 297 | } | ||
| 298 | launch, ok := sess.registerFloatingLaunch() | 298 | launch, ok := sess.registerFloatingLaunch() | ||
| 299 | if !ok { | 299 | if !ok { | ||
| 300 | - | d.failFloatingLaunch(tb, generation, userOpen, spec.sessionName, context.Canceled) | 300 | + | d.failFloatingLaunch(sess, tb, generation, userOpen, spec.sessionName, context.Canceled) |
| 301 | return | 301 | return | ||
| 302 | } | 302 | } | ||
| 303 | // Count the launch before its goroutine starts. It remains counted through | 303 | // Count the launch before its goroutine starts. It remains counted through | ||
| @@ -381,7 +381,7 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||||
| 381 | }() | 381 | }() | ||
| 382 | pty, err := d.ptys.Open(openCtx, spec.command, spec.args, spec.env, spec.cwd, spec.size) | 382 | pty, err := d.ptys.Open(openCtx, spec.command, spec.args, spec.env, spec.cwd, spec.size) | ||
| 383 | if err != nil { | 383 | if err != nil { | ||
| 384 | - | d.failFloatingLaunch(tb, generation, spec.userOpen, spec.sessionName, err) | 384 | + | d.failFloatingLaunch(sess, tb, generation, spec.userOpen, spec.sessionName, err) |
| 385 | return | 385 | return | ||
| 386 | } | 386 | } | ||
| 387 | p := newPaneWithStableID(layout.PaneID("floating"), spec.paneStableID, pty, spec.size) | 387 | p := newPaneWithStableID(layout.PaneID("floating"), spec.paneStableID, pty, spec.size) | ||
| @@ -402,7 +402,7 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||||
| 402 | d.installFloating(sess, tb, p, generation) | 402 | d.installFloating(sess, tb, p, generation) | ||
| 403 | } | 403 | } | ||
| 404 | 404 | ||||
| 405 | - | func (d *Daemon) failFloatingLaunch(tb *tab, generation uint64, userOpen bool, sessionName string, err error) { | 405 | + | func (d *Daemon) failFloatingLaunch(sess *session, tb *tab, generation uint64, userOpen bool, sessionName string, err error) { |
| 406 | tb.mu.Lock() | 406 | tb.mu.Lock() | ||
| 407 | current := tb.failFloatingLocked(generation) | 407 | current := tb.failFloatingLocked(generation) | ||
| 408 | tb.mu.Unlock() | 408 | tb.mu.Unlock() | ||
| @@ -412,6 +412,15 @@ func (d *Daemon) failFloatingLaunch(tb *tab, generation uint64, userOpen bool, s | |||||
| 412 | kind = "user-open" | 412 | kind = "user-open" | ||
| 413 | } | 413 | } | ||
| 414 | d.log.Warn("floating pty spawn failed", "err", err, "session", sessionName, "kind", kind) | 414 | d.log.Warn("floating pty spawn failed", "err", err, "session", sessionName, "kind", kind) | ||
| 415 | + | // A background prewarm is speculative: the user never asked for it, so | |||
| 416 | + | // a toast here would be confusing noise. Only a user-initiated open | |||
| 417 | + | // (the user pressed the toggle-floating key) earns a toast; if a | |||
| 418 | + | // prewarm failure matters, the user's later open attempt will fail | |||
| 419 | + | // too and surface then. | |||
| 420 | + | if userOpen { | |||
| 421 | + | d.reportError(sess, domain.UserErr(domain.NoticeFloatingSpawn, | |||
| 422 | + | "couldn't open floating pane: command failed to start", err)) | |||
| 423 | + | } | |||
| 415 | } | 424 | } | ||
| 416 | } | 425 | } | ||
| 417 | 426 | ||||
M internal/usecase/daemon/floating_test.go +48 -0
| @@ -3,6 +3,7 @@ package daemon | |||
| 3 | 3 | import ( | |
| 4 | 4 | "bytes" | |
| 5 | 5 | "context" | |
| 6 | + | "errors" | |
| 6 | 7 | "io" | |
| 7 | 8 | "log/slog" | |
| 8 | 9 | "strings" | |
| @@ -592,6 +593,53 @@ func TestFloatingInstallLogsSessionNameSafelyDuringRename(t *testing.T) { | |||
| 592 | 593 | d.sessWg.Wait() | |
| 593 | 594 | } | |
| 594 | 595 | ||
| 596 | + | // TestFloatingAsyncSpawnFailureToastsOnlyForUserOpen drives a failed | |
| 597 | + | // asynchronous PTY open through failFloatingLaunch. A user-initiated open | |
| 598 | + | // must surface a toast; a background prewarm must stay silent (the user | |
| 599 | + | // never asked for it). The failure completes on a worker goroutine, so the | |
| 600 | + | // test waits on d.sessWg (which the launch worker joins) before asserting on | |
| 601 | + | // the test goroutine -- no require inside the worker, no sleeps. | |
| 602 | + | func TestFloatingAsyncSpawnFailureToastsOnlyForUserOpen(t *testing.T) { | |
| 603 | + | tests := []struct { | |
| 604 | + | name string | |
| 605 | + | userOpen bool | |
| 606 | + | }{ | |
| 607 | + | {name: "user-initiated open toasts", userOpen: true}, | |
| 608 | + | {name: "background prewarm stays silent", userOpen: false}, | |
| 609 | + | } | |
| 610 | + | for _, tc := range tests { | |
| 611 | + | t.Run(tc.name, func(t *testing.T) { | |
| 612 | + | spawnErr := errors.New("fork/exec: no such file or directory") | |
| 613 | + | factory := portsmocks.NewMockPTYFactory(t) | |
| 614 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). | |
| 615 | + | Return(nil, spawnErr).Once() | |
| 616 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 617 | + | tb := newFloatingTestTab(t) | |
| 618 | + | tr, _ := newCapturingTransport(t) | |
| 619 | + | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |
| 620 | + | ac.initOverlays() | |
| 621 | + | sess := &session{id: "floating-spawn", name: "work", tabs: []*tab{tb}, ctx: t.Context(), client: ac} | |
| 622 | + | ac.setSession(sess) | |
| 623 | + | ||
| 624 | + | d.startFloating(sess, tb, tc.userOpen) | |
| 625 | + | ||
| 626 | + | // The launch worker (registered via d.sessWg.Go in launchFloating) | |
| 627 | + | // runs failFloatingLaunch synchronously before returning, so | |
| 628 | + | // waiting on the group is a deterministic completion signal. | |
| 629 | + | d.sessWg.Wait() | |
| 630 | + | ||
| 631 | + | if tc.userOpen { | |
| 632 | + | toasts := awaitToastCount(t, ac, 1) | |
| 633 | + | require.Equal(t, domain.NoticeFloatingSpawn, toasts[0].Code) | |
| 634 | + | require.Equal(t, "couldn't open floating pane: command failed to start", toasts[0].Message) | |
| 635 | + | } else { | |
| 636 | + | ns, _ := visibleToasts(ac) | |
| 637 | + | require.Empty(t, ns, "background prewarm failure must not toast") | |
| 638 | + | } | |
| 639 | + | }) | |
| 640 | + | } | |
| 641 | + | } | |
| 642 | + | ||
| 595 | 643 | func TestToggleFloatingStructuralErrorsAreUserErrors(t *testing.T) { | |
| 596 | 644 | tests := []struct { | |
| 597 | 645 | name string | |
| @@ -3,6 +3,7 @@ package daemon | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bytes" | 4 | "bytes" | ||
| 5 | "context" | 5 | "context" | ||
| 6 | + | "errors" | |||
| 6 | "io" | 7 | "io" | ||
| 7 | "log/slog" | 8 | "log/slog" | ||
| 8 | "strings" | 9 | "strings" | ||
| @@ -592,6 +593,53 @@ func TestFloatingInstallLogsSessionNameSafelyDuringRename(t *testing.T) { | |||||
| 592 | d.sessWg.Wait() | 593 | d.sessWg.Wait() | ||
| 593 | } | 594 | } | ||
| 594 | 595 | ||||
| 596 | + | // TestFloatingAsyncSpawnFailureToastsOnlyForUserOpen drives a failed | |||
| 597 | + | // asynchronous PTY open through failFloatingLaunch. A user-initiated open | |||
| 598 | + | // must surface a toast; a background prewarm must stay silent (the user | |||
| 599 | + | // never asked for it). The failure completes on a worker goroutine, so the | |||
| 600 | + | // test waits on d.sessWg (which the launch worker joins) before asserting on | |||
| 601 | + | // the test goroutine -- no require inside the worker, no sleeps. | |||
| 602 | + | func TestFloatingAsyncSpawnFailureToastsOnlyForUserOpen(t *testing.T) { | |||
| 603 | + | tests := []struct { | |||
| 604 | + | name string | |||
| 605 | + | userOpen bool | |||
| 606 | + | }{ | |||
| 607 | + | {name: "user-initiated open toasts", userOpen: true}, | |||
| 608 | + | {name: "background prewarm stays silent", userOpen: false}, | |||
| 609 | + | } | |||
| 610 | + | for _, tc := range tests { | |||
| 611 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 612 | + | spawnErr := errors.New("fork/exec: no such file or directory") | |||
| 613 | + | factory := portsmocks.NewMockPTYFactory(t) | |||
| 614 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). | |||
| 615 | + | Return(nil, spawnErr).Once() | |||
| 616 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 617 | + | tb := newFloatingTestTab(t) | |||
| 618 | + | tr, _ := newCapturingTransport(t) | |||
| 619 | + | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |||
| 620 | + | ac.initOverlays() | |||
| 621 | + | sess := &session{id: "floating-spawn", name: "work", tabs: []*tab{tb}, ctx: t.Context(), client: ac} | |||
| 622 | + | ac.setSession(sess) | |||
| 623 | + | ||||
| 624 | + | d.startFloating(sess, tb, tc.userOpen) | |||
| 625 | + | ||||
| 626 | + | // The launch worker (registered via d.sessWg.Go in launchFloating) | |||
| 627 | + | // runs failFloatingLaunch synchronously before returning, so | |||
| 628 | + | // waiting on the group is a deterministic completion signal. | |||
| 629 | + | d.sessWg.Wait() | |||
| 630 | + | ||||
| 631 | + | if tc.userOpen { | |||
| 632 | + | toasts := awaitToastCount(t, ac, 1) | |||
| 633 | + | require.Equal(t, domain.NoticeFloatingSpawn, toasts[0].Code) | |||
| 634 | + | require.Equal(t, "couldn't open floating pane: command failed to start", toasts[0].Message) | |||
| 635 | + | } else { | |||
| 636 | + | ns, _ := visibleToasts(ac) | |||
| 637 | + | require.Empty(t, ns, "background prewarm failure must not toast") | |||
| 638 | + | } | |||
| 639 | + | }) | |||
| 640 | + | } | |||
| 641 | + | } | |||
| 642 | + | ||||
| 595 | func TestToggleFloatingStructuralErrorsAreUserErrors(t *testing.T) { | 643 | func TestToggleFloatingStructuralErrorsAreUserErrors(t *testing.T) { | ||
| 596 | tests := []struct { | 644 | tests := []struct { | ||
| 597 | name string | 645 | name string | ||