fix(floating): preserve committed geometry on resize failure
- commit
- d1d65ad5073255d2ec2ce8da3a0cd3d6e5c73a25
- author
- brice · · 2026-07-27 16:11 UTC
- committer
- brice · · 2026-07-27 17:06 UTC
- parent
- e15edb0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +86 -8
M internal/usecase/daemon/floating_render_test.go +3 -3
| @@ -713,13 +713,13 @@ func TestToggleFloatingResizesHiddenPaneOnShowAndRetriesFailure(t *testing.T) { | |||
| 713 | 713 | require.Empty(t, pty.sizes()) | |
| 714 | 714 | ||
| 715 | 715 | // Showing attempts the current size before paint. A failed resize retains | |
| 716 | - | // the old screen, but commits the new popup rect so capture clips/pads it. | |
| 716 | + | // the complete previously committed render and input geometry. | |
| 717 | 717 | require.NoError(t, d.toggleFloating(sess, nil)) | |
| 718 | 718 | require.Equal(t, []domain.Size{rectSize(current)}, pty.sizes()) | |
| 719 | - | require.Equal(t, current, floating.rect) | |
| 719 | + | require.Equal(t, initial, floating.rect) | |
| 720 | 720 | require.Equal(t, initial.Width, floating.screen.Frame.Width) | |
| 721 | 721 | require.Equal(t, initial.Height, floating.screen.Frame.Height) | |
| 722 | - | require.Equal(t, currentGeometry, floating.popupGeometry) | |
| 722 | + | require.Equal(t, initialGeometry, floating.popupGeometry) | |
| 723 | 723 | ||
| 724 | 724 | require.NoError(t, d.toggleFloating(sess, nil)) // hide | |
| 725 | 725 | require.NoError(t, d.toggleFloating(sess, nil)) // retry show | |
| @@ -713,13 +713,13 @@ func TestToggleFloatingResizesHiddenPaneOnShowAndRetriesFailure(t *testing.T) { | |||||
| 713 | require.Empty(t, pty.sizes()) | 713 | require.Empty(t, pty.sizes()) | ||
| 714 | 714 | ||||
| 715 | // Showing attempts the current size before paint. A failed resize retains | 715 | // Showing attempts the current size before paint. A failed resize retains | ||
| 716 | - | // the old screen, but commits the new popup rect so capture clips/pads it. | 716 | + | // the complete previously committed render and input geometry. |
| 717 | require.NoError(t, d.toggleFloating(sess, nil)) | 717 | require.NoError(t, d.toggleFloating(sess, nil)) | ||
| 718 | require.Equal(t, []domain.Size{rectSize(current)}, pty.sizes()) | 718 | require.Equal(t, []domain.Size{rectSize(current)}, pty.sizes()) | ||
| 719 | - | require.Equal(t, current, floating.rect) | 719 | + | require.Equal(t, initial, floating.rect) |
| 720 | require.Equal(t, initial.Width, floating.screen.Frame.Width) | 720 | require.Equal(t, initial.Width, floating.screen.Frame.Width) | ||
| 721 | require.Equal(t, initial.Height, floating.screen.Frame.Height) | 721 | require.Equal(t, initial.Height, floating.screen.Frame.Height) | ||
| 722 | - | require.Equal(t, currentGeometry, floating.popupGeometry) | 722 | + | require.Equal(t, initialGeometry, floating.popupGeometry) |
| 723 | 723 | ||||
| 724 | require.NoError(t, d.toggleFloating(sess, nil)) // hide | 724 | require.NoError(t, d.toggleFloating(sess, nil)) // hide | ||
| 725 | require.NoError(t, d.toggleFloating(sess, nil)) // retry show | 725 | require.NoError(t, d.toggleFloating(sess, nil)) // retry show | ||
M internal/usecase/daemon/transactional_resize.go +3 -5
| @@ -385,14 +385,12 @@ func (d *Daemon) applyVisibleFloatingLayoutForMember(sess *session, tb *tab, cur | |||
| 385 | 385 | if current != nil && !current() { | |
| 386 | 386 | currentSlot = false | |
| 387 | 387 | } | |
| 388 | - | if currentSlot { | |
| 388 | + | if currentSlot && plan.members[0].ok { | |
| 389 | 389 | p.mu.Lock() | |
| 390 | 390 | p.rect = plan.members[0].rect | |
| 391 | 391 | p.popupGeometry = geometry | |
| 392 | - | if plan.members[0].ok { | |
| 393 | - | p.resizeRetry = false | |
| 394 | - | p.screen.Resize(plan.members[0].rect.Width, plan.members[0].rect.Height) | |
| 395 | - | } | |
| 392 | + | p.resizeRetry = false | |
| 393 | + | p.screen.Resize(plan.members[0].rect.Width, plan.members[0].rect.Height) | |
| 396 | 394 | p.mu.Unlock() | |
| 397 | 395 | } | |
| 398 | 396 | tb.mu.Unlock() | |
| @@ -385,14 +385,12 @@ func (d *Daemon) applyVisibleFloatingLayoutForMember(sess *session, tb *tab, cur | |||||
| 385 | if current != nil && !current() { | 385 | if current != nil && !current() { | ||
| 386 | currentSlot = false | 386 | currentSlot = false | ||
| 387 | } | 387 | } | ||
| 388 | - | if currentSlot { | 388 | + | if currentSlot && plan.members[0].ok { |
| 389 | p.mu.Lock() | 389 | p.mu.Lock() | ||
| 390 | p.rect = plan.members[0].rect | 390 | p.rect = plan.members[0].rect | ||
| 391 | p.popupGeometry = geometry | 391 | p.popupGeometry = geometry | ||
| 392 | - | if plan.members[0].ok { | 392 | + | p.resizeRetry = false |
| 393 | - | p.resizeRetry = false | 393 | + | p.screen.Resize(plan.members[0].rect.Width, plan.members[0].rect.Height) |
| 394 | - | p.screen.Resize(plan.members[0].rect.Width, plan.members[0].rect.Height) | |||
| 395 | - | } | |||
| 396 | p.mu.Unlock() | 394 | p.mu.Unlock() | ||
| 397 | } | 395 | } | ||
| 398 | tb.mu.Unlock() | 396 | tb.mu.Unlock() | ||
M internal/usecase/daemon/transactional_resize_test.go +80 -0
| @@ -12,6 +12,7 @@ import ( | |||
| 12 | 12 | "github.com/bnema/vev/internal/domain" | |
| 13 | 13 | "github.com/bnema/vev/internal/ports" | |
| 14 | 14 | "github.com/bnema/vev/internal/usecase/layout" | |
| 15 | + | "github.com/bnema/vev/internal/usecase/ui" | |
| 15 | 16 | "github.com/bnema/vev/internal/usecase/visualsearch" | |
| 16 | 17 | ) | |
| 17 | 18 | ||
| @@ -483,6 +484,85 @@ func TestHeadlessResizeRetriesInvalidatedPlan(t *testing.T) { | |||
| 483 | 484 | tb.mu.Unlock() | |
| 484 | 485 | } | |
| 485 | 486 | ||
| 487 | + | func TestApplyVisibleFloatingLayoutPreservesCommittedGeometryOnPTYResizeFailure(t *testing.T) { | |
| 488 | + | cfg := domain.FloatingConfig{Width: 50, Height: 50} | |
| 489 | + | committed := calculateContentFloatingGeometry(domain.Size{Cols: 80, Rows: 24}, cfg) | |
| 490 | + | requestedSize := domain.Size{Cols: 100, Rows: 40} | |
| 491 | + | requested := calculateContentFloatingGeometry(requestedSize, cfg) | |
| 492 | + | resizeErr := errors.New("floating resize failed") | |
| 493 | + | pty := &transactionalResizePTY{errs: []error{resizeErr}} | |
| 494 | + | popup := newPane("floating", pty, rectSize(committed.Inner)) | |
| 495 | + | popup.rect = committed.Inner | |
| 496 | + | popup.popupGeometry = committed | |
| 497 | + | tb := newTab(nil, requestedSize) | |
| 498 | + | installTestFloating(tb, popup, true) | |
| 499 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 500 | + | d.ApplyConfig(domain.Config{Floating: cfg}) | |
| 501 | + | var tabUnlocked, paneUnlocked bool | |
| 502 | + | pty.onResize = func() { | |
| 503 | + | tabUnlocked = tb.mu.TryLock() | |
| 504 | + | if tabUnlocked { | |
| 505 | + | tb.mu.Unlock() | |
| 506 | + | } | |
| 507 | + | paneUnlocked = popup.mu.TryLock() | |
| 508 | + | if paneUnlocked { | |
| 509 | + | popup.mu.Unlock() | |
| 510 | + | } | |
| 511 | + | } | |
| 512 | + | ||
| 513 | + | failed, ok := d.applyVisibleFloatingLayout(&session{tabs: []*tab{tb}}, tb, nil) | |
| 514 | + | ||
| 515 | + | require.True(t, ok) | |
| 516 | + | require.Len(t, failed, 1) | |
| 517 | + | require.ErrorIs(t, failed[0].err, resizeErr) | |
| 518 | + | require.Equal(t, []domain.Size{rectSize(requested.Inner)}, pty.requested()) | |
| 519 | + | require.True(t, tabUnlocked, "PTY.Resize must run outside tab.mu") | |
| 520 | + | require.True(t, paneUnlocked, "PTY.Resize must run outside pane.mu") | |
| 521 | + | popup.mu.Lock() | |
| 522 | + | require.Equal(t, committed.Inner, popup.rect, "failed PTY resize published a speculative rectangle") | |
| 523 | + | require.Equal(t, committed, popup.popupGeometry, "failed PTY resize published speculative popup geometry") | |
| 524 | + | require.Equal(t, rectSize(committed.Inner), domain.Size{Cols: popup.screen.Frame.Width, Rows: popup.screen.Frame.Height}) | |
| 525 | + | require.True(t, popup.resizeRetry, "failed PTY resize must remain retryable") | |
| 526 | + | require.False(t, popup.resizeApplying, "failed PTY resize must reopen the parser gate") | |
| 527 | + | popup.mu.Unlock() | |
| 528 | + | tb.mu.Lock() | |
| 529 | + | _, inputGeometry, visible := tb.visibleFloatingSnapshotLocked(cfg) | |
| 530 | + | tb.mu.Unlock() | |
| 531 | + | require.True(t, visible) | |
| 532 | + | require.Equal(t, committed, inputGeometry, "input mapping must retain committed geometry") | |
| 533 | + | } | |
| 534 | + | ||
| 535 | + | func TestApplyVisibleFloatingLayoutPublishesSameSizeGeometryWithoutPTYResize(t *testing.T) { | |
| 536 | + | cfg := domain.FloatingConfig{Width: 50, Height: 50} | |
| 537 | + | size := domain.Size{Cols: 79, Rows: 20} | |
| 538 | + | requested := calculateContentFloatingGeometry(size, cfg) | |
| 539 | + | require.Equal(t, ui.PresentationDrawer, requested.Mode) | |
| 540 | + | committed := requested | |
| 541 | + | committed.Mode = ui.PresentationFloating | |
| 542 | + | committed.Bounds.X++ | |
| 543 | + | committed.Inner.X++ | |
| 544 | + | pty := &transactionalResizePTY{} | |
| 545 | + | popup := newPane("floating", pty, rectSize(committed.Inner)) | |
| 546 | + | popup.rect = committed.Inner | |
| 547 | + | popup.popupGeometry = committed | |
| 548 | + | tb := newTab(nil, size) | |
| 549 | + | installTestFloating(tb, popup, true) | |
| 550 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 551 | + | d.ApplyConfig(domain.Config{Floating: cfg}) | |
| 552 | + | ||
| 553 | + | failed, ok := d.applyVisibleFloatingLayout(&session{tabs: []*tab{tb}}, tb, nil) | |
| 554 | + | ||
| 555 | + | require.True(t, ok) | |
| 556 | + | require.Empty(t, failed) | |
| 557 | + | require.Empty(t, pty.requested(), "same-size geometry publication must not resize the PTY") | |
| 558 | + | popup.mu.Lock() | |
| 559 | + | require.Equal(t, requested.Inner, popup.rect) | |
| 560 | + | require.Equal(t, requested, popup.popupGeometry) | |
| 561 | + | require.Equal(t, rectSize(requested.Inner), domain.Size{Cols: popup.screen.Frame.Width, Rows: popup.screen.Frame.Height}) | |
| 562 | + | require.False(t, popup.resizeRetry) | |
| 563 | + | popup.mu.Unlock() | |
| 564 | + | } | |
| 565 | + | ||
| 486 | 566 | func TestTransactionalResizeRetriesAcceptedFloatingSlotByIdentity(t *testing.T) { | |
| 487 | 567 | popupPTY := &transactionalResizePTY{errs: []error{errors.New("first popup resize fails")}} | |
| 488 | 568 | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | |
| @@ -12,6 +12,7 @@ import ( | |||||
| 12 | "github.com/bnema/vev/internal/domain" | 12 | "github.com/bnema/vev/internal/domain" | ||
| 13 | "github.com/bnema/vev/internal/ports" | 13 | "github.com/bnema/vev/internal/ports" | ||
| 14 | "github.com/bnema/vev/internal/usecase/layout" | 14 | "github.com/bnema/vev/internal/usecase/layout" | ||
| 15 | + | "github.com/bnema/vev/internal/usecase/ui" | |||
| 15 | "github.com/bnema/vev/internal/usecase/visualsearch" | 16 | "github.com/bnema/vev/internal/usecase/visualsearch" | ||
| 16 | ) | 17 | ) | ||
| 17 | 18 | ||||
| @@ -483,6 +484,85 @@ func TestHeadlessResizeRetriesInvalidatedPlan(t *testing.T) { | |||||
| 483 | tb.mu.Unlock() | 484 | tb.mu.Unlock() | ||
| 484 | } | 485 | } | ||
| 485 | 486 | ||||
| 487 | + | func TestApplyVisibleFloatingLayoutPreservesCommittedGeometryOnPTYResizeFailure(t *testing.T) { | |||
| 488 | + | cfg := domain.FloatingConfig{Width: 50, Height: 50} | |||
| 489 | + | committed := calculateContentFloatingGeometry(domain.Size{Cols: 80, Rows: 24}, cfg) | |||
| 490 | + | requestedSize := domain.Size{Cols: 100, Rows: 40} | |||
| 491 | + | requested := calculateContentFloatingGeometry(requestedSize, cfg) | |||
| 492 | + | resizeErr := errors.New("floating resize failed") | |||
| 493 | + | pty := &transactionalResizePTY{errs: []error{resizeErr}} | |||
| 494 | + | popup := newPane("floating", pty, rectSize(committed.Inner)) | |||
| 495 | + | popup.rect = committed.Inner | |||
| 496 | + | popup.popupGeometry = committed | |||
| 497 | + | tb := newTab(nil, requestedSize) | |||
| 498 | + | installTestFloating(tb, popup, true) | |||
| 499 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 500 | + | d.ApplyConfig(domain.Config{Floating: cfg}) | |||
| 501 | + | var tabUnlocked, paneUnlocked bool | |||
| 502 | + | pty.onResize = func() { | |||
| 503 | + | tabUnlocked = tb.mu.TryLock() | |||
| 504 | + | if tabUnlocked { | |||
| 505 | + | tb.mu.Unlock() | |||
| 506 | + | } | |||
| 507 | + | paneUnlocked = popup.mu.TryLock() | |||
| 508 | + | if paneUnlocked { | |||
| 509 | + | popup.mu.Unlock() | |||
| 510 | + | } | |||
| 511 | + | } | |||
| 512 | + | ||||
| 513 | + | failed, ok := d.applyVisibleFloatingLayout(&session{tabs: []*tab{tb}}, tb, nil) | |||
| 514 | + | ||||
| 515 | + | require.True(t, ok) | |||
| 516 | + | require.Len(t, failed, 1) | |||
| 517 | + | require.ErrorIs(t, failed[0].err, resizeErr) | |||
| 518 | + | require.Equal(t, []domain.Size{rectSize(requested.Inner)}, pty.requested()) | |||
| 519 | + | require.True(t, tabUnlocked, "PTY.Resize must run outside tab.mu") | |||
| 520 | + | require.True(t, paneUnlocked, "PTY.Resize must run outside pane.mu") | |||
| 521 | + | popup.mu.Lock() | |||
| 522 | + | require.Equal(t, committed.Inner, popup.rect, "failed PTY resize published a speculative rectangle") | |||
| 523 | + | require.Equal(t, committed, popup.popupGeometry, "failed PTY resize published speculative popup geometry") | |||
| 524 | + | require.Equal(t, rectSize(committed.Inner), domain.Size{Cols: popup.screen.Frame.Width, Rows: popup.screen.Frame.Height}) | |||
| 525 | + | require.True(t, popup.resizeRetry, "failed PTY resize must remain retryable") | |||
| 526 | + | require.False(t, popup.resizeApplying, "failed PTY resize must reopen the parser gate") | |||
| 527 | + | popup.mu.Unlock() | |||
| 528 | + | tb.mu.Lock() | |||
| 529 | + | _, inputGeometry, visible := tb.visibleFloatingSnapshotLocked(cfg) | |||
| 530 | + | tb.mu.Unlock() | |||
| 531 | + | require.True(t, visible) | |||
| 532 | + | require.Equal(t, committed, inputGeometry, "input mapping must retain committed geometry") | |||
| 533 | + | } | |||
| 534 | + | ||||
| 535 | + | func TestApplyVisibleFloatingLayoutPublishesSameSizeGeometryWithoutPTYResize(t *testing.T) { | |||
| 536 | + | cfg := domain.FloatingConfig{Width: 50, Height: 50} | |||
| 537 | + | size := domain.Size{Cols: 79, Rows: 20} | |||
| 538 | + | requested := calculateContentFloatingGeometry(size, cfg) | |||
| 539 | + | require.Equal(t, ui.PresentationDrawer, requested.Mode) | |||
| 540 | + | committed := requested | |||
| 541 | + | committed.Mode = ui.PresentationFloating | |||
| 542 | + | committed.Bounds.X++ | |||
| 543 | + | committed.Inner.X++ | |||
| 544 | + | pty := &transactionalResizePTY{} | |||
| 545 | + | popup := newPane("floating", pty, rectSize(committed.Inner)) | |||
| 546 | + | popup.rect = committed.Inner | |||
| 547 | + | popup.popupGeometry = committed | |||
| 548 | + | tb := newTab(nil, size) | |||
| 549 | + | installTestFloating(tb, popup, true) | |||
| 550 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 551 | + | d.ApplyConfig(domain.Config{Floating: cfg}) | |||
| 552 | + | ||||
| 553 | + | failed, ok := d.applyVisibleFloatingLayout(&session{tabs: []*tab{tb}}, tb, nil) | |||
| 554 | + | ||||
| 555 | + | require.True(t, ok) | |||
| 556 | + | require.Empty(t, failed) | |||
| 557 | + | require.Empty(t, pty.requested(), "same-size geometry publication must not resize the PTY") | |||
| 558 | + | popup.mu.Lock() | |||
| 559 | + | require.Equal(t, requested.Inner, popup.rect) | |||
| 560 | + | require.Equal(t, requested, popup.popupGeometry) | |||
| 561 | + | require.Equal(t, rectSize(requested.Inner), domain.Size{Cols: popup.screen.Frame.Width, Rows: popup.screen.Frame.Height}) | |||
| 562 | + | require.False(t, popup.resizeRetry) | |||
| 563 | + | popup.mu.Unlock() | |||
| 564 | + | } | |||
| 565 | + | ||||
| 486 | func TestTransactionalResizeRetriesAcceptedFloatingSlotByIdentity(t *testing.T) { | 566 | func TestTransactionalResizeRetriesAcceptedFloatingSlotByIdentity(t *testing.T) { | ||
| 487 | popupPTY := &transactionalResizePTY{errs: []error{errors.New("first popup resize fails")}} | 567 | popupPTY := &transactionalResizePTY{errs: []error{errors.New("first popup resize fails")}} | ||
| 488 | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | 568 | d, sess, ac, _ := newManualSessionWithPTYs(t, &transactionalResizePTY{}) | ||