fix(resize): guard canceled layout publication
- commit
- 3ead26da6fb37a09e6dd0b7592cf64f52d911f30
- author
- brice · · 2026-07-25 08:23 UTC
- parent
- 99ffb2f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +63 -30
M internal/usecase/daemon/floating_render_test.go +8 -0
| @@ -31,6 +31,9 @@ func applyFloatingResizePlanForTest(d *Daemon, p *pane, geometry floatingGeometr | |||
| 31 | 31 | p.popupGeometry = geometry | |
| 32 | 32 | p.screen.Resize(geometry.Inner.Width, geometry.Inner.Height) | |
| 33 | 33 | p.mu.Unlock() | |
| 34 | + | p.resizeMu.Lock() | |
| 35 | + | d.replayResizePending(member.session, member.tab, p, false, member.rect) | |
| 36 | + | p.resizeMu.Unlock() | |
| 34 | 37 | return true | |
| 35 | 38 | } | |
| 36 | 39 | ||
| @@ -719,6 +722,11 @@ func TestResizeFloatingPaneFailureAndSerialization(t *testing.T) { | |||
| 719 | 722 | require.Equal(t, second, p.popupGeometry) | |
| 720 | 723 | require.Equal(t, 1, pty.maxConcurrentCalls()) | |
| 721 | 724 | require.Equal(t, []domain.Size{{Cols: 4, Rows: 3}, {Cols: 8, Rows: 6}}, pty.sizes()) | |
| 725 | + | p.mu.Lock() | |
| 726 | + | applying, pending := p.resizeApplying, append([]byte(nil), p.resizePending...) | |
| 727 | + | p.mu.Unlock() | |
| 728 | + | require.False(t, applying, "successful helper apply must release its parser gate") | |
| 729 | + | require.Empty(t, pending, "successful helper apply must drain pending parser bytes") | |
| 722 | 730 | }) | |
| 723 | 731 | } | |
| 724 | 732 | ||
| @@ -31,6 +31,9 @@ func applyFloatingResizePlanForTest(d *Daemon, p *pane, geometry floatingGeometr | |||||
| 31 | p.popupGeometry = geometry | 31 | p.popupGeometry = geometry | ||
| 32 | p.screen.Resize(geometry.Inner.Width, geometry.Inner.Height) | 32 | p.screen.Resize(geometry.Inner.Width, geometry.Inner.Height) | ||
| 33 | p.mu.Unlock() | 33 | p.mu.Unlock() | ||
| 34 | + | p.resizeMu.Lock() | |||
| 35 | + | d.replayResizePending(member.session, member.tab, p, false, member.rect) | |||
| 36 | + | p.resizeMu.Unlock() | |||
| 34 | return true | 37 | return true | ||
| 35 | } | 38 | } | ||
| 36 | 39 | ||||
| @@ -719,6 +722,11 @@ func TestResizeFloatingPaneFailureAndSerialization(t *testing.T) { | |||||
| 719 | require.Equal(t, second, p.popupGeometry) | 722 | require.Equal(t, second, p.popupGeometry) | ||
| 720 | require.Equal(t, 1, pty.maxConcurrentCalls()) | 723 | require.Equal(t, 1, pty.maxConcurrentCalls()) | ||
| 721 | require.Equal(t, []domain.Size{{Cols: 4, Rows: 3}, {Cols: 8, Rows: 6}}, pty.sizes()) | 724 | require.Equal(t, []domain.Size{{Cols: 4, Rows: 3}, {Cols: 8, Rows: 6}}, pty.sizes()) | ||
| 725 | + | p.mu.Lock() | |||
| 726 | + | applying, pending := p.resizeApplying, append([]byte(nil), p.resizePending...) | |||
| 727 | + | p.mu.Unlock() | |||
| 728 | + | require.False(t, applying, "successful helper apply must release its parser gate") | |||
| 729 | + | require.Empty(t, pending, "successful helper apply must drain pending parser bytes") | |||
| 722 | }) | 730 | }) | ||
| 723 | } | 731 | } | ||
| 724 | 732 | ||||
M internal/usecase/daemon/transactional_resize.go +3 -0
| @@ -495,6 +495,9 @@ func (d *Daemon) applySessionLayout(sess *session, size domain.Size, current, ad | |||
| 495 | 495 | if valid && d.beforeSessionResizePublication != nil { | |
| 496 | 496 | d.beforeSessionResizePublication() | |
| 497 | 497 | } | |
| 498 | + | if valid && sess.ctx != nil && sess.ctx.Err() != nil { | |
| 499 | + | valid = false | |
| 500 | + | } | |
| 498 | 501 | if valid && current != nil && !current() { | |
| 499 | 502 | valid = false | |
| 500 | 503 | } | |
| @@ -495,6 +495,9 @@ func (d *Daemon) applySessionLayout(sess *session, size domain.Size, current, ad | |||||
| 495 | if valid && d.beforeSessionResizePublication != nil { | 495 | if valid && d.beforeSessionResizePublication != nil { | ||
| 496 | d.beforeSessionResizePublication() | 496 | d.beforeSessionResizePublication() | ||
| 497 | } | 497 | } | ||
| 498 | + | if valid && sess.ctx != nil && sess.ctx.Err() != nil { | |||
| 499 | + | valid = false | |||
| 500 | + | } | |||
| 498 | if valid && current != nil && !current() { | 501 | if valid && current != nil && !current() { | ||
| 499 | valid = false | 502 | valid = false | ||
| 500 | } | 503 | } | ||
M internal/usecase/daemon/transactional_resize_test.go +43 -19
| @@ -184,16 +184,20 @@ func TestProcessPTYDataRetainsCallbacksDuringResizeReplay(t *testing.T) { | |||
| 184 | 184 | func TestApplySessionLayoutStopsRetryWhenSessionCanceled(t *testing.T) { | |
| 185 | 185 | pty := &transactionalResizePTY{} | |
| 186 | 186 | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |
| 187 | - | admissions := 0 | |
| 187 | + | tb, p := sess.activeTab(), sess.activeTab().focusedPane() | |
| 188 | 188 | d.beforeSessionResizePublication = sess.cancel | |
| 189 | 189 | ||
| 190 | - | _, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, func() bool { | |
| 191 | - | admissions++ | |
| 192 | - | return admissions > 1 | |
| 193 | - | }) | |
| 190 | + | _, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, nil) | |
| 194 | 191 | ||
| 195 | 192 | require.False(t, ok) | |
| 196 | 193 | require.Len(t, pty.requested(), 1, "cancellation must stop before a stale retry applies another PTY resize") | |
| 194 | + | tb.mu.Lock() | |
| 195 | + | require.Equal(t, domain.Size{Cols: 80, Rows: 23}, tb.size, "canceled session published a tab size") | |
| 196 | + | tb.mu.Unlock() | |
| 197 | + | p.mu.Lock() | |
| 198 | + | require.Equal(t, domain.Rect{Width: 80, Height: 23}, p.rect, "canceled session published a pane rectangle") | |
| 199 | + | require.Equal(t, domain.Size{Cols: 80, Rows: 23}, domain.Size{Cols: p.screen.Frame.Width, Rows: p.screen.Frame.Height}, "canceled session published a VT size") | |
| 200 | + | p.mu.Unlock() | |
| 197 | 201 | } | |
| 198 | 202 | ||
| 199 | 203 | func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | |
| @@ -223,21 +227,41 @@ func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | |||
| 223 | 227 | // rectangle state. Successful members and the client layout commit together; | |
| 224 | 228 | // composition clips/pads the retained failed screen against that new layout. | |
| 225 | 229 | func TestApplySessionLayoutReportsResizeFailuresOnce(t *testing.T) { | |
| 226 | - | first := &transactionalResizePTY{errs: []error{errors.New("first failure")}} | |
| 227 | - | second := &transactionalResizePTY{errs: []error{errors.New("second failure")}} | |
| 228 | - | d, sess, _, _ := newManualSessionWithPTYs(t, first, second) | |
| 229 | - | ||
| 230 | - | failed, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, nil) | |
| 231 | - | ||
| 232 | - | require.True(t, ok) | |
| 233 | - | require.Len(t, failed, 2) | |
| 234 | - | count := 0 | |
| 235 | - | for _, notification := range d.notices.history() { | |
| 236 | - | if notification.Code == domain.NoticeResizeFailed { | |
| 237 | - | count++ | |
| 238 | - | } | |
| 230 | + | for _, tc := range []struct { | |
| 231 | + | name string | |
| 232 | + | failureCount int | |
| 233 | + | wantOK bool | |
| 234 | + | wantFailed int | |
| 235 | + | wantFailureNotice int | |
| 236 | + | }{ | |
| 237 | + | {name: "zero failures", wantOK: true}, | |
| 238 | + | {name: "one failure", failureCount: 1, wantOK: true, wantFailed: 1, wantFailureNotice: 1}, | |
| 239 | + | {name: "two failures", failureCount: 2, wantOK: true, wantFailed: 2, wantFailureNotice: 1}, | |
| 240 | + | } { | |
| 241 | + | t.Run(tc.name, func(t *testing.T) { | |
| 242 | + | first := &transactionalResizePTY{} | |
| 243 | + | second := &transactionalResizePTY{} | |
| 244 | + | if tc.failureCount >= 1 { | |
| 245 | + | first.errs = []error{errors.New("first failure")} | |
| 246 | + | } | |
| 247 | + | if tc.failureCount >= 2 { | |
| 248 | + | second.errs = []error{errors.New("second failure")} | |
| 249 | + | } | |
| 250 | + | d, sess, _, _ := newManualSessionWithPTYs(t, first, second) | |
| 251 | + | ||
| 252 | + | failed, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, nil) | |
| 253 | + | ||
| 254 | + | require.Equal(t, tc.wantOK, ok) | |
| 255 | + | require.Len(t, failed, tc.wantFailed) | |
| 256 | + | count := 0 | |
| 257 | + | for _, notification := range d.notices.history() { | |
| 258 | + | if notification.Code == domain.NoticeResizeFailed { | |
| 259 | + | count++ | |
| 260 | + | } | |
| 261 | + | } | |
| 262 | + | require.Equal(t, tc.wantFailureNotice, count) | |
| 263 | + | }) | |
| 239 | 264 | } | |
| 240 | - | require.Equal(t, 1, count) | |
| 241 | 265 | } | |
| 242 | 266 | ||
| 243 | 267 | func TestTransactionalResizePartialFailureCommitsOnlySuccessfulPTYState(t *testing.T) { | |
| @@ -184,16 +184,20 @@ func TestProcessPTYDataRetainsCallbacksDuringResizeReplay(t *testing.T) { | |||||
| 184 | func TestApplySessionLayoutStopsRetryWhenSessionCanceled(t *testing.T) { | 184 | func TestApplySessionLayoutStopsRetryWhenSessionCanceled(t *testing.T) { | ||
| 185 | pty := &transactionalResizePTY{} | 185 | pty := &transactionalResizePTY{} | ||
| 186 | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | 186 | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | ||
| 187 | - | admissions := 0 | 187 | + | tb, p := sess.activeTab(), sess.activeTab().focusedPane() |
| 188 | d.beforeSessionResizePublication = sess.cancel | 188 | d.beforeSessionResizePublication = sess.cancel | ||
| 189 | 189 | ||||
| 190 | - | _, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, func() bool { | 190 | + | _, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, nil) |
| 191 | - | admissions++ | |||
| 192 | - | return admissions > 1 | |||
| 193 | - | }) | |||
| 194 | 191 | ||||
| 195 | require.False(t, ok) | 192 | require.False(t, ok) | ||
| 196 | require.Len(t, pty.requested(), 1, "cancellation must stop before a stale retry applies another PTY resize") | 193 | require.Len(t, pty.requested(), 1, "cancellation must stop before a stale retry applies another PTY resize") | ||
| 194 | + | tb.mu.Lock() | |||
| 195 | + | require.Equal(t, domain.Size{Cols: 80, Rows: 23}, tb.size, "canceled session published a tab size") | |||
| 196 | + | tb.mu.Unlock() | |||
| 197 | + | p.mu.Lock() | |||
| 198 | + | require.Equal(t, domain.Rect{Width: 80, Height: 23}, p.rect, "canceled session published a pane rectangle") | |||
| 199 | + | require.Equal(t, domain.Size{Cols: 80, Rows: 23}, domain.Size{Cols: p.screen.Frame.Width, Rows: p.screen.Frame.Height}, "canceled session published a VT size") | |||
| 200 | + | p.mu.Unlock() | |||
| 197 | } | 201 | } | ||
| 198 | 202 | ||||
| 199 | func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | 203 | func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | ||
| @@ -223,21 +227,41 @@ func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | |||||
| 223 | // rectangle state. Successful members and the client layout commit together; | 227 | // rectangle state. Successful members and the client layout commit together; | ||
| 224 | // composition clips/pads the retained failed screen against that new layout. | 228 | // composition clips/pads the retained failed screen against that new layout. | ||
| 225 | func TestApplySessionLayoutReportsResizeFailuresOnce(t *testing.T) { | 229 | func TestApplySessionLayoutReportsResizeFailuresOnce(t *testing.T) { | ||
| 226 | - | first := &transactionalResizePTY{errs: []error{errors.New("first failure")}} | 230 | + | for _, tc := range []struct { |
| 227 | - | second := &transactionalResizePTY{errs: []error{errors.New("second failure")}} | 231 | + | name string |
| 228 | - | d, sess, _, _ := newManualSessionWithPTYs(t, first, second) | 232 | + | failureCount int |
| 229 | - | 233 | + | wantOK bool | |
| 230 | - | failed, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, nil) | 234 | + | wantFailed int |
| 231 | - | 235 | + | wantFailureNotice int | |
| 232 | - | require.True(t, ok) | 236 | + | }{ |
| 233 | - | require.Len(t, failed, 2) | 237 | + | {name: "zero failures", wantOK: true}, |
| 234 | - | count := 0 | 238 | + | {name: "one failure", failureCount: 1, wantOK: true, wantFailed: 1, wantFailureNotice: 1}, |
| 235 | - | for _, notification := range d.notices.history() { | 239 | + | {name: "two failures", failureCount: 2, wantOK: true, wantFailed: 2, wantFailureNotice: 1}, |
| 236 | - | if notification.Code == domain.NoticeResizeFailed { | 240 | + | } { |
| 237 | - | count++ | 241 | + | t.Run(tc.name, func(t *testing.T) { |
| 238 | - | } | 242 | + | first := &transactionalResizePTY{} |
| 243 | + | second := &transactionalResizePTY{} | |||
| 244 | + | if tc.failureCount >= 1 { | |||
| 245 | + | first.errs = []error{errors.New("first failure")} | |||
| 246 | + | } | |||
| 247 | + | if tc.failureCount >= 2 { | |||
| 248 | + | second.errs = []error{errors.New("second failure")} | |||
| 249 | + | } | |||
| 250 | + | d, sess, _, _ := newManualSessionWithPTYs(t, first, second) | |||
| 251 | + | ||||
| 252 | + | failed, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, nil) | |||
| 253 | + | ||||
| 254 | + | require.Equal(t, tc.wantOK, ok) | |||
| 255 | + | require.Len(t, failed, tc.wantFailed) | |||
| 256 | + | count := 0 | |||
| 257 | + | for _, notification := range d.notices.history() { | |||
| 258 | + | if notification.Code == domain.NoticeResizeFailed { | |||
| 259 | + | count++ | |||
| 260 | + | } | |||
| 261 | + | } | |||
| 262 | + | require.Equal(t, tc.wantFailureNotice, count) | |||
| 263 | + | }) | |||
| 239 | } | 264 | } | ||
| 240 | - | require.Equal(t, 1, count) | |||
| 241 | } | 265 | } | ||
| 242 | 266 | ||||
| 243 | func TestTransactionalResizePartialFailureCommitsOnlySuccessfulPTYState(t *testing.T) { | 267 | func TestTransactionalResizePartialFailureCommitsOnlySuccessfulPTYState(t *testing.T) { | ||
M internal/usecase/layout/resize_test.go +9 -11
| @@ -142,6 +142,7 @@ func TestResizeFocusSelectsNearestTargetAndAdjacentSibling(t *testing.T) { | |||
| 142 | 142 | func TestResizeFocusMinimumAndTransactionalFailures(t *testing.T) { | |
| 143 | 143 | t.Parallel() | |
| 144 | 144 | ||
| 145 | + | minInt := -int(^uint(0)>>1) - 1 | |
| 145 | 146 | tests := []struct { | |
| 146 | 147 | name string | |
| 147 | 148 | tree *Tree | |
| @@ -159,6 +160,14 @@ func TestResizeFocusMinimumAndTransactionalFailures(t *testing.T) { | |||
| 159 | 160 | area: domain.Rect{Width: 43, Height: 2}, | |
| 160 | 161 | want: map[PaneID]int{"a": 22, "b": 20}, | |
| 161 | 162 | }, | |
| 163 | + | { | |
| 164 | + | name: "minimum integer clamps without overflow", | |
| 165 | + | tree: &Tree{Root: split(Horizontal, weightedLeaf("a", 30), weightedLeaf("b", 20)), Focus: "a"}, | |
| 166 | + | axis: Width, | |
| 167 | + | delta: minInt, | |
| 168 | + | area: domain.Rect{Width: 51, Height: 2}, | |
| 169 | + | want: map[PaneID]int{"a": 20, "b": 30}, | |
| 170 | + | }, | |
| 162 | 171 | { | |
| 163 | 172 | name: "partial growth stops at recursive donor minimum", | |
| 164 | 173 | tree: &Tree{Root: split(Horizontal, | |
| @@ -289,17 +298,6 @@ func TestEqualizeFailureIsTransactional(t *testing.T) { | |||
| 289 | 298 | require.Equal(t, before, tr) | |
| 290 | 299 | } | |
| 291 | 300 | ||
| 292 | - | func TestResizeFocusMinIntClampsWithoutOverflow(t *testing.T) { | |
| 293 | - | t.Parallel() | |
| 294 | - | ||
| 295 | - | tr := &Tree{Root: split(Horizontal, weightedLeaf("a", 30), weightedLeaf("b", 20)), Focus: "a"} | |
| 296 | - | minInt := -int(^uint(0)>>1) - 1 | |
| 297 | - | require.NoError(t, tr.ResizeFocus(Width, minInt, domain.Rect{Width: 51, Height: 2})) | |
| 298 | - | placements, ok := Solve(tr.Root, domain.Rect{Width: 51, Height: 2}) | |
| 299 | - | require.True(t, ok) | |
| 300 | - | require.Equal(t, map[PaneID]int{"a": 20, "b": 30}, placementWidths(placements)) | |
| 301 | - | } | |
| 302 | - | ||
| 303 | 301 | func TestCanResize(t *testing.T) { | |
| 304 | 302 | t.Parallel() | |
| 305 | 303 | ||
| @@ -142,6 +142,7 @@ func TestResizeFocusSelectsNearestTargetAndAdjacentSibling(t *testing.T) { | |||||
| 142 | func TestResizeFocusMinimumAndTransactionalFailures(t *testing.T) { | 142 | func TestResizeFocusMinimumAndTransactionalFailures(t *testing.T) { | ||
| 143 | t.Parallel() | 143 | t.Parallel() | ||
| 144 | 144 | ||||
| 145 | + | minInt := -int(^uint(0)>>1) - 1 | |||
| 145 | tests := []struct { | 146 | tests := []struct { | ||
| 146 | name string | 147 | name string | ||
| 147 | tree *Tree | 148 | tree *Tree | ||
| @@ -159,6 +160,14 @@ func TestResizeFocusMinimumAndTransactionalFailures(t *testing.T) { | |||||
| 159 | area: domain.Rect{Width: 43, Height: 2}, | 160 | area: domain.Rect{Width: 43, Height: 2}, | ||
| 160 | want: map[PaneID]int{"a": 22, "b": 20}, | 161 | want: map[PaneID]int{"a": 22, "b": 20}, | ||
| 161 | }, | 162 | }, | ||
| 163 | + | { | |||
| 164 | + | name: "minimum integer clamps without overflow", | |||
| 165 | + | tree: &Tree{Root: split(Horizontal, weightedLeaf("a", 30), weightedLeaf("b", 20)), Focus: "a"}, | |||
| 166 | + | axis: Width, | |||
| 167 | + | delta: minInt, | |||
| 168 | + | area: domain.Rect{Width: 51, Height: 2}, | |||
| 169 | + | want: map[PaneID]int{"a": 20, "b": 30}, | |||
| 170 | + | }, | |||
| 162 | { | 171 | { | ||
| 163 | name: "partial growth stops at recursive donor minimum", | 172 | name: "partial growth stops at recursive donor minimum", | ||
| 164 | tree: &Tree{Root: split(Horizontal, | 173 | tree: &Tree{Root: split(Horizontal, | ||
| @@ -289,17 +298,6 @@ func TestEqualizeFailureIsTransactional(t *testing.T) { | |||||
| 289 | require.Equal(t, before, tr) | 298 | require.Equal(t, before, tr) | ||
| 290 | } | 299 | } | ||
| 291 | 300 | ||||
| 292 | - | func TestResizeFocusMinIntClampsWithoutOverflow(t *testing.T) { | |||
| 293 | - | t.Parallel() | |||
| 294 | - | ||||
| 295 | - | tr := &Tree{Root: split(Horizontal, weightedLeaf("a", 30), weightedLeaf("b", 20)), Focus: "a"} | |||
| 296 | - | minInt := -int(^uint(0)>>1) - 1 | |||
| 297 | - | require.NoError(t, tr.ResizeFocus(Width, minInt, domain.Rect{Width: 51, Height: 2})) | |||
| 298 | - | placements, ok := Solve(tr.Root, domain.Rect{Width: 51, Height: 2}) | |||
| 299 | - | require.True(t, ok) | |||
| 300 | - | require.Equal(t, map[PaneID]int{"a": 20, "b": 30}, placementWidths(placements)) | |||
| 301 | - | } | |||
| 302 | - | ||||
| 303 | func TestCanResize(t *testing.T) { | 301 | func TestCanResize(t *testing.T) { | ||
| 304 | t.Parallel() | 302 | t.Parallel() | ||
| 305 | 303 | ||||