merge(next): integrate floating popup lifecycle fixes
3 files · +289 -5
M internal/usecase/daemon/floating.go +21 -5
| @@ -200,10 +200,12 @@ func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | |||
| 200 | 200 | } | |
| 201 | 201 | cfg := d.currentFloatingConfig() | |
| 202 | 202 | tb.mu.Lock() | |
| 203 | + | wasVisible := tb.floating.state == floatingVisible | |
| 203 | 204 | start, generation := tb.toggleFloatingLocked() | |
| 204 | 205 | // desiredVisible is meaningful only while a launch is warming. Installed | |
| 205 | 206 | // slots derive their resize and paint work from the resulting slot state. | |
| 206 | 207 | visible := tb.floating.state == floatingVisible | |
| 208 | + | hidden := wasVisible && tb.floating.state == floatingHidden | |
| 207 | 209 | tb.mu.Unlock() | |
| 208 | 210 | if start { | |
| 209 | 211 | d.launchFloating(sess, tb, cfg, generation, true) | |
| @@ -212,6 +214,10 @@ func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | |||
| 212 | 214 | } | |
| 213 | 215 | return nil | |
| 214 | 216 | } | |
| 217 | + | if hidden { | |
| 218 | + | d.invalidateRender(sess, ac, true, "floating.go") | |
| 219 | + | return nil | |
| 220 | + | } | |
| 215 | 221 | if visible { | |
| 216 | 222 | tb.mu.Lock() | |
| 217 | 223 | size := domain.Size{Cols: tb.size.Cols, Rows: tb.size.Rows + 2} | |
| @@ -300,7 +306,7 @@ func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfi | |||
| 300 | 306 | // before a late Open completion has either been rejected or fully joined. | |
| 301 | 307 | d.sessWg.Go(func() { | |
| 302 | 308 | defer sess.finishFloatingLaunch(launch) | |
| 303 | - | d.openAndInstallFloating(sess, tb, spec, generation, launch) | |
| 309 | + | d.openAndInstallFloating(sess, tb, spec, generation) | |
| 304 | 310 | }) | |
| 305 | 311 | } | |
| 306 | 312 | ||
| @@ -357,7 +363,7 @@ func (d *Daemon) newFloatingLaunchSpec(sess *session, tb *tab, cfg domain.Floati | |||
| 357 | 363 | ||
| 358 | 364 | // openAndInstallFloating runs entirely in the launch worker. No PTY operation | |
| 359 | 365 | // occurs under tb.mu; the pane is fully initialized before publication. | |
| 360 | - | func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLaunchSpec, generation uint64, launch *floatingLaunch) { | |
| 366 | + | func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLaunchSpec, generation uint64) { | |
| 361 | 367 | // Open is context-aware and is intentionally called without any daemon, | |
| 362 | 368 | // session, tab, or launch-ownership lock. Cancellation makes teardown | |
| 363 | 369 | // bounded even if the adapter is waiting to create the child. | |
| @@ -366,9 +372,12 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||
| 366 | 372 | } | |
| 367 | 373 | openCtx, cancelOpen := context.WithCancel(spec.parentCtx) | |
| 368 | 374 | stopSessionCancel := context.AfterFunc(sess.ctx, cancelOpen) | |
| 375 | + | ownedByPane := false | |
| 369 | 376 | defer func() { | |
| 370 | - | stopSessionCancel() | |
| 371 | - | cancelOpen() | |
| 377 | + | if !ownedByPane { | |
| 378 | + | stopSessionCancel() | |
| 379 | + | cancelOpen() | |
| 380 | + | } | |
| 372 | 381 | }() | |
| 373 | 382 | pty, err := d.ptys.Open(openCtx, spec.command, spec.args, spec.env, spec.cwd, spec.size) | |
| 374 | 383 | if err != nil { | |
| @@ -379,7 +388,14 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||
| 379 | 388 | p.rect = spec.geometry.Inner | |
| 380 | 389 | p.popupGeometry = spec.geometry | |
| 381 | 390 | p.title.displayFallback = spec.fallback | |
| 382 | - | p.ctx, p.cancel = context.WithCancel(spec.parentCtx) | |
| 391 | + | // CommandContext owns the child through openCtx, so retain both this context | |
| 392 | + | // and its cancellation until the installed pane is reaped or torn down. | |
| 393 | + | p.ctx = openCtx | |
| 394 | + | p.cancel = func() { | |
| 395 | + | stopSessionCancel() | |
| 396 | + | cancelOpen() | |
| 397 | + | } | |
| 398 | + | ownedByPane = true | |
| 383 | 399 | // The reader may run as soon as install returns; make its exit policy | |
| 384 | 400 | // immutable before publishing the pane to the slot. | |
| 385 | 401 | p.onExit = func() { d.reapFloating(sess, tb, p, generation) } | |
| @@ -200,10 +200,12 @@ func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | |||||
| 200 | } | 200 | } | ||
| 201 | cfg := d.currentFloatingConfig() | 201 | cfg := d.currentFloatingConfig() | ||
| 202 | tb.mu.Lock() | 202 | tb.mu.Lock() | ||
| 203 | + | wasVisible := tb.floating.state == floatingVisible | |||
| 203 | start, generation := tb.toggleFloatingLocked() | 204 | start, generation := tb.toggleFloatingLocked() | ||
| 204 | // desiredVisible is meaningful only while a launch is warming. Installed | 205 | // desiredVisible is meaningful only while a launch is warming. Installed | ||
| 205 | // slots derive their resize and paint work from the resulting slot state. | 206 | // slots derive their resize and paint work from the resulting slot state. | ||
| 206 | visible := tb.floating.state == floatingVisible | 207 | visible := tb.floating.state == floatingVisible | ||
| 208 | + | hidden := wasVisible && tb.floating.state == floatingHidden | |||
| 207 | tb.mu.Unlock() | 209 | tb.mu.Unlock() | ||
| 208 | if start { | 210 | if start { | ||
| 209 | d.launchFloating(sess, tb, cfg, generation, true) | 211 | d.launchFloating(sess, tb, cfg, generation, true) | ||
| @@ -212,6 +214,10 @@ func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | |||||
| 212 | } | 214 | } | ||
| 213 | return nil | 215 | return nil | ||
| 214 | } | 216 | } | ||
| 217 | + | if hidden { | |||
| 218 | + | d.invalidateRender(sess, ac, true, "floating.go") | |||
| 219 | + | return nil | |||
| 220 | + | } | |||
| 215 | if visible { | 221 | if visible { | ||
| 216 | tb.mu.Lock() | 222 | tb.mu.Lock() | ||
| 217 | size := domain.Size{Cols: tb.size.Cols, Rows: tb.size.Rows + 2} | 223 | size := domain.Size{Cols: tb.size.Cols, Rows: tb.size.Rows + 2} | ||
| @@ -300,7 +306,7 @@ func (d *Daemon) launchFloating(sess *session, tb *tab, cfg domain.FloatingConfi | |||||
| 300 | // before a late Open completion has either been rejected or fully joined. | 306 | // before a late Open completion has either been rejected or fully joined. | ||
| 301 | d.sessWg.Go(func() { | 307 | d.sessWg.Go(func() { | ||
| 302 | defer sess.finishFloatingLaunch(launch) | 308 | defer sess.finishFloatingLaunch(launch) | ||
| 303 | - | d.openAndInstallFloating(sess, tb, spec, generation, launch) | 309 | + | d.openAndInstallFloating(sess, tb, spec, generation) |
| 304 | }) | 310 | }) | ||
| 305 | } | 311 | } | ||
| 306 | 312 | ||||
| @@ -357,7 +363,7 @@ func (d *Daemon) newFloatingLaunchSpec(sess *session, tb *tab, cfg domain.Floati | |||||
| 357 | 363 | ||||
| 358 | // openAndInstallFloating runs entirely in the launch worker. No PTY operation | 364 | // openAndInstallFloating runs entirely in the launch worker. No PTY operation | ||
| 359 | // occurs under tb.mu; the pane is fully initialized before publication. | 365 | // occurs under tb.mu; the pane is fully initialized before publication. | ||
| 360 | - | func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLaunchSpec, generation uint64, launch *floatingLaunch) { | 366 | + | func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLaunchSpec, generation uint64) { |
| 361 | // Open is context-aware and is intentionally called without any daemon, | 367 | // Open is context-aware and is intentionally called without any daemon, | ||
| 362 | // session, tab, or launch-ownership lock. Cancellation makes teardown | 368 | // session, tab, or launch-ownership lock. Cancellation makes teardown | ||
| 363 | // bounded even if the adapter is waiting to create the child. | 369 | // bounded even if the adapter is waiting to create the child. | ||
| @@ -366,9 +372,12 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||||
| 366 | } | 372 | } | ||
| 367 | openCtx, cancelOpen := context.WithCancel(spec.parentCtx) | 373 | openCtx, cancelOpen := context.WithCancel(spec.parentCtx) | ||
| 368 | stopSessionCancel := context.AfterFunc(sess.ctx, cancelOpen) | 374 | stopSessionCancel := context.AfterFunc(sess.ctx, cancelOpen) | ||
| 375 | + | ownedByPane := false | |||
| 369 | defer func() { | 376 | defer func() { | ||
| 370 | - | stopSessionCancel() | 377 | + | if !ownedByPane { |
| 371 | - | cancelOpen() | 378 | + | stopSessionCancel() |
| 379 | + | cancelOpen() | |||
| 380 | + | } | |||
| 372 | }() | 381 | }() | ||
| 373 | 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) | ||
| 374 | if err != nil { | 383 | if err != nil { | ||
| @@ -379,7 +388,14 @@ func (d *Daemon) openAndInstallFloating(sess *session, tb *tab, spec floatingLau | |||||
| 379 | p.rect = spec.geometry.Inner | 388 | p.rect = spec.geometry.Inner | ||
| 380 | p.popupGeometry = spec.geometry | 389 | p.popupGeometry = spec.geometry | ||
| 381 | p.title.displayFallback = spec.fallback | 390 | p.title.displayFallback = spec.fallback | ||
| 382 | - | p.ctx, p.cancel = context.WithCancel(spec.parentCtx) | 391 | + | // CommandContext owns the child through openCtx, so retain both this context |
| 392 | + | // and its cancellation until the installed pane is reaped or torn down. | |||
| 393 | + | p.ctx = openCtx | |||
| 394 | + | p.cancel = func() { | |||
| 395 | + | stopSessionCancel() | |||
| 396 | + | cancelOpen() | |||
| 397 | + | } | |||
| 398 | + | ownedByPane = true | |||
| 383 | // The reader may run as soon as install returns; make its exit policy | 399 | // The reader may run as soon as install returns; make its exit policy | ||
| 384 | // immutable before publishing the pane to the slot. | 400 | // immutable before publishing the pane to the slot. | ||
| 385 | p.onExit = func() { d.reapFloating(sess, tb, p, generation) } | 401 | p.onExit = func() { d.reapFloating(sess, tb, p, generation) } | ||
M internal/usecase/daemon/floating_open_test.go +203 -0
| @@ -14,6 +14,209 @@ import ( | |||
| 14 | 14 | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |
| 15 | 15 | ) | |
| 16 | 16 | ||
| 17 | + | type contextAwareFloatingPTY struct { | |
| 18 | + | ctx context.Context | |
| 19 | + | readerStarted chan struct{} | |
| 20 | + | closed chan struct{} | |
| 21 | + | readerOnce sync.Once | |
| 22 | + | closeOnce sync.Once | |
| 23 | + | closeCount int | |
| 24 | + | } | |
| 25 | + | ||
| 26 | + | func newContextAwareFloatingPTY() *contextAwareFloatingPTY { | |
| 27 | + | return &contextAwareFloatingPTY{ | |
| 28 | + | readerStarted: make(chan struct{}), | |
| 29 | + | closed: make(chan struct{}), | |
| 30 | + | } | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | func (p *contextAwareFloatingPTY) Read([]byte) (int, error) { | |
| 34 | + | p.readerOnce.Do(func() { close(p.readerStarted) }) | |
| 35 | + | <-p.ctx.Done() | |
| 36 | + | return 0, p.ctx.Err() | |
| 37 | + | } | |
| 38 | + | ||
| 39 | + | func (*contextAwareFloatingPTY) Write(b []byte) (int, error) { return len(b), nil } | |
| 40 | + | func (*contextAwareFloatingPTY) Resize(domain.Size) error { return nil } | |
| 41 | + | func (*contextAwareFloatingPTY) Pid() int { return 1 } | |
| 42 | + | func (*contextAwareFloatingPTY) ForegroundPgid() (int, error) { | |
| 43 | + | return 1, nil | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | func (p *contextAwareFloatingPTY) Close() error { | |
| 47 | + | p.closeOnce.Do(func() { | |
| 48 | + | p.closeCount++ | |
| 49 | + | close(p.closed) | |
| 50 | + | }) | |
| 51 | + | return nil | |
| 52 | + | } | |
| 53 | + | ||
| 54 | + | type contextAwareFloatingFactory struct { | |
| 55 | + | pty *contextAwareFloatingPTY | |
| 56 | + | opened chan context.Context | |
| 57 | + | err error | |
| 58 | + | onOpen func() | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | func (f *contextAwareFloatingFactory) Open(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) { | |
| 62 | + | if f.pty != nil { | |
| 63 | + | f.pty.ctx = ctx | |
| 64 | + | } | |
| 65 | + | f.opened <- ctx | |
| 66 | + | if f.onOpen != nil { | |
| 67 | + | f.onOpen() | |
| 68 | + | } | |
| 69 | + | return f.pty, f.err | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | func TestFloatingSuccessfulOpenTransfersContextOwnershipToPane(t *testing.T) { | |
| 73 | + | teardowns := []struct { | |
| 74 | + | name string | |
| 75 | + | teardown func(*Daemon, *session, *tab) | |
| 76 | + | }{ | |
| 77 | + | { | |
| 78 | + | name: "floating pane", | |
| 79 | + | teardown: func(d *Daemon, _ *session, tb *tab) { | |
| 80 | + | d.teardownFloating(tb, nil) | |
| 81 | + | }, | |
| 82 | + | }, | |
| 83 | + | { | |
| 84 | + | name: "tab", | |
| 85 | + | teardown: func(_ *Daemon, _ *session, tb *tab) { | |
| 86 | + | tb.closeAllPanes() | |
| 87 | + | }, | |
| 88 | + | }, | |
| 89 | + | { | |
| 90 | + | name: "session", | |
| 91 | + | teardown: func(d *Daemon, sess *session, _ *tab) { | |
| 92 | + | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, false)) | |
| 93 | + | }, | |
| 94 | + | }, | |
| 95 | + | } | |
| 96 | + | for _, tt := range teardowns { | |
| 97 | + | t.Run(tt.name, func(t *testing.T) { | |
| 98 | + | pty := newContextAwareFloatingPTY() | |
| 99 | + | factory := &contextAwareFloatingFactory{pty: pty, opened: make(chan context.Context, 1)} | |
| 100 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 101 | + | tb := newFloatingTestTab(t) | |
| 102 | + | tabCtx, cancelTab := context.WithCancel(t.Context()) | |
| 103 | + | tb.ctx, tb.cancel = tabCtx, cancelTab | |
| 104 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |
| 105 | + | sess := &session{id: "context-" + domain.SessionID(tt.name), name: "work", tabs: []*tab{tb}, ctx: sessCtx, cancel: cancelSession} | |
| 106 | + | d.mu.Lock() | |
| 107 | + | d.sessions[sess.id] = sess | |
| 108 | + | d.mu.Unlock() | |
| 109 | + | tb.mu.Lock() | |
| 110 | + | generation := tb.beginFloatingWarmLocked(true) | |
| 111 | + | tb.mu.Unlock() | |
| 112 | + | ||
| 113 | + | d.openAndInstallFloating(sess, tb, floatingLaunchSpec{ | |
| 114 | + | sessionName: "work", | |
| 115 | + | size: domain.Size{Cols: 20, Rows: 8}, | |
| 116 | + | geometry: floatingGeometry{Inner: domain.Rect{Width: 20, Height: 8}}, | |
| 117 | + | paneStableID: "p_floating", | |
| 118 | + | parentCtx: tabCtx, | |
| 119 | + | }, generation) | |
| 120 | + | ||
| 121 | + | openCtx := <-factory.opened | |
| 122 | + | require.NoError(t, openCtx.Err(), "the PTY context must outlive the launch worker") | |
| 123 | + | tb.mu.Lock() | |
| 124 | + | floating := tb.floating.pane | |
| 125 | + | tb.mu.Unlock() | |
| 126 | + | require.NotNil(t, floating) | |
| 127 | + | sess.mu.Lock() | |
| 128 | + | sess.client = &attachedClient{} | |
| 129 | + | sess.mu.Unlock() | |
| 130 | + | require.True(t, d.paneRenderable(sess, tb, floating), "installed visible popup must remain renderable") | |
| 131 | + | sess.mu.Lock() | |
| 132 | + | sess.client = nil | |
| 133 | + | sess.mu.Unlock() | |
| 134 | + | select { | |
| 135 | + | case <-pty.readerStarted: | |
| 136 | + | case <-time.After(time.Second): | |
| 137 | + | t.Fatal("floating reader did not start") | |
| 138 | + | } | |
| 139 | + | ||
| 140 | + | tt.teardown(d, sess, tb) | |
| 141 | + | select { | |
| 142 | + | case <-openCtx.Done(): | |
| 143 | + | case <-time.After(time.Second): | |
| 144 | + | t.Fatal("floating PTY context was not cancelled by teardown") | |
| 145 | + | } | |
| 146 | + | select { | |
| 147 | + | case <-pty.closed: | |
| 148 | + | case <-time.After(time.Second): | |
| 149 | + | t.Fatal("floating PTY was not closed by teardown") | |
| 150 | + | } | |
| 151 | + | d.sessWg.Wait() | |
| 152 | + | require.Equal(t, 1, pty.closeCount, "floating PTY must close exactly once") | |
| 153 | + | }) | |
| 154 | + | } | |
| 155 | + | } | |
| 156 | + | ||
| 157 | + | func TestFloatingFailedAndStaleOpenCancelContext(t *testing.T) { | |
| 158 | + | tests := []struct { | |
| 159 | + | name string | |
| 160 | + | setup func(*tab, *contextAwareFloatingFactory) | |
| 161 | + | }{ | |
| 162 | + | { | |
| 163 | + | name: "failed Open", | |
| 164 | + | setup: func(_ *tab, factory *contextAwareFloatingFactory) { | |
| 165 | + | factory.err = context.Canceled | |
| 166 | + | }, | |
| 167 | + | }, | |
| 168 | + | { | |
| 169 | + | name: "stale install", | |
| 170 | + | setup: func(tb *tab, factory *contextAwareFloatingFactory) { | |
| 171 | + | factory.onOpen = func() { | |
| 172 | + | tb.mu.Lock() | |
| 173 | + | tb.takeFloatingLocked() | |
| 174 | + | tb.mu.Unlock() | |
| 175 | + | } | |
| 176 | + | }, | |
| 177 | + | }, | |
| 178 | + | } | |
| 179 | + | for _, tt := range tests { | |
| 180 | + | t.Run(tt.name, func(t *testing.T) { | |
| 181 | + | pty := newContextAwareFloatingPTY() | |
| 182 | + | factory := &contextAwareFloatingFactory{pty: pty, opened: make(chan context.Context, 1)} | |
| 183 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 184 | + | tb := newFloatingTestTab(t) | |
| 185 | + | sess := &session{name: "work", tabs: []*tab{tb}, ctx: t.Context()} | |
| 186 | + | tb.mu.Lock() | |
| 187 | + | generation := tb.beginFloatingWarmLocked(true) | |
| 188 | + | tb.mu.Unlock() | |
| 189 | + | tt.setup(tb, factory) | |
| 190 | + | ||
| 191 | + | d.openAndInstallFloating(sess, tb, floatingLaunchSpec{ | |
| 192 | + | sessionName: "work", | |
| 193 | + | size: domain.Size{Cols: 20, Rows: 8}, | |
| 194 | + | geometry: floatingGeometry{Inner: domain.Rect{Width: 20, Height: 8}}, | |
| 195 | + | paneStableID: "p_floating", | |
| 196 | + | parentCtx: tb.ctx, | |
| 197 | + | }, generation) | |
| 198 | + | ||
| 199 | + | openCtx := <-factory.opened | |
| 200 | + | select { | |
| 201 | + | case <-openCtx.Done(): | |
| 202 | + | case <-time.After(time.Second): | |
| 203 | + | t.Fatal("failed or stale floating Open retained its context") | |
| 204 | + | } | |
| 205 | + | tb.mu.Lock() | |
| 206 | + | require.Nil(t, tb.floating.pane) | |
| 207 | + | tb.mu.Unlock() | |
| 208 | + | if tt.name == "stale install" { | |
| 209 | + | select { | |
| 210 | + | case <-pty.closed: | |
| 211 | + | case <-time.After(time.Second): | |
| 212 | + | t.Fatal("stale floating PTY was not closed") | |
| 213 | + | } | |
| 214 | + | require.Equal(t, 1, pty.closeCount) | |
| 215 | + | } | |
| 216 | + | }) | |
| 217 | + | } | |
| 218 | + | } | |
| 219 | + | ||
| 17 | 220 | func TestFloatingOpenCancellationBoundsSessionTeardownAndClosesLatePTY(t *testing.T) { | |
| 18 | 221 | factory := portsmocks.NewMockPTYFactory(t) | |
| 19 | 222 | opened := make(chan struct{}) | |
| @@ -14,6 +14,209 @@ import ( | |||||
| 14 | portsmocks "github.com/bnema/vev/internal/ports/mocks" | 14 | portsmocks "github.com/bnema/vev/internal/ports/mocks" | ||
| 15 | ) | 15 | ) | ||
| 16 | 16 | ||||
| 17 | + | type contextAwareFloatingPTY struct { | |||
| 18 | + | ctx context.Context | |||
| 19 | + | readerStarted chan struct{} | |||
| 20 | + | closed chan struct{} | |||
| 21 | + | readerOnce sync.Once | |||
| 22 | + | closeOnce sync.Once | |||
| 23 | + | closeCount int | |||
| 24 | + | } | |||
| 25 | + | ||||
| 26 | + | func newContextAwareFloatingPTY() *contextAwareFloatingPTY { | |||
| 27 | + | return &contextAwareFloatingPTY{ | |||
| 28 | + | readerStarted: make(chan struct{}), | |||
| 29 | + | closed: make(chan struct{}), | |||
| 30 | + | } | |||
| 31 | + | } | |||
| 32 | + | ||||
| 33 | + | func (p *contextAwareFloatingPTY) Read([]byte) (int, error) { | |||
| 34 | + | p.readerOnce.Do(func() { close(p.readerStarted) }) | |||
| 35 | + | <-p.ctx.Done() | |||
| 36 | + | return 0, p.ctx.Err() | |||
| 37 | + | } | |||
| 38 | + | ||||
| 39 | + | func (*contextAwareFloatingPTY) Write(b []byte) (int, error) { return len(b), nil } | |||
| 40 | + | func (*contextAwareFloatingPTY) Resize(domain.Size) error { return nil } | |||
| 41 | + | func (*contextAwareFloatingPTY) Pid() int { return 1 } | |||
| 42 | + | func (*contextAwareFloatingPTY) ForegroundPgid() (int, error) { | |||
| 43 | + | return 1, nil | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | func (p *contextAwareFloatingPTY) Close() error { | |||
| 47 | + | p.closeOnce.Do(func() { | |||
| 48 | + | p.closeCount++ | |||
| 49 | + | close(p.closed) | |||
| 50 | + | }) | |||
| 51 | + | return nil | |||
| 52 | + | } | |||
| 53 | + | ||||
| 54 | + | type contextAwareFloatingFactory struct { | |||
| 55 | + | pty *contextAwareFloatingPTY | |||
| 56 | + | opened chan context.Context | |||
| 57 | + | err error | |||
| 58 | + | onOpen func() | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | func (f *contextAwareFloatingFactory) Open(ctx context.Context, _ string, _ []string, _ []string, _ string, _ domain.Size) (ports.PTY, error) { | |||
| 62 | + | if f.pty != nil { | |||
| 63 | + | f.pty.ctx = ctx | |||
| 64 | + | } | |||
| 65 | + | f.opened <- ctx | |||
| 66 | + | if f.onOpen != nil { | |||
| 67 | + | f.onOpen() | |||
| 68 | + | } | |||
| 69 | + | return f.pty, f.err | |||
| 70 | + | } | |||
| 71 | + | ||||
| 72 | + | func TestFloatingSuccessfulOpenTransfersContextOwnershipToPane(t *testing.T) { | |||
| 73 | + | teardowns := []struct { | |||
| 74 | + | name string | |||
| 75 | + | teardown func(*Daemon, *session, *tab) | |||
| 76 | + | }{ | |||
| 77 | + | { | |||
| 78 | + | name: "floating pane", | |||
| 79 | + | teardown: func(d *Daemon, _ *session, tb *tab) { | |||
| 80 | + | d.teardownFloating(tb, nil) | |||
| 81 | + | }, | |||
| 82 | + | }, | |||
| 83 | + | { | |||
| 84 | + | name: "tab", | |||
| 85 | + | teardown: func(_ *Daemon, _ *session, tb *tab) { | |||
| 86 | + | tb.closeAllPanes() | |||
| 87 | + | }, | |||
| 88 | + | }, | |||
| 89 | + | { | |||
| 90 | + | name: "session", | |||
| 91 | + | teardown: func(d *Daemon, sess *session, _ *tab) { | |||
| 92 | + | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, false)) | |||
| 93 | + | }, | |||
| 94 | + | }, | |||
| 95 | + | } | |||
| 96 | + | for _, tt := range teardowns { | |||
| 97 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 98 | + | pty := newContextAwareFloatingPTY() | |||
| 99 | + | factory := &contextAwareFloatingFactory{pty: pty, opened: make(chan context.Context, 1)} | |||
| 100 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 101 | + | tb := newFloatingTestTab(t) | |||
| 102 | + | tabCtx, cancelTab := context.WithCancel(t.Context()) | |||
| 103 | + | tb.ctx, tb.cancel = tabCtx, cancelTab | |||
| 104 | + | sessCtx, cancelSession := context.WithCancel(t.Context()) | |||
| 105 | + | sess := &session{id: "context-" + domain.SessionID(tt.name), name: "work", tabs: []*tab{tb}, ctx: sessCtx, cancel: cancelSession} | |||
| 106 | + | d.mu.Lock() | |||
| 107 | + | d.sessions[sess.id] = sess | |||
| 108 | + | d.mu.Unlock() | |||
| 109 | + | tb.mu.Lock() | |||
| 110 | + | generation := tb.beginFloatingWarmLocked(true) | |||
| 111 | + | tb.mu.Unlock() | |||
| 112 | + | ||||
| 113 | + | d.openAndInstallFloating(sess, tb, floatingLaunchSpec{ | |||
| 114 | + | sessionName: "work", | |||
| 115 | + | size: domain.Size{Cols: 20, Rows: 8}, | |||
| 116 | + | geometry: floatingGeometry{Inner: domain.Rect{Width: 20, Height: 8}}, | |||
| 117 | + | paneStableID: "p_floating", | |||
| 118 | + | parentCtx: tabCtx, | |||
| 119 | + | }, generation) | |||
| 120 | + | ||||
| 121 | + | openCtx := <-factory.opened | |||
| 122 | + | require.NoError(t, openCtx.Err(), "the PTY context must outlive the launch worker") | |||
| 123 | + | tb.mu.Lock() | |||
| 124 | + | floating := tb.floating.pane | |||
| 125 | + | tb.mu.Unlock() | |||
| 126 | + | require.NotNil(t, floating) | |||
| 127 | + | sess.mu.Lock() | |||
| 128 | + | sess.client = &attachedClient{} | |||
| 129 | + | sess.mu.Unlock() | |||
| 130 | + | require.True(t, d.paneRenderable(sess, tb, floating), "installed visible popup must remain renderable") | |||
| 131 | + | sess.mu.Lock() | |||
| 132 | + | sess.client = nil | |||
| 133 | + | sess.mu.Unlock() | |||
| 134 | + | select { | |||
| 135 | + | case <-pty.readerStarted: | |||
| 136 | + | case <-time.After(time.Second): | |||
| 137 | + | t.Fatal("floating reader did not start") | |||
| 138 | + | } | |||
| 139 | + | ||||
| 140 | + | tt.teardown(d, sess, tb) | |||
| 141 | + | select { | |||
| 142 | + | case <-openCtx.Done(): | |||
| 143 | + | case <-time.After(time.Second): | |||
| 144 | + | t.Fatal("floating PTY context was not cancelled by teardown") | |||
| 145 | + | } | |||
| 146 | + | select { | |||
| 147 | + | case <-pty.closed: | |||
| 148 | + | case <-time.After(time.Second): | |||
| 149 | + | t.Fatal("floating PTY was not closed by teardown") | |||
| 150 | + | } | |||
| 151 | + | d.sessWg.Wait() | |||
| 152 | + | require.Equal(t, 1, pty.closeCount, "floating PTY must close exactly once") | |||
| 153 | + | }) | |||
| 154 | + | } | |||
| 155 | + | } | |||
| 156 | + | ||||
| 157 | + | func TestFloatingFailedAndStaleOpenCancelContext(t *testing.T) { | |||
| 158 | + | tests := []struct { | |||
| 159 | + | name string | |||
| 160 | + | setup func(*tab, *contextAwareFloatingFactory) | |||
| 161 | + | }{ | |||
| 162 | + | { | |||
| 163 | + | name: "failed Open", | |||
| 164 | + | setup: func(_ *tab, factory *contextAwareFloatingFactory) { | |||
| 165 | + | factory.err = context.Canceled | |||
| 166 | + | }, | |||
| 167 | + | }, | |||
| 168 | + | { | |||
| 169 | + | name: "stale install", | |||
| 170 | + | setup: func(tb *tab, factory *contextAwareFloatingFactory) { | |||
| 171 | + | factory.onOpen = func() { | |||
| 172 | + | tb.mu.Lock() | |||
| 173 | + | tb.takeFloatingLocked() | |||
| 174 | + | tb.mu.Unlock() | |||
| 175 | + | } | |||
| 176 | + | }, | |||
| 177 | + | }, | |||
| 178 | + | } | |||
| 179 | + | for _, tt := range tests { | |||
| 180 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 181 | + | pty := newContextAwareFloatingPTY() | |||
| 182 | + | factory := &contextAwareFloatingFactory{pty: pty, opened: make(chan context.Context, 1)} | |||
| 183 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 184 | + | tb := newFloatingTestTab(t) | |||
| 185 | + | sess := &session{name: "work", tabs: []*tab{tb}, ctx: t.Context()} | |||
| 186 | + | tb.mu.Lock() | |||
| 187 | + | generation := tb.beginFloatingWarmLocked(true) | |||
| 188 | + | tb.mu.Unlock() | |||
| 189 | + | tt.setup(tb, factory) | |||
| 190 | + | ||||
| 191 | + | d.openAndInstallFloating(sess, tb, floatingLaunchSpec{ | |||
| 192 | + | sessionName: "work", | |||
| 193 | + | size: domain.Size{Cols: 20, Rows: 8}, | |||
| 194 | + | geometry: floatingGeometry{Inner: domain.Rect{Width: 20, Height: 8}}, | |||
| 195 | + | paneStableID: "p_floating", | |||
| 196 | + | parentCtx: tb.ctx, | |||
| 197 | + | }, generation) | |||
| 198 | + | ||||
| 199 | + | openCtx := <-factory.opened | |||
| 200 | + | select { | |||
| 201 | + | case <-openCtx.Done(): | |||
| 202 | + | case <-time.After(time.Second): | |||
| 203 | + | t.Fatal("failed or stale floating Open retained its context") | |||
| 204 | + | } | |||
| 205 | + | tb.mu.Lock() | |||
| 206 | + | require.Nil(t, tb.floating.pane) | |||
| 207 | + | tb.mu.Unlock() | |||
| 208 | + | if tt.name == "stale install" { | |||
| 209 | + | select { | |||
| 210 | + | case <-pty.closed: | |||
| 211 | + | case <-time.After(time.Second): | |||
| 212 | + | t.Fatal("stale floating PTY was not closed") | |||
| 213 | + | } | |||
| 214 | + | require.Equal(t, 1, pty.closeCount) | |||
| 215 | + | } | |||
| 216 | + | }) | |||
| 217 | + | } | |||
| 218 | + | } | |||
| 219 | + | ||||
| 17 | func TestFloatingOpenCancellationBoundsSessionTeardownAndClosesLatePTY(t *testing.T) { | 220 | func TestFloatingOpenCancellationBoundsSessionTeardownAndClosesLatePTY(t *testing.T) { | ||
| 18 | factory := portsmocks.NewMockPTYFactory(t) | 221 | factory := portsmocks.NewMockPTYFactory(t) | ||
| 19 | opened := make(chan struct{}) | 222 | opened := make(chan struct{}) | ||
M internal/usecase/daemon/input_test.go +65 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "encoding/base64" | |
| 5 | 6 | "fmt" | |
| 6 | 7 | "strings" | |
| @@ -169,6 +170,70 @@ func TestAltCForwardsToPTY(t *testing.T) { | |||
| 169 | 170 | releasePTY() | |
| 170 | 171 | } | |
| 171 | 172 | ||
| 173 | + | func TestAltFToggleRetainedFloatingPaneRepaintsImmediately(t *testing.T) { | |
| 174 | + | normal, releaseNormal := newBlockingPTY(t) | |
| 175 | + | floatingPTY, releaseFloating := newBlockingPTY(t) | |
| 176 | + | defer releaseNormal() | |
| 177 | + | defer releaseFloating() | |
| 178 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, normal) | |
| 179 | + | client := vt.NewScreen(80, 25) | |
| 180 | + | floatingCtx, cancelFloating := context.WithCancel(sess.ctx) | |
| 181 | + | defer cancelFloating() | |
| 182 | + | floating := newPane("floating", floatingPTY, domain.Size{Cols: 20, Rows: 5}) | |
| 183 | + | floating.ctx = floatingCtx | |
| 184 | + | floating.screen.Write([]byte("popup-content")) | |
| 185 | + | installTestFloating(sess.activeTab(), floating, false) | |
| 186 | + | ||
| 187 | + | // Establish the client shadow while the retained popup is hidden. | |
| 188 | + | d.paint(sess, ac, true) | |
| 189 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |
| 190 | + | ||
| 191 | + | // Route the real Alt+F binding. Showing a retained pane must paint once. | |
| 192 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |
| 193 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |
| 194 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |
| 195 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "┌") | |
| 196 | + | select { | |
| 197 | + | case extra := <-sends: | |
| 198 | + | t.Fatalf("show emitted duplicate output: %#v", extra) | |
| 199 | + | default: | |
| 200 | + | } | |
| 201 | + | ||
| 202 | + | // The second real key must repaint immediately, rather than waiting for | |
| 203 | + | // unrelated output, while retaining the existing PTY and context. | |
| 204 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |
| 205 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |
| 206 | + | require.NotContains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |
| 207 | + | tb := sess.activeTab() | |
| 208 | + | tb.mu.Lock() | |
| 209 | + | require.Equal(t, floatingHidden, tb.floating.state) | |
| 210 | + | require.Same(t, floating, tb.floating.pane) | |
| 211 | + | generation := tb.floating.generation | |
| 212 | + | tb.mu.Unlock() | |
| 213 | + | require.Same(t, floatingPTY, floating.pty) | |
| 214 | + | select { | |
| 215 | + | case <-floating.ctx.Done(): | |
| 216 | + | t.Fatal("hiding retained popup cancelled its context") | |
| 217 | + | default: | |
| 218 | + | } | |
| 219 | + | ||
| 220 | + | // A third show uses the installed pane; it must not launch or emit a second | |
| 221 | + | // resize/start frame. | |
| 222 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |
| 223 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |
| 224 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |
| 225 | + | tb.mu.Lock() | |
| 226 | + | require.Equal(t, floatingVisible, tb.floating.state) | |
| 227 | + | require.Same(t, floating, tb.floating.pane) | |
| 228 | + | require.Equal(t, generation, tb.floating.generation) | |
| 229 | + | tb.mu.Unlock() | |
| 230 | + | select { | |
| 231 | + | case extra := <-sends: | |
| 232 | + | t.Fatalf("retained show emitted duplicate output: %#v", extra) | |
| 233 | + | default: | |
| 234 | + | } | |
| 235 | + | } | |
| 236 | + | ||
| 172 | 237 | func requireFloatingInitialized(t *testing.T, tb *tab) { | |
| 173 | 238 | t.Helper() | |
| 174 | 239 | tb.mu.Lock() | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "encoding/base64" | 5 | "encoding/base64" | ||
| 5 | "fmt" | 6 | "fmt" | ||
| 6 | "strings" | 7 | "strings" | ||
| @@ -169,6 +170,70 @@ func TestAltCForwardsToPTY(t *testing.T) { | |||||
| 169 | releasePTY() | 170 | releasePTY() | ||
| 170 | } | 171 | } | ||
| 171 | 172 | ||||
| 173 | + | func TestAltFToggleRetainedFloatingPaneRepaintsImmediately(t *testing.T) { | |||
| 174 | + | normal, releaseNormal := newBlockingPTY(t) | |||
| 175 | + | floatingPTY, releaseFloating := newBlockingPTY(t) | |||
| 176 | + | defer releaseNormal() | |||
| 177 | + | defer releaseFloating() | |||
| 178 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, normal) | |||
| 179 | + | client := vt.NewScreen(80, 25) | |||
| 180 | + | floatingCtx, cancelFloating := context.WithCancel(sess.ctx) | |||
| 181 | + | defer cancelFloating() | |||
| 182 | + | floating := newPane("floating", floatingPTY, domain.Size{Cols: 20, Rows: 5}) | |||
| 183 | + | floating.ctx = floatingCtx | |||
| 184 | + | floating.screen.Write([]byte("popup-content")) | |||
| 185 | + | installTestFloating(sess.activeTab(), floating, false) | |||
| 186 | + | ||||
| 187 | + | // Establish the client shadow while the retained popup is hidden. | |||
| 188 | + | d.paint(sess, ac, true) | |||
| 189 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |||
| 190 | + | ||||
| 191 | + | // Route the real Alt+F binding. Showing a retained pane must paint once. | |||
| 192 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |||
| 193 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |||
| 194 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |||
| 195 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "┌") | |||
| 196 | + | select { | |||
| 197 | + | case extra := <-sends: | |||
| 198 | + | t.Fatalf("show emitted duplicate output: %#v", extra) | |||
| 199 | + | default: | |||
| 200 | + | } | |||
| 201 | + | ||||
| 202 | + | // The second real key must repaint immediately, rather than waiting for | |||
| 203 | + | // unrelated output, while retaining the existing PTY and context. | |||
| 204 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |||
| 205 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |||
| 206 | + | require.NotContains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |||
| 207 | + | tb := sess.activeTab() | |||
| 208 | + | tb.mu.Lock() | |||
| 209 | + | require.Equal(t, floatingHidden, tb.floating.state) | |||
| 210 | + | require.Same(t, floating, tb.floating.pane) | |||
| 211 | + | generation := tb.floating.generation | |||
| 212 | + | tb.mu.Unlock() | |||
| 213 | + | require.Same(t, floatingPTY, floating.pty) | |||
| 214 | + | select { | |||
| 215 | + | case <-floating.ctx.Done(): | |||
| 216 | + | t.Fatal("hiding retained popup cancelled its context") | |||
| 217 | + | default: | |||
| 218 | + | } | |||
| 219 | + | ||||
| 220 | + | // A third show uses the installed pane; it must not launch or emit a second | |||
| 221 | + | // resize/start frame. | |||
| 222 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |||
| 223 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |||
| 224 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |||
| 225 | + | tb.mu.Lock() | |||
| 226 | + | require.Equal(t, floatingVisible, tb.floating.state) | |||
| 227 | + | require.Same(t, floating, tb.floating.pane) | |||
| 228 | + | require.Equal(t, generation, tb.floating.generation) | |||
| 229 | + | tb.mu.Unlock() | |||
| 230 | + | select { | |||
| 231 | + | case extra := <-sends: | |||
| 232 | + | t.Fatalf("retained show emitted duplicate output: %#v", extra) | |||
| 233 | + | default: | |||
| 234 | + | } | |||
| 235 | + | } | |||
| 236 | + | ||||
| 172 | func requireFloatingInitialized(t *testing.T, tb *tab) { | 237 | func requireFloatingInitialized(t *testing.T, tb *tab) { | ||
| 173 | t.Helper() | 238 | t.Helper() | ||
| 174 | tb.mu.Lock() | 239 | tb.mu.Lock() | ||