fix(daemon): reject stale floating layout publication
- commit
- c9575d3fa25b5ebec0aed598284be3e79c9c0de0
- author
- brice · · 2026-07-25 04:06 UTC
- committer
- brice · · 2026-07-25 04:08 UTC
- parent
- de9b334
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +75 -12
M internal/usecase/daemon/floating_render_test.go +46 -0
| @@ -636,6 +636,52 @@ func TestFailedFloatingResizeKeepsCommittedRenderAndInputGeometry(t *testing.T) | |||
| 636 | 636 | } | |
| 637 | 637 | } | |
| 638 | 638 | ||
| 639 | + | func TestFloatingLayoutRejectsStaleResizeBeforePublishing(t *testing.T) { | |
| 640 | + | cfg := domain.FloatingConfig{Width: 50, Height: 50} | |
| 641 | + | oldSize := domain.Size{Cols: 80, Rows: 24} | |
| 642 | + | newSize := domain.Size{Cols: 100, Rows: 40} | |
| 643 | + | previousGeometry := calculateContentFloatingGeometry(domain.Size{Cols: 60, Rows: 20}, cfg) | |
| 644 | + | oldGeometry := calculateContentFloatingGeometry(oldSize, cfg) | |
| 645 | + | newGeometry := calculateContentFloatingGeometry(newSize, cfg) | |
| 646 | + | pty := &resizePTY{entered: make(chan struct{}), release: make(chan struct{})} | |
| 647 | + | p := newPane("floating", pty, rectSize(previousGeometry.Inner)) | |
| 648 | + | p.rect = previousGeometry.Inner | |
| 649 | + | p.popupGeometry = previousGeometry | |
| 650 | + | tb := newTab(nil, oldSize) | |
| 651 | + | installTestFloating(tb, p, true) | |
| 652 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 653 | + | d.ApplyConfig(domain.Config{Floating: cfg}) | |
| 654 | + | ||
| 655 | + | done := make(chan bool, 1) | |
| 656 | + | go func() { | |
| 657 | + | _, ok := d.applyVisibleFloatingLayout(&session{tabs: []*tab{tb}}, tb, nil) | |
| 658 | + | done <- ok | |
| 659 | + | }() | |
| 660 | + | <-pty.entered | |
| 661 | + | tb.mu.Lock() | |
| 662 | + | tb.size = newSize | |
| 663 | + | tb.bumpLayoutGenerationLocked() | |
| 664 | + | tb.mu.Unlock() | |
| 665 | + | close(pty.release) | |
| 666 | + | require.True(t, <-done) | |
| 667 | + | ||
| 668 | + | p.mu.Lock() | |
| 669 | + | require.Equal(t, previousGeometry.Inner, p.rect, "a stale floating apply must not publish its rectangle") | |
| 670 | + | require.Equal(t, previousGeometry, p.popupGeometry, "a stale floating apply must not publish its geometry") | |
| 671 | + | require.Equal(t, previousGeometry.Inner.Width, p.screen.Frame.Width, "a stale floating apply must not resize the screen") | |
| 672 | + | require.Equal(t, previousGeometry.Inner.Height, p.screen.Frame.Height, "a stale floating apply must not resize the screen") | |
| 673 | + | require.False(t, p.resizeApplying, "a stale floating apply must release its parser gate") | |
| 674 | + | p.mu.Unlock() | |
| 675 | + | ||
| 676 | + | _, ok := d.applyVisibleFloatingLayout(&session{tabs: []*tab{tb}}, tb, nil) | |
| 677 | + | require.True(t, ok) | |
| 678 | + | require.Equal(t, []domain.Size{rectSize(oldGeometry.Inner), rectSize(newGeometry.Inner)}, pty.sizes()) | |
| 679 | + | require.Equal(t, newGeometry.Inner, p.rect) | |
| 680 | + | require.Equal(t, newGeometry, p.popupGeometry) | |
| 681 | + | require.Equal(t, newGeometry.Inner.Width, p.screen.Frame.Width) | |
| 682 | + | require.Equal(t, newGeometry.Inner.Height, p.screen.Frame.Height) | |
| 683 | + | } | |
| 684 | + | ||
| 639 | 685 | func TestResizeFloatingPaneFailureAndSerialization(t *testing.T) { | |
| 640 | 686 | t.Run("failure preserves state", func(t *testing.T) { | |
| 641 | 687 | pty := &resizePTY{err: errors.New("nope")} | |
| @@ -636,6 +636,52 @@ func TestFailedFloatingResizeKeepsCommittedRenderAndInputGeometry(t *testing.T) | |||||
| 636 | } | 636 | } | ||
| 637 | } | 637 | } | ||
| 638 | 638 | ||||
| 639 | + | func TestFloatingLayoutRejectsStaleResizeBeforePublishing(t *testing.T) { | |||
| 640 | + | cfg := domain.FloatingConfig{Width: 50, Height: 50} | |||
| 641 | + | oldSize := domain.Size{Cols: 80, Rows: 24} | |||
| 642 | + | newSize := domain.Size{Cols: 100, Rows: 40} | |||
| 643 | + | previousGeometry := calculateContentFloatingGeometry(domain.Size{Cols: 60, Rows: 20}, cfg) | |||
| 644 | + | oldGeometry := calculateContentFloatingGeometry(oldSize, cfg) | |||
| 645 | + | newGeometry := calculateContentFloatingGeometry(newSize, cfg) | |||
| 646 | + | pty := &resizePTY{entered: make(chan struct{}), release: make(chan struct{})} | |||
| 647 | + | p := newPane("floating", pty, rectSize(previousGeometry.Inner)) | |||
| 648 | + | p.rect = previousGeometry.Inner | |||
| 649 | + | p.popupGeometry = previousGeometry | |||
| 650 | + | tb := newTab(nil, oldSize) | |||
| 651 | + | installTestFloating(tb, p, true) | |||
| 652 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 653 | + | d.ApplyConfig(domain.Config{Floating: cfg}) | |||
| 654 | + | ||||
| 655 | + | done := make(chan bool, 1) | |||
| 656 | + | go func() { | |||
| 657 | + | _, ok := d.applyVisibleFloatingLayout(&session{tabs: []*tab{tb}}, tb, nil) | |||
| 658 | + | done <- ok | |||
| 659 | + | }() | |||
| 660 | + | <-pty.entered | |||
| 661 | + | tb.mu.Lock() | |||
| 662 | + | tb.size = newSize | |||
| 663 | + | tb.bumpLayoutGenerationLocked() | |||
| 664 | + | tb.mu.Unlock() | |||
| 665 | + | close(pty.release) | |||
| 666 | + | require.True(t, <-done) | |||
| 667 | + | ||||
| 668 | + | p.mu.Lock() | |||
| 669 | + | require.Equal(t, previousGeometry.Inner, p.rect, "a stale floating apply must not publish its rectangle") | |||
| 670 | + | require.Equal(t, previousGeometry, p.popupGeometry, "a stale floating apply must not publish its geometry") | |||
| 671 | + | require.Equal(t, previousGeometry.Inner.Width, p.screen.Frame.Width, "a stale floating apply must not resize the screen") | |||
| 672 | + | require.Equal(t, previousGeometry.Inner.Height, p.screen.Frame.Height, "a stale floating apply must not resize the screen") | |||
| 673 | + | require.False(t, p.resizeApplying, "a stale floating apply must release its parser gate") | |||
| 674 | + | p.mu.Unlock() | |||
| 675 | + | ||||
| 676 | + | _, ok := d.applyVisibleFloatingLayout(&session{tabs: []*tab{tb}}, tb, nil) | |||
| 677 | + | require.True(t, ok) | |||
| 678 | + | require.Equal(t, []domain.Size{rectSize(oldGeometry.Inner), rectSize(newGeometry.Inner)}, pty.sizes()) | |||
| 679 | + | require.Equal(t, newGeometry.Inner, p.rect) | |||
| 680 | + | require.Equal(t, newGeometry, p.popupGeometry) | |||
| 681 | + | require.Equal(t, newGeometry.Inner.Width, p.screen.Frame.Width) | |||
| 682 | + | require.Equal(t, newGeometry.Inner.Height, p.screen.Frame.Height) | |||
| 683 | + | } | |||
| 684 | + | ||||
| 639 | func TestResizeFloatingPaneFailureAndSerialization(t *testing.T) { | 685 | func TestResizeFloatingPaneFailureAndSerialization(t *testing.T) { | ||
| 640 | t.Run("failure preserves state", func(t *testing.T) { | 686 | t.Run("failure preserves state", func(t *testing.T) { | ||
| 641 | pty := &resizePTY{err: errors.New("nope")} | 687 | pty := &resizePTY{err: errors.New("nope")} | ||
M internal/usecase/daemon/transactional_resize.go +29 -12
| @@ -417,41 +417,58 @@ func (d *Daemon) applyVisibleFloatingLayout(sess *session, tb *tab, current func | |||
| 417 | 417 | } | |
| 418 | 418 | p := tb.floating.pane | |
| 419 | 419 | generation := tb.floating.generation | |
| 420 | - | geometry := calculateContentFloatingGeometry(tb.size, d.currentFloatingConfig()) | |
| 420 | + | size := tb.size | |
| 421 | + | geometry := calculateContentFloatingGeometry(size, d.currentFloatingConfig()) | |
| 421 | 422 | tb.mu.Unlock() | |
| 422 | 423 | if !geometry.valid() { | |
| 423 | 424 | return nil, true | |
| 424 | 425 | } | |
| 425 | 426 | ||
| 426 | - | plan := resizePlan{members: []resizeMember{{ | |
| 427 | + | // Unlike applyResize, this keeps successful PTY resizes gated until the | |
| 428 | + | // floating slot and tab size are revalidated. A newer client resize may | |
| 429 | + | // otherwise publish this obsolete popup geometry after its PTY call returns. | |
| 430 | + | plan := preparedTabLayout{members: []resizeMember{{ | |
| 427 | 431 | session: sess, tab: tb, pane: p, rect: geometry.Inner, floating: geometry, isFloating: true, | |
| 428 | 432 | }}} | |
| 429 | - | if current == nil { | |
| 430 | - | if !d.applyResize(&plan) { | |
| 431 | - | return nil, false | |
| 432 | - | } | |
| 433 | - | } else if !d.applyResize(&plan, current) { | |
| 434 | - | return nil, false | |
| 435 | - | } | |
| 433 | + | d.applyPreparedTabMembers(&plan) | |
| 436 | 434 | ||
| 437 | 435 | // The PTY may have accepted an intermediate size, but a hidden, replaced, | |
| 438 | - | // or relaunched slot must never receive this attempt's geometry. | |
| 436 | + | // relaunched, or resized slot must never receive this attempt's geometry. | |
| 439 | 437 | tb.mu.Lock() | |
| 440 | - | currentSlot := tb.floating.state == floatingVisible && tb.floating.generation == generation && tb.floating.pane == p | |
| 438 | + | currentSlot := tb.floating.state == floatingVisible && tb.floating.generation == generation && tb.floating.pane == p && tb.size == size | |
| 439 | + | if current != nil && !current() { | |
| 440 | + | currentSlot = false | |
| 441 | + | } | |
| 441 | 442 | if currentSlot { | |
| 442 | 443 | p.mu.Lock() | |
| 443 | 444 | p.rect = geometry.Inner | |
| 444 | 445 | p.popupGeometry = geometry | |
| 445 | - | if plan.members[0].ok && !plan.members[0].screenResized { | |
| 446 | + | if plan.members[0].ok { | |
| 447 | + | p.resizeRetry = false | |
| 446 | 448 | p.screen.Resize(geometry.Inner.Width, geometry.Inner.Height) | |
| 447 | 449 | } | |
| 448 | 450 | p.mu.Unlock() | |
| 449 | 451 | } | |
| 450 | 452 | tb.mu.Unlock() | |
| 451 | 453 | if !currentSlot { | |
| 454 | + | // This attempt has no tiled transaction loop to retain the gate for, so | |
| 455 | + | // discard its buffered bytes at the old screen size before returning. | |
| 456 | + | for i := range plan.members { | |
| 457 | + | member := &plan.members[i] | |
| 458 | + | if !member.screenResized { | |
| 459 | + | continue | |
| 460 | + | } | |
| 461 | + | member.pane.resizeMu.Lock() | |
| 462 | + | d.replayResizePending(member.session, member.tab, member.pane, false, member.rect) | |
| 463 | + | member.pane.resizeMu.Unlock() | |
| 464 | + | } | |
| 465 | + | if current != nil && !current() { | |
| 466 | + | return nil, false | |
| 467 | + | } | |
| 452 | 468 | return nil, true | |
| 453 | 469 | } | |
| 454 | 470 | if plan.members[0].ok { | |
| 471 | + | d.finishPreparedTabMembers(&plan, true) | |
| 455 | 472 | return nil, true | |
| 456 | 473 | } | |
| 457 | 474 | return plan.members, true | |
| @@ -417,41 +417,58 @@ func (d *Daemon) applyVisibleFloatingLayout(sess *session, tb *tab, current func | |||||
| 417 | } | 417 | } | ||
| 418 | p := tb.floating.pane | 418 | p := tb.floating.pane | ||
| 419 | generation := tb.floating.generation | 419 | generation := tb.floating.generation | ||
| 420 | - | geometry := calculateContentFloatingGeometry(tb.size, d.currentFloatingConfig()) | 420 | + | size := tb.size |
| 421 | + | geometry := calculateContentFloatingGeometry(size, d.currentFloatingConfig()) | |||
| 421 | tb.mu.Unlock() | 422 | tb.mu.Unlock() | ||
| 422 | if !geometry.valid() { | 423 | if !geometry.valid() { | ||
| 423 | return nil, true | 424 | return nil, true | ||
| 424 | } | 425 | } | ||
| 425 | 426 | ||||
| 426 | - | plan := resizePlan{members: []resizeMember{{ | 427 | + | // Unlike applyResize, this keeps successful PTY resizes gated until the |
| 428 | + | // floating slot and tab size are revalidated. A newer client resize may | |||
| 429 | + | // otherwise publish this obsolete popup geometry after its PTY call returns. | |||
| 430 | + | plan := preparedTabLayout{members: []resizeMember{{ | |||
| 427 | session: sess, tab: tb, pane: p, rect: geometry.Inner, floating: geometry, isFloating: true, | 431 | session: sess, tab: tb, pane: p, rect: geometry.Inner, floating: geometry, isFloating: true, | ||
| 428 | }}} | 432 | }}} | ||
| 429 | - | if current == nil { | 433 | + | d.applyPreparedTabMembers(&plan) |
| 430 | - | if !d.applyResize(&plan) { | |||
| 431 | - | return nil, false | |||
| 432 | - | } | |||
| 433 | - | } else if !d.applyResize(&plan, current) { | |||
| 434 | - | return nil, false | |||
| 435 | - | } | |||
| 436 | 434 | ||||
| 437 | // The PTY may have accepted an intermediate size, but a hidden, replaced, | 435 | // The PTY may have accepted an intermediate size, but a hidden, replaced, | ||
| 438 | - | // or relaunched slot must never receive this attempt's geometry. | 436 | + | // relaunched, or resized slot must never receive this attempt's geometry. |
| 439 | tb.mu.Lock() | 437 | tb.mu.Lock() | ||
| 440 | - | currentSlot := tb.floating.state == floatingVisible && tb.floating.generation == generation && tb.floating.pane == p | 438 | + | currentSlot := tb.floating.state == floatingVisible && tb.floating.generation == generation && tb.floating.pane == p && tb.size == size |
| 439 | + | if current != nil && !current() { | |||
| 440 | + | currentSlot = false | |||
| 441 | + | } | |||
| 441 | if currentSlot { | 442 | if currentSlot { | ||
| 442 | p.mu.Lock() | 443 | p.mu.Lock() | ||
| 443 | p.rect = geometry.Inner | 444 | p.rect = geometry.Inner | ||
| 444 | p.popupGeometry = geometry | 445 | p.popupGeometry = geometry | ||
| 445 | - | if plan.members[0].ok && !plan.members[0].screenResized { | 446 | + | if plan.members[0].ok { |
| 447 | + | p.resizeRetry = false | |||
| 446 | p.screen.Resize(geometry.Inner.Width, geometry.Inner.Height) | 448 | p.screen.Resize(geometry.Inner.Width, geometry.Inner.Height) | ||
| 447 | } | 449 | } | ||
| 448 | p.mu.Unlock() | 450 | p.mu.Unlock() | ||
| 449 | } | 451 | } | ||
| 450 | tb.mu.Unlock() | 452 | tb.mu.Unlock() | ||
| 451 | if !currentSlot { | 453 | if !currentSlot { | ||
| 454 | + | // This attempt has no tiled transaction loop to retain the gate for, so | |||
| 455 | + | // discard its buffered bytes at the old screen size before returning. | |||
| 456 | + | for i := range plan.members { | |||
| 457 | + | member := &plan.members[i] | |||
| 458 | + | if !member.screenResized { | |||
| 459 | + | continue | |||
| 460 | + | } | |||
| 461 | + | member.pane.resizeMu.Lock() | |||
| 462 | + | d.replayResizePending(member.session, member.tab, member.pane, false, member.rect) | |||
| 463 | + | member.pane.resizeMu.Unlock() | |||
| 464 | + | } | |||
| 465 | + | if current != nil && !current() { | |||
| 466 | + | return nil, false | |||
| 467 | + | } | |||
| 452 | return nil, true | 468 | return nil, true | ||
| 453 | } | 469 | } | ||
| 454 | if plan.members[0].ok { | 470 | if plan.members[0].ok { | ||
| 471 | + | d.finishPreparedTabMembers(&plan, true) | |||
| 455 | return nil, true | 472 | return nil, true | ||
| 456 | } | 473 | } | ||
| 457 | return plan.members, true | 474 | return plan.members, true | ||