fix(daemon): harden floating pane cleanup
- commit
- c3765482dc84a6e72fae67ce9bdc0078ecf8b4d6
- author
- brice · · 2026-07-10 00:32 UTC
- committer
- brice · · 2026-07-10 05:40 UTC
- parent
- f1db0f5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +152 -30
M internal/usecase/daemon/floating.go +19 -15
| @@ -206,12 +206,12 @@ func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfi | |||
| 206 | 206 | cwd = live | |
| 207 | 207 | } | |
| 208 | 208 | } | |
| 209 | - | if cwd == "" && d.dirOrHome != nil { | |
| 210 | - | cwd = d.dirOrHome("") | |
| 209 | + | if d.dirOrHome != nil { | |
| 210 | + | cwd = d.dirOrHome(cwd) | |
| 211 | 211 | } | |
| 212 | 212 | paneStableID, err := newStableID("p") | |
| 213 | 213 | if err != nil { | |
| 214 | - | d.failFloatingLaunch(sess, tb, generation, userOpen, err) | |
| 214 | + | d.failFloatingLaunch(tb, generation, userOpen, name, err) | |
| 215 | 215 | return | |
| 216 | 216 | } | |
| 217 | 217 | env := d.childEnv(name, tabStableID, paneStableID, term) | |
| @@ -223,7 +223,7 @@ func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfi | |||
| 223 | 223 | go func() { | |
| 224 | 224 | pty, openErr := d.ptys.Open(command, args, env, cwd, sz) | |
| 225 | 225 | if openErr != nil { | |
| 226 | - | d.failFloatingLaunch(sess, tb, generation, userOpen, openErr) | |
| 226 | + | d.failFloatingLaunch(tb, generation, userOpen, name, openErr) | |
| 227 | 227 | return | |
| 228 | 228 | } | |
| 229 | 229 | p := newPaneWithStableID(layout.PaneID("floating"), paneStableID, pty, sz) | |
| @@ -239,7 +239,7 @@ func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfi | |||
| 239 | 239 | }() | |
| 240 | 240 | } | |
| 241 | 241 | ||
| 242 | - | func (d *Daemon) failFloatingLaunch(sess *session, tb *tab, generation uint64, userOpen bool, err error) { | |
| 242 | + | func (d *Daemon) failFloatingLaunch(tb *tab, generation uint64, userOpen bool, sessionName string, err error) { | |
| 243 | 243 | tb.mu.Lock() | |
| 244 | 244 | current := tb.failFloatingLocked(generation) | |
| 245 | 245 | tb.mu.Unlock() | |
| @@ -248,7 +248,7 @@ func (d *Daemon) failFloatingLaunch(sess *session, tb *tab, generation uint64, u | |||
| 248 | 248 | if userOpen { | |
| 249 | 249 | kind = "user-open" | |
| 250 | 250 | } | |
| 251 | - | d.log.Warn("floating pty spawn failed", "err", err, "session", sess.name, "kind", kind) | |
| 251 | + | d.log.Warn("floating pty spawn failed", "err", err, "session", sessionName, "kind", kind) | |
| 252 | 252 | } | |
| 253 | 253 | } | |
| 254 | 254 | ||
| @@ -260,10 +260,7 @@ func (d *Daemon) installFloating(sess *session, tb *tab, p *pane, generation uin | |||
| 260 | 260 | visible := installed && tb.floating.state == floatingVisible | |
| 261 | 261 | tb.mu.Unlock() | |
| 262 | 262 | if !installed { | |
| 263 | - | if p.cancel != nil { | |
| 264 | - | p.cancel() | |
| 265 | - | } | |
| 266 | - | _ = p.pty.Close() | |
| 263 | + | closeFloatingPane(p) | |
| 267 | 264 | return | |
| 268 | 265 | } | |
| 269 | 266 | p.onExit = func() { d.reapFloating(sess, tb, p, generation) } | |
| @@ -288,9 +285,7 @@ func (d *Daemon) reapFloating(sess *session, tb *tab, p *pane, generation uint64 | |||
| 288 | 285 | if !cleared { | |
| 289 | 286 | return | |
| 290 | 287 | } | |
| 291 | - | if p.cancel != nil { | |
| 292 | - | p.cancel() | |
| 293 | - | } | |
| 288 | + | closeFloatingPane(p) | |
| 294 | 289 | if visible { | |
| 295 | 290 | sess.mu.Lock() | |
| 296 | 291 | ac := sess.client | |
| @@ -314,14 +309,23 @@ func (d *Daemon) teardownFloating(tb *tab) { | |||
| 314 | 309 | tb.floating.desiredVisible = false | |
| 315 | 310 | tb.floating.launch = domain.FloatingConfig{} | |
| 316 | 311 | tb.mu.Unlock() | |
| 317 | - | if p != nil { | |
| 312 | + | closeFloatingPane(p) | |
| 313 | + | } | |
| 314 | + | ||
| 315 | + | // closeFloatingPane releases a floating runtime at most once. It is called | |
| 316 | + | // only after a generation/identity check has detached the pane from its slot. | |
| 317 | + | func closeFloatingPane(p *pane) { | |
| 318 | + | if p == nil { | |
| 319 | + | return | |
| 320 | + | } | |
| 321 | + | p.floatingCloseOnce.Do(func() { | |
| 318 | 322 | if p.cancel != nil { | |
| 319 | 323 | p.cancel() | |
| 320 | 324 | } | |
| 321 | 325 | if p.pty != nil { | |
| 322 | 326 | _ = p.pty.Close() | |
| 323 | 327 | } | |
| 324 | - | } | |
| 328 | + | }) | |
| 325 | 329 | } | |
| 326 | 330 | ||
| 327 | 331 | func floatingCommandFallback(command, shell string) string { | |
| @@ -206,12 +206,12 @@ func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfi | |||||
| 206 | cwd = live | 206 | cwd = live | ||
| 207 | } | 207 | } | ||
| 208 | } | 208 | } | ||
| 209 | - | if cwd == "" && d.dirOrHome != nil { | 209 | + | if d.dirOrHome != nil { |
| 210 | - | cwd = d.dirOrHome("") | 210 | + | cwd = d.dirOrHome(cwd) |
| 211 | } | 211 | } | ||
| 212 | paneStableID, err := newStableID("p") | 212 | paneStableID, err := newStableID("p") | ||
| 213 | if err != nil { | 213 | if err != nil { | ||
| 214 | - | d.failFloatingLaunch(sess, tb, generation, userOpen, err) | 214 | + | d.failFloatingLaunch(tb, generation, userOpen, name, err) |
| 215 | return | 215 | return | ||
| 216 | } | 216 | } | ||
| 217 | env := d.childEnv(name, tabStableID, paneStableID, term) | 217 | env := d.childEnv(name, tabStableID, paneStableID, term) | ||
| @@ -223,7 +223,7 @@ func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfi | |||||
| 223 | go func() { | 223 | go func() { | ||
| 224 | pty, openErr := d.ptys.Open(command, args, env, cwd, sz) | 224 | pty, openErr := d.ptys.Open(command, args, env, cwd, sz) | ||
| 225 | if openErr != nil { | 225 | if openErr != nil { | ||
| 226 | - | d.failFloatingLaunch(sess, tb, generation, userOpen, openErr) | 226 | + | d.failFloatingLaunch(tb, generation, userOpen, name, openErr) |
| 227 | return | 227 | return | ||
| 228 | } | 228 | } | ||
| 229 | p := newPaneWithStableID(layout.PaneID("floating"), paneStableID, pty, sz) | 229 | p := newPaneWithStableID(layout.PaneID("floating"), paneStableID, pty, sz) | ||
| @@ -239,7 +239,7 @@ func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfi | |||||
| 239 | }() | 239 | }() | ||
| 240 | } | 240 | } | ||
| 241 | 241 | ||||
| 242 | - | func (d *Daemon) failFloatingLaunch(sess *session, tb *tab, generation uint64, userOpen bool, err error) { | 242 | + | func (d *Daemon) failFloatingLaunch(tb *tab, generation uint64, userOpen bool, sessionName string, err error) { |
| 243 | tb.mu.Lock() | 243 | tb.mu.Lock() | ||
| 244 | current := tb.failFloatingLocked(generation) | 244 | current := tb.failFloatingLocked(generation) | ||
| 245 | tb.mu.Unlock() | 245 | tb.mu.Unlock() | ||
| @@ -248,7 +248,7 @@ func (d *Daemon) failFloatingLaunch(sess *session, tb *tab, generation uint64, u | |||||
| 248 | if userOpen { | 248 | if userOpen { | ||
| 249 | kind = "user-open" | 249 | kind = "user-open" | ||
| 250 | } | 250 | } | ||
| 251 | - | d.log.Warn("floating pty spawn failed", "err", err, "session", sess.name, "kind", kind) | 251 | + | d.log.Warn("floating pty spawn failed", "err", err, "session", sessionName, "kind", kind) |
| 252 | } | 252 | } | ||
| 253 | } | 253 | } | ||
| 254 | 254 | ||||
| @@ -260,10 +260,7 @@ func (d *Daemon) installFloating(sess *session, tb *tab, p *pane, generation uin | |||||
| 260 | visible := installed && tb.floating.state == floatingVisible | 260 | visible := installed && tb.floating.state == floatingVisible | ||
| 261 | tb.mu.Unlock() | 261 | tb.mu.Unlock() | ||
| 262 | if !installed { | 262 | if !installed { | ||
| 263 | - | if p.cancel != nil { | 263 | + | closeFloatingPane(p) |
| 264 | - | p.cancel() | |||
| 265 | - | } | |||
| 266 | - | _ = p.pty.Close() | |||
| 267 | return | 264 | return | ||
| 268 | } | 265 | } | ||
| 269 | p.onExit = func() { d.reapFloating(sess, tb, p, generation) } | 266 | p.onExit = func() { d.reapFloating(sess, tb, p, generation) } | ||
| @@ -288,9 +285,7 @@ func (d *Daemon) reapFloating(sess *session, tb *tab, p *pane, generation uint64 | |||||
| 288 | if !cleared { | 285 | if !cleared { | ||
| 289 | return | 286 | return | ||
| 290 | } | 287 | } | ||
| 291 | - | if p.cancel != nil { | 288 | + | closeFloatingPane(p) |
| 292 | - | p.cancel() | |||
| 293 | - | } | |||
| 294 | if visible { | 289 | if visible { | ||
| 295 | sess.mu.Lock() | 290 | sess.mu.Lock() | ||
| 296 | ac := sess.client | 291 | ac := sess.client | ||
| @@ -314,14 +309,23 @@ func (d *Daemon) teardownFloating(tb *tab) { | |||||
| 314 | tb.floating.desiredVisible = false | 309 | tb.floating.desiredVisible = false | ||
| 315 | tb.floating.launch = domain.FloatingConfig{} | 310 | tb.floating.launch = domain.FloatingConfig{} | ||
| 316 | tb.mu.Unlock() | 311 | tb.mu.Unlock() | ||
| 317 | - | if p != nil { | 312 | + | closeFloatingPane(p) |
| 313 | + | } | |||
| 314 | + | ||||
| 315 | + | // closeFloatingPane releases a floating runtime at most once. It is called | |||
| 316 | + | // only after a generation/identity check has detached the pane from its slot. | |||
| 317 | + | func closeFloatingPane(p *pane) { | |||
| 318 | + | if p == nil { | |||
| 319 | + | return | |||
| 320 | + | } | |||
| 321 | + | p.floatingCloseOnce.Do(func() { | |||
| 318 | if p.cancel != nil { | 322 | if p.cancel != nil { | ||
| 319 | p.cancel() | 323 | p.cancel() | ||
| 320 | } | 324 | } | ||
| 321 | if p.pty != nil { | 325 | if p.pty != nil { | ||
| 322 | _ = p.pty.Close() | 326 | _ = p.pty.Close() | ||
| 323 | } | 327 | } | ||
| 324 | - | } | 328 | + | }) |
| 325 | } | 329 | } | ||
| 326 | 330 | ||||
| 327 | func floatingCommandFallback(command, shell string) string { | 331 | func floatingCommandFallback(command, shell string) string { | ||
M internal/usecase/daemon/floating_test.go +119 -2
| @@ -1,8 +1,11 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "bytes" | |
| 4 | 5 | "context" | |
| 5 | 6 | "io" | |
| 7 | + | "log/slog" | |
| 8 | + | "strings" | |
| 6 | 9 | "sync" | |
| 7 | 10 | "testing" | |
| 8 | 11 | "time" | |
| @@ -129,9 +132,10 @@ func TestFloatingLifecycleCapturesLaunchBeforeOpenAndDoesNotHoldTabLock(t *testi | |||
| 129 | 132 | }).Once() | |
| 130 | 133 | d := newTestDaemon(t, factory, stubClock{}) | |
| 131 | 134 | d.shell = "/bin/custom-shell" | |
| 135 | + | cwd := t.TempDir() | |
| 132 | 136 | tb := newTabWithStableID("t_stable", "p_normal", newBlockingPanePTY(t), domain.Size{Cols: 100, Rows: 40}) | |
| 133 | 137 | tb.ctx, tb.cancel = context.WithCancel(t.Context()) | |
| 134 | - | sess := &session{name: "work", cwd: "/captured", tabs: []*tab{tb}, ctx: t.Context()} | |
| 138 | + | sess := &session{name: "work", cwd: cwd, tabs: []*tab{tb}, ctx: t.Context()} | |
| 135 | 139 | d.ApplyConfig(domain.Config{Floating: domain.FloatingConfig{Command: "btop --utf", Width: 50, Height: 50}}) | |
| 136 | 140 | d.ensureFloatingWarm(sess, tb) | |
| 137 | 141 | // Open has started while this goroutine owns tab.mu: an external factory | |
| @@ -152,7 +156,7 @@ func TestFloatingLifecycleCapturesLaunchBeforeOpenAndDoesNotHoldTabLock(t *testi | |||
| 152 | 156 | } | |
| 153 | 157 | require.Equal(t, "/bin/custom-shell", gotCommand) | |
| 154 | 158 | require.Equal(t, []string{"-lc", "btop --utf"}, gotArgs) | |
| 155 | - | require.Equal(t, "/captured", gotDir) | |
| 159 | + | require.Equal(t, cwd, gotDir) | |
| 156 | 160 | require.Equal(t, domain.Size{Cols: 48, Rows: 18}, gotSize) | |
| 157 | 161 | d.teardownFloating(tb) | |
| 158 | 162 | unblock() | |
| @@ -200,6 +204,20 @@ func TestFloatingLifecycleStaleSuccessAndOldExitCannotReplaceCurrentSlot(t *test | |||
| 200 | 204 | tb.mu.Unlock() | |
| 201 | 205 | } | |
| 202 | 206 | ||
| 207 | + | func TestFloatingReapAndTeardownCloseMatchingPaneOnce(t *testing.T) { | |
| 208 | + | pty, _ := newBlockingPTY(t) | |
| 209 | + | tb := newFloatingTestTab(t) | |
| 210 | + | p := newPane(layout.PaneID("floating"), pty, domain.Size{Cols: 10, Rows: 10}) | |
| 211 | + | tb.mu.Lock() | |
| 212 | + | g := tb.beginFloatingWarmLocked(domain.FloatingConfig{}, true) | |
| 213 | + | require.True(t, tb.installFloatingLocked(p, g)) | |
| 214 | + | tb.mu.Unlock() | |
| 215 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 216 | + | d.reapFloating(&session{}, tb, p, g) | |
| 217 | + | d.teardownFloating(tb) | |
| 218 | + | pty.AssertNumberOfCalls(t, "Close", 1) | |
| 219 | + | } | |
| 220 | + | ||
| 203 | 221 | func TestFloatingTeardownClosesInstalledPaneOnce(t *testing.T) { | |
| 204 | 222 | pty, _ := newBlockingPTY(t) | |
| 205 | 223 | tb := newFloatingTestTab(t) | |
| @@ -228,6 +246,7 @@ func TestFloatingEOFRepaintsVisibleSlotOnly(t *testing.T) { | |||
| 228 | 246 | normal.mu.Unlock() | |
| 229 | 247 | ||
| 230 | 248 | floatingPTY := portsmocks.NewMockPTY(t) | |
| 249 | + | floatingPTY.EXPECT().Close().Return(nil).Maybe() | |
| 231 | 250 | releaseEOF := make(chan struct{}) | |
| 232 | 251 | floatingPTY.EXPECT().Read(mock.Anything).RunAndReturn(func([]byte) (int, error) { | |
| 233 | 252 | <-releaseEOF | |
| @@ -331,6 +350,104 @@ func newBlockingPanePTY(t *testing.T) *portsmocks.MockPTY { | |||
| 331 | 350 | return p | |
| 332 | 351 | } | |
| 333 | 352 | ||
| 353 | + | func TestFloatingLaunchUsesLiveOrValidatedSessionCwd(t *testing.T) { | |
| 354 | + | tests := []struct { | |
| 355 | + | name string | |
| 356 | + | sessionCwd string | |
| 357 | + | liveCwd string | |
| 358 | + | wantCwd string | |
| 359 | + | }{ | |
| 360 | + | {name: "valid session cwd", sessionCwd: "/valid", wantCwd: "/valid"}, | |
| 361 | + | {name: "invalid session cwd falls home", sessionCwd: "/invalid", wantCwd: "/home/test"}, | |
| 362 | + | {name: "live focused cwd", sessionCwd: "/invalid", liveCwd: "/live", wantCwd: "/live"}, | |
| 363 | + | } | |
| 364 | + | for _, tt := range tests { | |
| 365 | + | t.Run(tt.name, func(t *testing.T) { | |
| 366 | + | factory := portsmocks.NewMockPTYFactory(t) | |
| 367 | + | opened := make(chan string, 1) | |
| 368 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |
| 369 | + | func(_ string, _ []string, _ []string, cwd string, _ domain.Size) (ports.PTY, error) { | |
| 370 | + | opened <- cwd | |
| 371 | + | return nil, io.ErrUnexpectedEOF | |
| 372 | + | }).Once() | |
| 373 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 374 | + | d.dirOrHome = func(cwd string) string { | |
| 375 | + | if cwd == "/invalid" { | |
| 376 | + | return "/home/test" | |
| 377 | + | } | |
| 378 | + | return cwd | |
| 379 | + | } | |
| 380 | + | if tt.liveCwd != "" { | |
| 381 | + | d.procCwd = func(int) (string, error) { return tt.liveCwd, nil } | |
| 382 | + | } | |
| 383 | + | tb := newFloatingTestTab(t) | |
| 384 | + | sess := &session{name: "work", cwd: tt.sessionCwd, tabs: []*tab{tb}, ctx: t.Context()} | |
| 385 | + | d.startFloating(sess, tb, false) | |
| 386 | + | select { | |
| 387 | + | case got := <-opened: | |
| 388 | + | require.Equal(t, tt.wantCwd, got) | |
| 389 | + | case <-time.After(time.Second): | |
| 390 | + | t.Fatal("floating PTY was not opened") | |
| 391 | + | } | |
| 392 | + | }) | |
| 393 | + | } | |
| 394 | + | } | |
| 395 | + | ||
| 396 | + | type floatingLogHandler struct { | |
| 397 | + | slog.Handler | |
| 398 | + | handled chan struct{} | |
| 399 | + | } | |
| 400 | + | ||
| 401 | + | func (h floatingLogHandler) Handle(ctx context.Context, r slog.Record) error { | |
| 402 | + | err := h.Handler.Handle(ctx, r) | |
| 403 | + | close(h.handled) | |
| 404 | + | return err | |
| 405 | + | } | |
| 406 | + | ||
| 407 | + | func TestFloatingLaunchFailureCapturesSessionNameBeforeConcurrentRename(t *testing.T) { | |
| 408 | + | factory := portsmocks.NewMockPTYFactory(t) | |
| 409 | + | opened := make(chan struct{}) | |
| 410 | + | release := make(chan struct{}) | |
| 411 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |
| 412 | + | func(string, []string, []string, string, domain.Size) (ports.PTY, error) { | |
| 413 | + | close(opened) | |
| 414 | + | <-release | |
| 415 | + | return nil, io.ErrUnexpectedEOF | |
| 416 | + | }).Once() | |
| 417 | + | var logs bytes.Buffer | |
| 418 | + | handled := make(chan struct{}) | |
| 419 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 420 | + | d.log = slog.New(floatingLogHandler{Handler: slog.NewTextHandler(&logs, nil), handled: handled}) | |
| 421 | + | tb := newFloatingTestTab(t) | |
| 422 | + | sess := &session{name: "captured", tabs: []*tab{tb}, ctx: t.Context()} | |
| 423 | + | d.startFloating(sess, tb, false) | |
| 424 | + | <-opened | |
| 425 | + | stopRename := make(chan struct{}) | |
| 426 | + | renameDone := make(chan struct{}) | |
| 427 | + | go func() { | |
| 428 | + | defer close(renameDone) | |
| 429 | + | for { | |
| 430 | + | select { | |
| 431 | + | case <-stopRename: | |
| 432 | + | return | |
| 433 | + | default: | |
| 434 | + | } | |
| 435 | + | sess.mu.Lock() | |
| 436 | + | sess.name = "renamed" | |
| 437 | + | sess.mu.Unlock() | |
| 438 | + | } | |
| 439 | + | }() | |
| 440 | + | close(release) | |
| 441 | + | select { | |
| 442 | + | case <-handled: | |
| 443 | + | case <-time.After(time.Second): | |
| 444 | + | t.Fatal("floating launch failure was not logged") | |
| 445 | + | } | |
| 446 | + | close(stopRename) | |
| 447 | + | <-renameDone | |
| 448 | + | require.True(t, strings.Contains(logs.String(), "session=captured"), logs.String()) | |
| 449 | + | } | |
| 450 | + | ||
| 334 | 451 | func TestFloatingToggleUsesVisibleTarget(t *testing.T) { | |
| 335 | 452 | d := newTestDaemon(t, nil, stubClock{}) | |
| 336 | 453 | tb := newFloatingTestTab(t) | |
| @@ -1,8 +1,11 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "bytes" | |||
| 4 | "context" | 5 | "context" | ||
| 5 | "io" | 6 | "io" | ||
| 7 | + | "log/slog" | |||
| 8 | + | "strings" | |||
| 6 | "sync" | 9 | "sync" | ||
| 7 | "testing" | 10 | "testing" | ||
| 8 | "time" | 11 | "time" | ||
| @@ -129,9 +132,10 @@ func TestFloatingLifecycleCapturesLaunchBeforeOpenAndDoesNotHoldTabLock(t *testi | |||||
| 129 | }).Once() | 132 | }).Once() | ||
| 130 | d := newTestDaemon(t, factory, stubClock{}) | 133 | d := newTestDaemon(t, factory, stubClock{}) | ||
| 131 | d.shell = "/bin/custom-shell" | 134 | d.shell = "/bin/custom-shell" | ||
| 135 | + | cwd := t.TempDir() | |||
| 132 | tb := newTabWithStableID("t_stable", "p_normal", newBlockingPanePTY(t), domain.Size{Cols: 100, Rows: 40}) | 136 | tb := newTabWithStableID("t_stable", "p_normal", newBlockingPanePTY(t), domain.Size{Cols: 100, Rows: 40}) | ||
| 133 | tb.ctx, tb.cancel = context.WithCancel(t.Context()) | 137 | tb.ctx, tb.cancel = context.WithCancel(t.Context()) | ||
| 134 | - | sess := &session{name: "work", cwd: "/captured", tabs: []*tab{tb}, ctx: t.Context()} | 138 | + | sess := &session{name: "work", cwd: cwd, tabs: []*tab{tb}, ctx: t.Context()} |
| 135 | d.ApplyConfig(domain.Config{Floating: domain.FloatingConfig{Command: "btop --utf", Width: 50, Height: 50}}) | 139 | d.ApplyConfig(domain.Config{Floating: domain.FloatingConfig{Command: "btop --utf", Width: 50, Height: 50}}) | ||
| 136 | d.ensureFloatingWarm(sess, tb) | 140 | d.ensureFloatingWarm(sess, tb) | ||
| 137 | // Open has started while this goroutine owns tab.mu: an external factory | 141 | // Open has started while this goroutine owns tab.mu: an external factory | ||
| @@ -152,7 +156,7 @@ func TestFloatingLifecycleCapturesLaunchBeforeOpenAndDoesNotHoldTabLock(t *testi | |||||
| 152 | } | 156 | } | ||
| 153 | require.Equal(t, "/bin/custom-shell", gotCommand) | 157 | require.Equal(t, "/bin/custom-shell", gotCommand) | ||
| 154 | require.Equal(t, []string{"-lc", "btop --utf"}, gotArgs) | 158 | require.Equal(t, []string{"-lc", "btop --utf"}, gotArgs) | ||
| 155 | - | require.Equal(t, "/captured", gotDir) | 159 | + | require.Equal(t, cwd, gotDir) |
| 156 | require.Equal(t, domain.Size{Cols: 48, Rows: 18}, gotSize) | 160 | require.Equal(t, domain.Size{Cols: 48, Rows: 18}, gotSize) | ||
| 157 | d.teardownFloating(tb) | 161 | d.teardownFloating(tb) | ||
| 158 | unblock() | 162 | unblock() | ||
| @@ -200,6 +204,20 @@ func TestFloatingLifecycleStaleSuccessAndOldExitCannotReplaceCurrentSlot(t *test | |||||
| 200 | tb.mu.Unlock() | 204 | tb.mu.Unlock() | ||
| 201 | } | 205 | } | ||
| 202 | 206 | ||||
| 207 | + | func TestFloatingReapAndTeardownCloseMatchingPaneOnce(t *testing.T) { | |||
| 208 | + | pty, _ := newBlockingPTY(t) | |||
| 209 | + | tb := newFloatingTestTab(t) | |||
| 210 | + | p := newPane(layout.PaneID("floating"), pty, domain.Size{Cols: 10, Rows: 10}) | |||
| 211 | + | tb.mu.Lock() | |||
| 212 | + | g := tb.beginFloatingWarmLocked(domain.FloatingConfig{}, true) | |||
| 213 | + | require.True(t, tb.installFloatingLocked(p, g)) | |||
| 214 | + | tb.mu.Unlock() | |||
| 215 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 216 | + | d.reapFloating(&session{}, tb, p, g) | |||
| 217 | + | d.teardownFloating(tb) | |||
| 218 | + | pty.AssertNumberOfCalls(t, "Close", 1) | |||
| 219 | + | } | |||
| 220 | + | ||||
| 203 | func TestFloatingTeardownClosesInstalledPaneOnce(t *testing.T) { | 221 | func TestFloatingTeardownClosesInstalledPaneOnce(t *testing.T) { | ||
| 204 | pty, _ := newBlockingPTY(t) | 222 | pty, _ := newBlockingPTY(t) | ||
| 205 | tb := newFloatingTestTab(t) | 223 | tb := newFloatingTestTab(t) | ||
| @@ -228,6 +246,7 @@ func TestFloatingEOFRepaintsVisibleSlotOnly(t *testing.T) { | |||||
| 228 | normal.mu.Unlock() | 246 | normal.mu.Unlock() | ||
| 229 | 247 | ||||
| 230 | floatingPTY := portsmocks.NewMockPTY(t) | 248 | floatingPTY := portsmocks.NewMockPTY(t) | ||
| 249 | + | floatingPTY.EXPECT().Close().Return(nil).Maybe() | |||
| 231 | releaseEOF := make(chan struct{}) | 250 | releaseEOF := make(chan struct{}) | ||
| 232 | floatingPTY.EXPECT().Read(mock.Anything).RunAndReturn(func([]byte) (int, error) { | 251 | floatingPTY.EXPECT().Read(mock.Anything).RunAndReturn(func([]byte) (int, error) { | ||
| 233 | <-releaseEOF | 252 | <-releaseEOF | ||
| @@ -331,6 +350,104 @@ func newBlockingPanePTY(t *testing.T) *portsmocks.MockPTY { | |||||
| 331 | return p | 350 | return p | ||
| 332 | } | 351 | } | ||
| 333 | 352 | ||||
| 353 | + | func TestFloatingLaunchUsesLiveOrValidatedSessionCwd(t *testing.T) { | |||
| 354 | + | tests := []struct { | |||
| 355 | + | name string | |||
| 356 | + | sessionCwd string | |||
| 357 | + | liveCwd string | |||
| 358 | + | wantCwd string | |||
| 359 | + | }{ | |||
| 360 | + | {name: "valid session cwd", sessionCwd: "/valid", wantCwd: "/valid"}, | |||
| 361 | + | {name: "invalid session cwd falls home", sessionCwd: "/invalid", wantCwd: "/home/test"}, | |||
| 362 | + | {name: "live focused cwd", sessionCwd: "/invalid", liveCwd: "/live", wantCwd: "/live"}, | |||
| 363 | + | } | |||
| 364 | + | for _, tt := range tests { | |||
| 365 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 366 | + | factory := portsmocks.NewMockPTYFactory(t) | |||
| 367 | + | opened := make(chan string, 1) | |||
| 368 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |||
| 369 | + | func(_ string, _ []string, _ []string, cwd string, _ domain.Size) (ports.PTY, error) { | |||
| 370 | + | opened <- cwd | |||
| 371 | + | return nil, io.ErrUnexpectedEOF | |||
| 372 | + | }).Once() | |||
| 373 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 374 | + | d.dirOrHome = func(cwd string) string { | |||
| 375 | + | if cwd == "/invalid" { | |||
| 376 | + | return "/home/test" | |||
| 377 | + | } | |||
| 378 | + | return cwd | |||
| 379 | + | } | |||
| 380 | + | if tt.liveCwd != "" { | |||
| 381 | + | d.procCwd = func(int) (string, error) { return tt.liveCwd, nil } | |||
| 382 | + | } | |||
| 383 | + | tb := newFloatingTestTab(t) | |||
| 384 | + | sess := &session{name: "work", cwd: tt.sessionCwd, tabs: []*tab{tb}, ctx: t.Context()} | |||
| 385 | + | d.startFloating(sess, tb, false) | |||
| 386 | + | select { | |||
| 387 | + | case got := <-opened: | |||
| 388 | + | require.Equal(t, tt.wantCwd, got) | |||
| 389 | + | case <-time.After(time.Second): | |||
| 390 | + | t.Fatal("floating PTY was not opened") | |||
| 391 | + | } | |||
| 392 | + | }) | |||
| 393 | + | } | |||
| 394 | + | } | |||
| 395 | + | ||||
| 396 | + | type floatingLogHandler struct { | |||
| 397 | + | slog.Handler | |||
| 398 | + | handled chan struct{} | |||
| 399 | + | } | |||
| 400 | + | ||||
| 401 | + | func (h floatingLogHandler) Handle(ctx context.Context, r slog.Record) error { | |||
| 402 | + | err := h.Handler.Handle(ctx, r) | |||
| 403 | + | close(h.handled) | |||
| 404 | + | return err | |||
| 405 | + | } | |||
| 406 | + | ||||
| 407 | + | func TestFloatingLaunchFailureCapturesSessionNameBeforeConcurrentRename(t *testing.T) { | |||
| 408 | + | factory := portsmocks.NewMockPTYFactory(t) | |||
| 409 | + | opened := make(chan struct{}) | |||
| 410 | + | release := make(chan struct{}) | |||
| 411 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |||
| 412 | + | func(string, []string, []string, string, domain.Size) (ports.PTY, error) { | |||
| 413 | + | close(opened) | |||
| 414 | + | <-release | |||
| 415 | + | return nil, io.ErrUnexpectedEOF | |||
| 416 | + | }).Once() | |||
| 417 | + | var logs bytes.Buffer | |||
| 418 | + | handled := make(chan struct{}) | |||
| 419 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 420 | + | d.log = slog.New(floatingLogHandler{Handler: slog.NewTextHandler(&logs, nil), handled: handled}) | |||
| 421 | + | tb := newFloatingTestTab(t) | |||
| 422 | + | sess := &session{name: "captured", tabs: []*tab{tb}, ctx: t.Context()} | |||
| 423 | + | d.startFloating(sess, tb, false) | |||
| 424 | + | <-opened | |||
| 425 | + | stopRename := make(chan struct{}) | |||
| 426 | + | renameDone := make(chan struct{}) | |||
| 427 | + | go func() { | |||
| 428 | + | defer close(renameDone) | |||
| 429 | + | for { | |||
| 430 | + | select { | |||
| 431 | + | case <-stopRename: | |||
| 432 | + | return | |||
| 433 | + | default: | |||
| 434 | + | } | |||
| 435 | + | sess.mu.Lock() | |||
| 436 | + | sess.name = "renamed" | |||
| 437 | + | sess.mu.Unlock() | |||
| 438 | + | } | |||
| 439 | + | }() | |||
| 440 | + | close(release) | |||
| 441 | + | select { | |||
| 442 | + | case <-handled: | |||
| 443 | + | case <-time.After(time.Second): | |||
| 444 | + | t.Fatal("floating launch failure was not logged") | |||
| 445 | + | } | |||
| 446 | + | close(stopRename) | |||
| 447 | + | <-renameDone | |||
| 448 | + | require.True(t, strings.Contains(logs.String(), "session=captured"), logs.String()) | |||
| 449 | + | } | |||
| 450 | + | ||||
| 334 | func TestFloatingToggleUsesVisibleTarget(t *testing.T) { | 451 | func TestFloatingToggleUsesVisibleTarget(t *testing.T) { | ||
| 335 | d := newTestDaemon(t, nil, stubClock{}) | 452 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 336 | tb := newFloatingTestTab(t) | 453 | tb := newFloatingTestTab(t) | ||
M internal/usecase/daemon/pane.go +14 -13
| @@ -16,19 +16,20 @@ import ( | |||
| 16 | 16 | // attachedClient.sendMu > Daemon.mu > session.mu > tab.mu > pane.mu. | |
| 17 | 17 | // The PTY reader takes only pane.mu, so child output never waits on client IO. | |
| 18 | 18 | type pane struct { | |
| 19 | - | id layout.PaneID | |
| 20 | - | stableID string | |
| 21 | - | pty ports.PTY | |
| 22 | - | mu sync.Mutex // guards screen, scrollback, syncGen, rect, and title | |
| 23 | - | screen *vt.Screen | |
| 24 | - | scrollback *scopy.Scrollback | |
| 25 | - | dirty chan struct{} | |
| 26 | - | flush chan struct{} | |
| 27 | - | syncGen uint64 | |
| 28 | - | rect domain.Rect | |
| 29 | - | title paneTitleState | |
| 30 | - | ctx context.Context | |
| 31 | - | cancel context.CancelFunc | |
| 19 | + | id layout.PaneID | |
| 20 | + | stableID string | |
| 21 | + | pty ports.PTY | |
| 22 | + | mu sync.Mutex // guards screen, scrollback, syncGen, rect, and title | |
| 23 | + | screen *vt.Screen | |
| 24 | + | scrollback *scopy.Scrollback | |
| 25 | + | dirty chan struct{} | |
| 26 | + | flush chan struct{} | |
| 27 | + | syncGen uint64 | |
| 28 | + | rect domain.Rect | |
| 29 | + | title paneTitleState | |
| 30 | + | ctx context.Context | |
| 31 | + | cancel context.CancelFunc | |
| 32 | + | floatingCloseOnce sync.Once // used only by floating lifecycle teardown paths | |
| 32 | 33 | // onExit is set before the reader starts and never changed. Floating panes | |
| 33 | 34 | // use it to reap their independent slot without touching the layout tree. | |
| 34 | 35 | onExit func() | |
| @@ -16,19 +16,20 @@ import ( | |||||
| 16 | // attachedClient.sendMu > Daemon.mu > session.mu > tab.mu > pane.mu. | 16 | // attachedClient.sendMu > Daemon.mu > session.mu > tab.mu > pane.mu. | ||
| 17 | // The PTY reader takes only pane.mu, so child output never waits on client IO. | 17 | // The PTY reader takes only pane.mu, so child output never waits on client IO. | ||
| 18 | type pane struct { | 18 | type pane struct { | ||
| 19 | - | id layout.PaneID | 19 | + | id layout.PaneID |
| 20 | - | stableID string | 20 | + | stableID string |
| 21 | - | pty ports.PTY | 21 | + | pty ports.PTY |
| 22 | - | mu sync.Mutex // guards screen, scrollback, syncGen, rect, and title | 22 | + | mu sync.Mutex // guards screen, scrollback, syncGen, rect, and title |
| 23 | - | screen *vt.Screen | 23 | + | screen *vt.Screen |
| 24 | - | scrollback *scopy.Scrollback | 24 | + | scrollback *scopy.Scrollback |
| 25 | - | dirty chan struct{} | 25 | + | dirty chan struct{} |
| 26 | - | flush chan struct{} | 26 | + | flush chan struct{} |
| 27 | - | syncGen uint64 | 27 | + | syncGen uint64 |
| 28 | - | rect domain.Rect | 28 | + | rect domain.Rect |
| 29 | - | title paneTitleState | 29 | + | title paneTitleState |
| 30 | - | ctx context.Context | 30 | + | ctx context.Context |
| 31 | - | cancel context.CancelFunc | 31 | + | cancel context.CancelFunc |
| 32 | + | floatingCloseOnce sync.Once // used only by floating lifecycle teardown paths | |||
| 32 | // onExit is set before the reader starts and never changed. Floating panes | 33 | // onExit is set before the reader starts and never changed. Floating panes | ||
| 33 | // use it to reap their independent slot without touching the layout tree. | 34 | // use it to reap their independent slot without touching the layout tree. | ||
| 34 | onExit func() | 35 | onExit func() | ||