fix(resize): address final review findings
- commit
- 99ffb2f73c1c3dad5cf582f38ce198f7122816f6
- author
- brice · · 2026-07-25 07:52 UTC
- parent
- edadec9
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
12 files · +129 -287
M internal/usecase/daemon/control.go +1 -1
| @@ -425,7 +425,7 @@ func (e controlExec) resize(axis layout.Axis, delta int) error { | |||
| 425 | 425 | return e.runAction(daemonActionRequest{kind: daemonActionResizePane, axis: axis, delta: delta}) | |
| 426 | 426 | } | |
| 427 | 427 | func (e controlExec) EqualizePanes() error { | |
| 428 | - | return e.runAction(daemonActionRequest{kind: daemonActionEqualizePanes}) | |
| 428 | + | return resizeUserError(e.runAction(daemonActionRequest{kind: daemonActionEqualizePanes})) | |
| 429 | 429 | } | |
| 430 | 430 | func (e controlExec) focus(direction layout.Direction) error { | |
| 431 | 431 | return e.runAction(daemonActionRequest{kind: daemonActionFocusPane, direction: direction}) | |
| @@ -425,7 +425,7 @@ func (e controlExec) resize(axis layout.Axis, delta int) error { | |||||
| 425 | return e.runAction(daemonActionRequest{kind: daemonActionResizePane, axis: axis, delta: delta}) | 425 | return e.runAction(daemonActionRequest{kind: daemonActionResizePane, axis: axis, delta: delta}) | ||
| 426 | } | 426 | } | ||
| 427 | func (e controlExec) EqualizePanes() error { | 427 | func (e controlExec) EqualizePanes() error { | ||
| 428 | - | return e.runAction(daemonActionRequest{kind: daemonActionEqualizePanes}) | 428 | + | return resizeUserError(e.runAction(daemonActionRequest{kind: daemonActionEqualizePanes})) |
| 429 | } | 429 | } | ||
| 430 | func (e controlExec) focus(direction layout.Direction) error { | 430 | func (e controlExec) focus(direction layout.Direction) error { | ||
| 431 | return e.runAction(daemonActionRequest{kind: daemonActionFocusPane, direction: direction}) | 431 | return e.runAction(daemonActionRequest{kind: daemonActionFocusPane, direction: direction}) | ||
M internal/usecase/daemon/control_test.go +2 -1
| @@ -42,11 +42,12 @@ func TestPaletteAndControlShareExplicitDaemonActionTarget(t *testing.T) { | |||
| 42 | 42 | ||
| 43 | 43 | type actionRunnerSpy struct { | |
| 44 | 44 | requests []daemonActionRequest | |
| 45 | + | err error | |
| 45 | 46 | } | |
| 46 | 47 | ||
| 47 | 48 | func (s *actionRunnerSpy) Run(request daemonActionRequest) error { | |
| 48 | 49 | s.requests = append(s.requests, request) | |
| 49 | - | return nil | |
| 50 | + | return s.err | |
| 50 | 51 | } | |
| 51 | 52 | ||
| 52 | 53 | func TestHandleCommandRejectsVersionBeforeDecodeOrDispatch(t *testing.T) { | |
| @@ -42,11 +42,12 @@ func TestPaletteAndControlShareExplicitDaemonActionTarget(t *testing.T) { | |||||
| 42 | 42 | ||||
| 43 | type actionRunnerSpy struct { | 43 | type actionRunnerSpy struct { | ||
| 44 | requests []daemonActionRequest | 44 | requests []daemonActionRequest | ||
| 45 | + | err error | |||
| 45 | } | 46 | } | ||
| 46 | 47 | ||||
| 47 | func (s *actionRunnerSpy) Run(request daemonActionRequest) error { | 48 | func (s *actionRunnerSpy) Run(request daemonActionRequest) error { | ||
| 48 | s.requests = append(s.requests, request) | 49 | s.requests = append(s.requests, request) | ||
| 49 | - | return nil | 50 | + | return s.err |
| 50 | } | 51 | } | ||
| 51 | 52 | ||||
| 52 | func TestHandleCommandRejectsVersionBeforeDecodeOrDispatch(t *testing.T) { | 53 | func TestHandleCommandRejectsVersionBeforeDecodeOrDispatch(t *testing.T) { | ||
M internal/usecase/daemon/floating_render_test.go +2 -2
| @@ -20,8 +20,8 @@ func applyFloatingResizePlanForTest(d *Daemon, p *pane, geometry floatingGeometr | |||
| 20 | 20 | if p == nil || !geometry.valid() { | |
| 21 | 21 | return false | |
| 22 | 22 | } | |
| 23 | - | plan := resizePlan{members: []resizeMember{{pane: p, rect: geometry.Inner, floating: geometry, isFloating: true}}} | |
| 24 | - | d.applyResize(&plan) | |
| 23 | + | plan := preparedTabLayout{members: []resizeMember{{pane: p, rect: geometry.Inner, floating: geometry, isFloating: true}}} | |
| 24 | + | d.applyPreparedTabMembers(&plan) | |
| 25 | 25 | member := plan.members[0] | |
| 26 | 26 | if !member.ok { | |
| 27 | 27 | return false | |
| @@ -20,8 +20,8 @@ func applyFloatingResizePlanForTest(d *Daemon, p *pane, geometry floatingGeometr | |||||
| 20 | if p == nil || !geometry.valid() { | 20 | if p == nil || !geometry.valid() { | ||
| 21 | return false | 21 | return false | ||
| 22 | } | 22 | } | ||
| 23 | - | plan := resizePlan{members: []resizeMember{{pane: p, rect: geometry.Inner, floating: geometry, isFloating: true}}} | 23 | + | plan := preparedTabLayout{members: []resizeMember{{pane: p, rect: geometry.Inner, floating: geometry, isFloating: true}}} |
| 24 | - | d.applyResize(&plan) | 24 | + | d.applyPreparedTabMembers(&plan) |
| 25 | member := plan.members[0] | 25 | member := plan.members[0] | ||
| 26 | if !member.ok { | 26 | if !member.ok { | ||
| 27 | return false | 27 | return false | ||
M internal/usecase/daemon/mouse_panes.go +3 -11
| @@ -72,11 +72,13 @@ func layoutFingerprint(root *layout.Node) string { | |||
| 72 | 72 | return b.String() | |
| 73 | 73 | } | |
| 74 | 74 | ||
| 75 | + | const weightFingerprintMaxLength = 16 | |
| 76 | + | ||
| 75 | 77 | func layoutFingerprintLength(n *layout.Node) int { | |
| 76 | 78 | if n == nil { | |
| 77 | 79 | return 1 | |
| 78 | 80 | } | |
| 79 | - | length := 2 + weightFingerprintLength(n.Weight) + 1 + paneIDFingerprintLength(n.Leaf) + 1 + paneIDFingerprintLength(n.Expanded) + 2 | |
| 81 | + | length := 2 + weightFingerprintMaxLength + 1 + paneIDFingerprintLength(n.Leaf) + 1 + paneIDFingerprintLength(n.Expanded) + 2 | |
| 80 | 82 | for _, child := range n.Children { | |
| 81 | 83 | length += layoutFingerprintLength(child) + 1 | |
| 82 | 84 | } | |
| @@ -100,16 +102,6 @@ func decimalDigits(value int) int { | |||
| 100 | 102 | return digits | |
| 101 | 103 | } | |
| 102 | 104 | ||
| 103 | - | func weightFingerprintLength(weight float64) int { | |
| 104 | - | bits := math.Float64bits(weight) | |
| 105 | - | length := 1 | |
| 106 | - | for bits >= 16 { | |
| 107 | - | bits >>= 4 | |
| 108 | - | length++ | |
| 109 | - | } | |
| 110 | - | return length | |
| 111 | - | } | |
| 112 | - | ||
| 113 | 105 | func writeLayoutFingerprint(b *strings.Builder, n *layout.Node) { | |
| 114 | 106 | if n == nil { | |
| 115 | 107 | b.WriteByte('0') | |
| @@ -72,11 +72,13 @@ func layoutFingerprint(root *layout.Node) string { | |||||
| 72 | return b.String() | 72 | return b.String() | ||
| 73 | } | 73 | } | ||
| 74 | 74 | ||||
| 75 | + | const weightFingerprintMaxLength = 16 | |||
| 76 | + | ||||
| 75 | func layoutFingerprintLength(n *layout.Node) int { | 77 | func layoutFingerprintLength(n *layout.Node) int { | ||
| 76 | if n == nil { | 78 | if n == nil { | ||
| 77 | return 1 | 79 | return 1 | ||
| 78 | } | 80 | } | ||
| 79 | - | length := 2 + weightFingerprintLength(n.Weight) + 1 + paneIDFingerprintLength(n.Leaf) + 1 + paneIDFingerprintLength(n.Expanded) + 2 | 81 | + | length := 2 + weightFingerprintMaxLength + 1 + paneIDFingerprintLength(n.Leaf) + 1 + paneIDFingerprintLength(n.Expanded) + 2 |
| 80 | for _, child := range n.Children { | 82 | for _, child := range n.Children { | ||
| 81 | length += layoutFingerprintLength(child) + 1 | 83 | length += layoutFingerprintLength(child) + 1 | ||
| 82 | } | 84 | } | ||
| @@ -100,16 +102,6 @@ func decimalDigits(value int) int { | |||||
| 100 | return digits | 102 | return digits | ||
| 101 | } | 103 | } | ||
| 102 | 104 | ||||
| 103 | - | func weightFingerprintLength(weight float64) int { | |||
| 104 | - | bits := math.Float64bits(weight) | |||
| 105 | - | length := 1 | |||
| 106 | - | for bits >= 16 { | |||
| 107 | - | bits >>= 4 | |||
| 108 | - | length++ | |||
| 109 | - | } | |||
| 110 | - | return length | |||
| 111 | - | } | |||
| 112 | - | ||||
| 113 | func writeLayoutFingerprint(b *strings.Builder, n *layout.Node) { | 105 | func writeLayoutFingerprint(b *strings.Builder, n *layout.Node) { | ||
| 114 | if n == nil { | 106 | if n == nil { | ||
| 115 | b.WriteByte('0') | 107 | b.WriteByte('0') | ||
M internal/usecase/daemon/performance_benchmark_test.go +11 -2
| @@ -1251,9 +1251,18 @@ func (f *performanceFixture) resizeTo(size domain.Size) { | |||
| 1251 | 1251 | ||
| 1252 | 1252 | func (f *performanceFixture) retryLatest() { | |
| 1253 | 1253 | epoch := f.sess.renderCoordinator().resizeSnapshot().committed | |
| 1254 | - | plan := f.d.prepareResize(f.sess, f.ac.size) | |
| 1254 | + | f.sess.mu.Lock() | |
| 1255 | + | tabs := append([]*tab(nil), f.sess.tabs...) | |
| 1256 | + | f.sess.mu.Unlock() | |
| 1257 | + | members := make([]resizeMember, 0) | |
| 1258 | + | for _, tb := range tabs { | |
| 1259 | + | tb.mu.Lock() | |
| 1260 | + | plan := prepareTabLayoutForSizeLocked(f.sess, tb, tabSize(f.ac.size)) | |
| 1261 | + | tb.mu.Unlock() | |
| 1262 | + | members = append(members, plan.members...) | |
| 1263 | + | } | |
| 1255 | 1264 | failuresBefore, callsBefore := f.pty.metrics() | |
| 1256 | - | f.d.retryResizeMembers(f.sess, f.ac, f.sess.renderCoordinator().attachmentLease(f.ac), epoch, plan.members) | |
| 1265 | + | f.d.retryResizeMembers(f.sess, f.ac, f.sess.renderCoordinator().attachmentLease(f.ac), epoch, members) | |
| 1257 | 1266 | failuresAfter, callsAfter := f.pty.metrics() | |
| 1258 | 1267 | f.ptyFailures += failuresAfter - failuresBefore | |
| 1259 | 1268 | if callsAfter > callsBefore { | |
| @@ -1251,9 +1251,18 @@ func (f *performanceFixture) resizeTo(size domain.Size) { | |||||
| 1251 | 1251 | ||||
| 1252 | func (f *performanceFixture) retryLatest() { | 1252 | func (f *performanceFixture) retryLatest() { | ||
| 1253 | epoch := f.sess.renderCoordinator().resizeSnapshot().committed | 1253 | epoch := f.sess.renderCoordinator().resizeSnapshot().committed | ||
| 1254 | - | plan := f.d.prepareResize(f.sess, f.ac.size) | 1254 | + | f.sess.mu.Lock() |
| 1255 | + | tabs := append([]*tab(nil), f.sess.tabs...) | |||
| 1256 | + | f.sess.mu.Unlock() | |||
| 1257 | + | members := make([]resizeMember, 0) | |||
| 1258 | + | for _, tb := range tabs { | |||
| 1259 | + | tb.mu.Lock() | |||
| 1260 | + | plan := prepareTabLayoutForSizeLocked(f.sess, tb, tabSize(f.ac.size)) | |||
| 1261 | + | tb.mu.Unlock() | |||
| 1262 | + | members = append(members, plan.members...) | |||
| 1263 | + | } | |||
| 1255 | failuresBefore, callsBefore := f.pty.metrics() | 1264 | failuresBefore, callsBefore := f.pty.metrics() | ||
| 1256 | - | f.d.retryResizeMembers(f.sess, f.ac, f.sess.renderCoordinator().attachmentLease(f.ac), epoch, plan.members) | 1265 | + | f.d.retryResizeMembers(f.sess, f.ac, f.sess.renderCoordinator().attachmentLease(f.ac), epoch, members) |
| 1257 | failuresAfter, callsAfter := f.pty.metrics() | 1266 | failuresAfter, callsAfter := f.pty.metrics() | ||
| 1258 | f.ptyFailures += failuresAfter - failuresBefore | 1267 | f.ptyFailures += failuresAfter - failuresBefore | ||
| 1259 | if callsAfter > callsBefore { | 1268 | if callsAfter > callsBefore { | ||
M internal/usecase/daemon/resize_mode_test.go +48 -29
| @@ -42,6 +42,16 @@ func TestPaletteAndControlShareResizeAction(t *testing.T) { | |||
| 42 | 42 | } | |
| 43 | 43 | } | |
| 44 | 44 | ||
| 45 | + | func TestControlEqualizeMapsResizeErrors(t *testing.T) { | |
| 46 | + | runner := &actionRunnerSpy{err: layout.ErrTooSmall} | |
| 47 | + | err := (controlExec{actions: runner}).EqualizePanes() | |
| 48 | + | require.ErrorIs(t, err, layout.ErrTooSmall) | |
| 49 | + | var userErr *domain.UserError | |
| 50 | + | require.ErrorAs(t, err, &userErr) | |
| 51 | + | require.Len(t, runner.requests, 1) | |
| 52 | + | require.Equal(t, daemonActionEqualizePanes, runner.requests[0].kind) | |
| 53 | + | } | |
| 54 | + | ||
| 45 | 55 | func TestResizeControlHeadlessAndErrors(t *testing.T) { | |
| 46 | 56 | factory := &controlPTYFactory{} | |
| 47 | 57 | d := newTestDaemon(t, factory, stubClock{}) | |
| @@ -162,28 +172,29 @@ func TestRouteOverlayDecodesHorizontalArrows(t *testing.T) { | |||
| 162 | 172 | // parser prefixes are deliberately split across reads. | |
| 163 | 173 | func TestResizeModeKeysAndEscapes(t *testing.T) { | |
| 164 | 174 | tests := []struct { | |
| 165 | - | name string | |
| 166 | - | chunks [][]byte | |
| 167 | - | change bool | |
| 168 | - | exit bool | |
| 175 | + | name string | |
| 176 | + | chunks [][]byte | |
| 177 | + | change bool | |
| 178 | + | exit bool | |
| 179 | + | equalize bool | |
| 169 | 180 | }{ | |
| 170 | - | {"h", [][]byte{[]byte("h")}, true, false}, | |
| 171 | - | {"j", [][]byte{[]byte("j")}, true, false}, | |
| 172 | - | {"k", [][]byte{[]byte("k")}, true, false}, | |
| 173 | - | {"l", [][]byte{[]byte("l")}, true, false}, | |
| 174 | - | {"CSI left split", [][]byte{[]byte("\x1b["), []byte("D")}, true, false}, | |
| 175 | - | {"CSI right", [][]byte{[]byte("\x1b[C")}, true, false}, | |
| 176 | - | {"CSI up", [][]byte{[]byte("\x1b[A")}, true, false}, | |
| 177 | - | {"CSI down", [][]byte{[]byte("\x1b[B")}, true, false}, | |
| 178 | - | {"SS3 left", [][]byte{[]byte("\x1bOD")}, true, false}, | |
| 179 | - | {"SS3 right split", [][]byte{[]byte("\x1bO"), []byte("C")}, true, false}, | |
| 180 | - | {"SS3 up", [][]byte{[]byte("\x1bOA")}, true, false}, | |
| 181 | - | {"SS3 down", [][]byte{[]byte("\x1bOB")}, true, false}, | |
| 182 | - | {"equalize", [][]byte{[]byte("=")}, true, false}, | |
| 183 | - | {"ignored", [][]byte{[]byte("z\x00")}, false, false}, | |
| 184 | - | {"q exits", [][]byte{[]byte("q")}, false, true}, | |
| 185 | - | {"enter exits", [][]byte{[]byte("\r")}, false, true}, | |
| 186 | - | {"escape exits", [][]byte{[]byte("\x1b"), []byte("x")}, false, true}, | |
| 181 | + | {"h", [][]byte{[]byte("h")}, true, false, false}, | |
| 182 | + | {"j", [][]byte{[]byte("j")}, true, false, false}, | |
| 183 | + | {"k", [][]byte{[]byte("k")}, true, false, false}, | |
| 184 | + | {"l", [][]byte{[]byte("l")}, true, false, false}, | |
| 185 | + | {"CSI left split", [][]byte{[]byte("\x1b["), []byte("D")}, true, false, false}, | |
| 186 | + | {"CSI right", [][]byte{[]byte("\x1b[C")}, true, false, false}, | |
| 187 | + | {"CSI up", [][]byte{[]byte("\x1b[A")}, true, false, false}, | |
| 188 | + | {"CSI down", [][]byte{[]byte("\x1b[B")}, true, false, false}, | |
| 189 | + | {"SS3 left", [][]byte{[]byte("\x1bOD")}, true, false, false}, | |
| 190 | + | {"SS3 right split", [][]byte{[]byte("\x1bO"), []byte("C")}, true, false, false}, | |
| 191 | + | {"SS3 up", [][]byte{[]byte("\x1bOA")}, true, false, false}, | |
| 192 | + | {"SS3 down", [][]byte{[]byte("\x1bOB")}, true, false, false}, | |
| 193 | + | {"equalize", [][]byte{[]byte("=")}, true, false, true}, | |
| 194 | + | {"ignored", [][]byte{[]byte("z\x00")}, false, false, false}, | |
| 195 | + | {"q exits", [][]byte{[]byte("q")}, false, true, false}, | |
| 196 | + | {"enter exits", [][]byte{[]byte("\r")}, false, true, false}, | |
| 197 | + | {"escape exits", [][]byte{[]byte("\x1b"), []byte("x")}, false, true, false}, | |
| 187 | 198 | } | |
| 188 | 199 | for _, tt := range tests { | |
| 189 | 200 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -191,7 +202,7 @@ func TestResizeModeKeysAndEscapes(t *testing.T) { | |||
| 191 | 202 | tb := sess.activeTab() | |
| 192 | 203 | tb.mu.Lock() | |
| 193 | 204 | before := tb.layoutGeneration | |
| 194 | - | if tt.name == "equalize" { | |
| 205 | + | if tt.equalize { | |
| 195 | 206 | tb.tree.Root.Children[0].Weight = 3 | |
| 196 | 207 | } | |
| 197 | 208 | tb.mu.Unlock() | |
| @@ -200,7 +211,7 @@ func TestResizeModeKeysAndEscapes(t *testing.T) { | |||
| 200 | 211 | } | |
| 201 | 212 | tb.mu.Lock() | |
| 202 | 213 | after := tb.layoutGeneration | |
| 203 | - | if tt.name == "equalize" { | |
| 214 | + | if tt.equalize { | |
| 204 | 215 | for _, child := range tb.tree.Root.Children { | |
| 205 | 216 | require.Zero(t, child.Weight) | |
| 206 | 217 | } | |
| @@ -348,8 +359,10 @@ func (*resizeLockProbePTY) Pid() int { return 0 } | |||
| 348 | 359 | func (*resizeLockProbePTY) ForegroundPgid() (int, error) { return 0, nil } | |
| 349 | 360 | func (p *resizeLockProbePTY) Resize(domain.Size) error { | |
| 350 | 361 | p.once.Do(func() { | |
| 351 | - | p.check() | |
| 352 | - | close(p.entered) | |
| 362 | + | func() { | |
| 363 | + | defer close(p.entered) | |
| 364 | + | p.check() | |
| 365 | + | }() | |
| 353 | 366 | <-p.release | |
| 354 | 367 | }) | |
| 355 | 368 | return nil | |
| @@ -367,10 +380,16 @@ func TestResizeModeReleasesTabAndOverlayLocksBeforePTY(t *testing.T) { | |||
| 367 | 380 | tb.panes[second.id] = second | |
| 368 | 381 | tb.mu.Unlock() | |
| 369 | 382 | probe.check = func() { | |
| 370 | - | require.True(t, tb.mu.TryLock(), "PTY callback must not inherit tab.mu") | |
| 371 | - | tb.mu.Unlock() | |
| 372 | - | require.True(t, ac.overlays.resizeMu.TryLock(), "PTY callback must not inherit resize overlay mutex") | |
| 373 | - | ac.overlays.resizeMu.Unlock() | |
| 383 | + | if tb.mu.TryLock() { | |
| 384 | + | tb.mu.Unlock() | |
| 385 | + | } else { | |
| 386 | + | t.Error("PTY callback must not inherit tab.mu") | |
| 387 | + | } | |
| 388 | + | if ac.overlays.resizeMu.TryLock() { | |
| 389 | + | ac.overlays.resizeMu.Unlock() | |
| 390 | + | } else { | |
| 391 | + | t.Error("PTY callback must not inherit resize overlay mutex") | |
| 392 | + | } | |
| 374 | 393 | } | |
| 375 | 394 | require.NoError(t, d.enterResizeMode(sess, ac)) | |
| 376 | 395 | done := make(chan struct{}) | |
| @@ -42,6 +42,16 @@ func TestPaletteAndControlShareResizeAction(t *testing.T) { | |||||
| 42 | } | 42 | } | ||
| 43 | } | 43 | } | ||
| 44 | 44 | ||||
| 45 | + | func TestControlEqualizeMapsResizeErrors(t *testing.T) { | |||
| 46 | + | runner := &actionRunnerSpy{err: layout.ErrTooSmall} | |||
| 47 | + | err := (controlExec{actions: runner}).EqualizePanes() | |||
| 48 | + | require.ErrorIs(t, err, layout.ErrTooSmall) | |||
| 49 | + | var userErr *domain.UserError | |||
| 50 | + | require.ErrorAs(t, err, &userErr) | |||
| 51 | + | require.Len(t, runner.requests, 1) | |||
| 52 | + | require.Equal(t, daemonActionEqualizePanes, runner.requests[0].kind) | |||
| 53 | + | } | |||
| 54 | + | ||||
| 45 | func TestResizeControlHeadlessAndErrors(t *testing.T) { | 55 | func TestResizeControlHeadlessAndErrors(t *testing.T) { | ||
| 46 | factory := &controlPTYFactory{} | 56 | factory := &controlPTYFactory{} | ||
| 47 | d := newTestDaemon(t, factory, stubClock{}) | 57 | d := newTestDaemon(t, factory, stubClock{}) | ||
| @@ -162,28 +172,29 @@ func TestRouteOverlayDecodesHorizontalArrows(t *testing.T) { | |||||
| 162 | // parser prefixes are deliberately split across reads. | 172 | // parser prefixes are deliberately split across reads. | ||
| 163 | func TestResizeModeKeysAndEscapes(t *testing.T) { | 173 | func TestResizeModeKeysAndEscapes(t *testing.T) { | ||
| 164 | tests := []struct { | 174 | tests := []struct { | ||
| 165 | - | name string | 175 | + | name string |
| 166 | - | chunks [][]byte | 176 | + | chunks [][]byte |
| 167 | - | change bool | 177 | + | change bool |
| 168 | - | exit bool | 178 | + | exit bool |
| 179 | + | equalize bool | |||
| 169 | }{ | 180 | }{ | ||
| 170 | - | {"h", [][]byte{[]byte("h")}, true, false}, | 181 | + | {"h", [][]byte{[]byte("h")}, true, false, false}, |
| 171 | - | {"j", [][]byte{[]byte("j")}, true, false}, | 182 | + | {"j", [][]byte{[]byte("j")}, true, false, false}, |
| 172 | - | {"k", [][]byte{[]byte("k")}, true, false}, | 183 | + | {"k", [][]byte{[]byte("k")}, true, false, false}, |
| 173 | - | {"l", [][]byte{[]byte("l")}, true, false}, | 184 | + | {"l", [][]byte{[]byte("l")}, true, false, false}, |
| 174 | - | {"CSI left split", [][]byte{[]byte("\x1b["), []byte("D")}, true, false}, | 185 | + | {"CSI left split", [][]byte{[]byte("\x1b["), []byte("D")}, true, false, false}, |
| 175 | - | {"CSI right", [][]byte{[]byte("\x1b[C")}, true, false}, | 186 | + | {"CSI right", [][]byte{[]byte("\x1b[C")}, true, false, false}, |
| 176 | - | {"CSI up", [][]byte{[]byte("\x1b[A")}, true, false}, | 187 | + | {"CSI up", [][]byte{[]byte("\x1b[A")}, true, false, false}, |
| 177 | - | {"CSI down", [][]byte{[]byte("\x1b[B")}, true, false}, | 188 | + | {"CSI down", [][]byte{[]byte("\x1b[B")}, true, false, false}, |
| 178 | - | {"SS3 left", [][]byte{[]byte("\x1bOD")}, true, false}, | 189 | + | {"SS3 left", [][]byte{[]byte("\x1bOD")}, true, false, false}, |
| 179 | - | {"SS3 right split", [][]byte{[]byte("\x1bO"), []byte("C")}, true, false}, | 190 | + | {"SS3 right split", [][]byte{[]byte("\x1bO"), []byte("C")}, true, false, false}, |
| 180 | - | {"SS3 up", [][]byte{[]byte("\x1bOA")}, true, false}, | 191 | + | {"SS3 up", [][]byte{[]byte("\x1bOA")}, true, false, false}, |
| 181 | - | {"SS3 down", [][]byte{[]byte("\x1bOB")}, true, false}, | 192 | + | {"SS3 down", [][]byte{[]byte("\x1bOB")}, true, false, false}, |
| 182 | - | {"equalize", [][]byte{[]byte("=")}, true, false}, | 193 | + | {"equalize", [][]byte{[]byte("=")}, true, false, true}, |
| 183 | - | {"ignored", [][]byte{[]byte("z\x00")}, false, false}, | 194 | + | {"ignored", [][]byte{[]byte("z\x00")}, false, false, false}, |
| 184 | - | {"q exits", [][]byte{[]byte("q")}, false, true}, | 195 | + | {"q exits", [][]byte{[]byte("q")}, false, true, false}, |
| 185 | - | {"enter exits", [][]byte{[]byte("\r")}, false, true}, | 196 | + | {"enter exits", [][]byte{[]byte("\r")}, false, true, false}, |
| 186 | - | {"escape exits", [][]byte{[]byte("\x1b"), []byte("x")}, false, true}, | 197 | + | {"escape exits", [][]byte{[]byte("\x1b"), []byte("x")}, false, true, false}, |
| 187 | } | 198 | } | ||
| 188 | for _, tt := range tests { | 199 | for _, tt := range tests { | ||
| 189 | t.Run(tt.name, func(t *testing.T) { | 200 | t.Run(tt.name, func(t *testing.T) { | ||
| @@ -191,7 +202,7 @@ func TestResizeModeKeysAndEscapes(t *testing.T) { | |||||
| 191 | tb := sess.activeTab() | 202 | tb := sess.activeTab() | ||
| 192 | tb.mu.Lock() | 203 | tb.mu.Lock() | ||
| 193 | before := tb.layoutGeneration | 204 | before := tb.layoutGeneration | ||
| 194 | - | if tt.name == "equalize" { | 205 | + | if tt.equalize { |
| 195 | tb.tree.Root.Children[0].Weight = 3 | 206 | tb.tree.Root.Children[0].Weight = 3 | ||
| 196 | } | 207 | } | ||
| 197 | tb.mu.Unlock() | 208 | tb.mu.Unlock() | ||
| @@ -200,7 +211,7 @@ func TestResizeModeKeysAndEscapes(t *testing.T) { | |||||
| 200 | } | 211 | } | ||
| 201 | tb.mu.Lock() | 212 | tb.mu.Lock() | ||
| 202 | after := tb.layoutGeneration | 213 | after := tb.layoutGeneration | ||
| 203 | - | if tt.name == "equalize" { | 214 | + | if tt.equalize { |
| 204 | for _, child := range tb.tree.Root.Children { | 215 | for _, child := range tb.tree.Root.Children { | ||
| 205 | require.Zero(t, child.Weight) | 216 | require.Zero(t, child.Weight) | ||
| 206 | } | 217 | } | ||
| @@ -348,8 +359,10 @@ func (*resizeLockProbePTY) Pid() int { return 0 } | |||||
| 348 | func (*resizeLockProbePTY) ForegroundPgid() (int, error) { return 0, nil } | 359 | func (*resizeLockProbePTY) ForegroundPgid() (int, error) { return 0, nil } | ||
| 349 | func (p *resizeLockProbePTY) Resize(domain.Size) error { | 360 | func (p *resizeLockProbePTY) Resize(domain.Size) error { | ||
| 350 | p.once.Do(func() { | 361 | p.once.Do(func() { | ||
| 351 | - | p.check() | 362 | + | func() { |
| 352 | - | close(p.entered) | 363 | + | defer close(p.entered) |
| 364 | + | p.check() | |||
| 365 | + | }() | |||
| 353 | <-p.release | 366 | <-p.release | ||
| 354 | }) | 367 | }) | ||
| 355 | return nil | 368 | return nil | ||
| @@ -367,10 +380,16 @@ func TestResizeModeReleasesTabAndOverlayLocksBeforePTY(t *testing.T) { | |||||
| 367 | tb.panes[second.id] = second | 380 | tb.panes[second.id] = second | ||
| 368 | tb.mu.Unlock() | 381 | tb.mu.Unlock() | ||
| 369 | probe.check = func() { | 382 | probe.check = func() { | ||
| 370 | - | require.True(t, tb.mu.TryLock(), "PTY callback must not inherit tab.mu") | 383 | + | if tb.mu.TryLock() { |
| 371 | - | tb.mu.Unlock() | 384 | + | tb.mu.Unlock() |
| 372 | - | require.True(t, ac.overlays.resizeMu.TryLock(), "PTY callback must not inherit resize overlay mutex") | 385 | + | } else { |
| 373 | - | ac.overlays.resizeMu.Unlock() | 386 | + | t.Error("PTY callback must not inherit tab.mu") |
| 387 | + | } | |||
| 388 | + | if ac.overlays.resizeMu.TryLock() { | |||
| 389 | + | ac.overlays.resizeMu.Unlock() | |||
| 390 | + | } else { | |||
| 391 | + | t.Error("PTY callback must not inherit resize overlay mutex") | |||
| 392 | + | } | |||
| 374 | } | 393 | } | ||
| 375 | require.NoError(t, d.enterResizeMode(sess, ac)) | 394 | require.NoError(t, d.enterResizeMode(sess, ac)) | ||
| 376 | done := make(chan struct{}) | 395 | done := make(chan struct{}) | ||
M internal/usecase/daemon/transactional_resize.go +4 -100
| @@ -27,12 +27,6 @@ type resizeMember struct { | |||
| 27 | 27 | screenResized bool | |
| 28 | 28 | } | |
| 29 | 29 | ||
| 30 | - | type resizePlan struct { | |
| 31 | - | size domain.Size | |
| 32 | - | tabs []*tab | |
| 33 | - | members []resizeMember | |
| 34 | - | } | |
| 35 | - | ||
| 36 | 30 | type preparedTabLayout struct { | |
| 37 | 31 | tab *tab | |
| 38 | 32 | generation uint64 | |
| @@ -314,99 +308,6 @@ func (d *Daemon) scheduleAcceptedTabLayoutRetry(sess *session, tb *tab) { | |||
| 314 | 308 | }() | |
| 315 | 309 | } | |
| 316 | 310 | ||
| 317 | - | func (d *Daemon) prepareResize(sess *session, size domain.Size) resizePlan { | |
| 318 | - | plan := resizePlan{size: size} | |
| 319 | - | sess.mu.Lock() | |
| 320 | - | plan.tabs = append(plan.tabs, sess.tabs...) | |
| 321 | - | active := sess.active | |
| 322 | - | sess.mu.Unlock() | |
| 323 | - | for i, tb := range plan.tabs { | |
| 324 | - | tb.mu.Lock() | |
| 325 | - | if tb.tree != nil && tb.tree.Root != nil { | |
| 326 | - | ts := tabSize(size) | |
| 327 | - | area := domain.Rect{Width: ts.Cols, Height: ts.Rows} | |
| 328 | - | if placements, ok := layout.Solve(tb.tree.Root, area); ok { | |
| 329 | - | for _, pl := range placements { | |
| 330 | - | if !pl.Collapsed && pl.Content.Width > 0 && pl.Content.Height > 0 { | |
| 331 | - | if p := tb.panes[pl.ID]; p != nil { | |
| 332 | - | plan.members = append(plan.members, resizeMember{session: sess, tab: tb, pane: p, rect: pl.Content}) | |
| 333 | - | } | |
| 334 | - | } | |
| 335 | - | } | |
| 336 | - | } else if tb.tree.Root.Kind == layout.Leaf { | |
| 337 | - | // A single pane remains a valid PTY target even when a tiny client | |
| 338 | - | // is below the interactive layout minimum. Keeping it live avoids | |
| 339 | - | // stale screen dimensions (and lost scrollback) until the client | |
| 340 | - | // grows back into a solvable layout. | |
| 341 | - | if p := tb.panes[tb.tree.Root.Leaf]; p != nil { | |
| 342 | - | plan.members = append(plan.members, resizeMember{session: sess, tab: tb, pane: p, rect: area}) | |
| 343 | - | } | |
| 344 | - | } | |
| 345 | - | } | |
| 346 | - | // Hidden retained popups enter this same primitive when shown; only a | |
| 347 | - | // visible popup participates in an unrelated client resize. | |
| 348 | - | if i == active && tb.floating.state == floatingVisible && tb.floating.pane != nil { | |
| 349 | - | g := calculateContentFloatingGeometry(tabSize(size), d.currentFloatingConfig()) | |
| 350 | - | if g.valid() { | |
| 351 | - | plan.members = append(plan.members, resizeMember{session: sess, tab: tb, pane: tb.floating.pane, rect: g.Inner, floating: g, isFloating: true}) | |
| 352 | - | } | |
| 353 | - | } | |
| 354 | - | tb.mu.Unlock() | |
| 355 | - | } | |
| 356 | - | return plan | |
| 357 | - | } | |
| 358 | - | ||
| 359 | - | // applyResize deliberately holds only a pane's resizeMu around its PTY call; | |
| 360 | - | // no daemon, session, tab, or pane lock crosses the external boundary. | |
| 361 | - | func (d *Daemon) applyResize(plan *resizePlan, current ...func() bool) bool { | |
| 362 | - | // Collect degradation across the loop and report once from the explicit | |
| 363 | - | // lock-free return points. notify acquires sess.mu/d.mu and repaints, so it | |
| 364 | - | // must never run while a pane's resizeMu is held. | |
| 365 | - | failed := 0 | |
| 366 | - | var lastErr error | |
| 367 | - | reportFailure := func() { | |
| 368 | - | if failed == 0 { | |
| 369 | - | return | |
| 370 | - | } | |
| 371 | - | // A failure can only come from a plan member, so members[0] is safe. | |
| 372 | - | d.notify(plan.members[0].session, domain.NoticeWarn, domain.NoticeResizeFailed, | |
| 373 | - | "pane resize failed; retrying in background", lastErr) | |
| 374 | - | } | |
| 375 | - | for i := range plan.members { | |
| 376 | - | if len(current) != 0 && !current[0]() { | |
| 377 | - | reportFailure() | |
| 378 | - | return false | |
| 379 | - | } | |
| 380 | - | m := &plan.members[i] | |
| 381 | - | m.pane.resizeMu.Lock() | |
| 382 | - | m.pane.mu.Lock() | |
| 383 | - | old, pty := m.pane.rect, m.pane.pty | |
| 384 | - | screenSize := domain.Size{Cols: m.pane.screen.Frame.Width, Rows: m.pane.screen.Frame.Height} | |
| 385 | - | needsApply := m.retry || old.Width != m.rect.Width || old.Height != m.rect.Height || screenSize != rectSize(m.rect) | |
| 386 | - | if needsApply && pty != nil { | |
| 387 | - | // Publish this gate before the external call. ptyReader keeps draining | |
| 388 | - | // but does not parse bytes at the stale width while Resize is in flight. | |
| 389 | - | m.pane.resizeApplying = true | |
| 390 | - | } | |
| 391 | - | m.pane.mu.Unlock() | |
| 392 | - | if !needsApply || pty == nil { | |
| 393 | - | m.ok = true | |
| 394 | - | } else if err := pty.Resize(rectSize(m.rect)); err != nil { | |
| 395 | - | d.log.Warn("pty resize failed", "err", err) | |
| 396 | - | failed++ | |
| 397 | - | lastErr = err | |
| 398 | - | d.replayResizePending(m.session, m.tab, m.pane, false, m.rect) | |
| 399 | - | } else { | |
| 400 | - | m.ok = true | |
| 401 | - | d.replayResizePending(m.session, m.tab, m.pane, true, m.rect) | |
| 402 | - | m.screenResized = true | |
| 403 | - | } | |
| 404 | - | m.pane.resizeMu.Unlock() | |
| 405 | - | } | |
| 406 | - | reportFailure() | |
| 407 | - | return true | |
| 408 | - | } | |
| 409 | - | ||
| 410 | 311 | // replayResizePending completes a pane's apply epoch. It keeps the gate set | |
| 411 | 312 | // while each buffered batch goes through the normal VT path, so reads arriving | |
| 412 | 313 | // during replay join the same ordered stream. Failure deliberately omits the | |
| @@ -469,7 +370,7 @@ func (d *Daemon) applyVisibleFloatingLayoutForMember(sess *session, tb *tab, cur | |||
| 469 | 370 | return nil, true | |
| 470 | 371 | } | |
| 471 | 372 | ||
| 472 | - | // Unlike applyResize, this keeps successful PTY resizes gated until the | |
| 373 | + | // This keeps successful PTY resizes gated until the | |
| 473 | 374 | // floating slot and tab size are revalidated. A newer client resize may | |
| 474 | 375 | // otherwise publish this obsolete popup geometry after its PTY call returns. | |
| 475 | 376 | plan := preparedTabLayout{members: []resizeMember{{ | |
| @@ -554,6 +455,9 @@ func (d *Daemon) applySessionLayout(sess *session, size domain.Size, current, ad | |||
| 554 | 455 | ||
| 555 | 456 | target := tabSize(size) | |
| 556 | 457 | for { | |
| 458 | + | if sess.ctx != nil && sess.ctx.Err() != nil { | |
| 459 | + | return nil, false | |
| 460 | + | } | |
| 557 | 461 | if current != nil && !current() { | |
| 558 | 462 | return nil, false | |
| 559 | 463 | } | |
| @@ -27,12 +27,6 @@ type resizeMember struct { | |||||
| 27 | screenResized bool | 27 | screenResized bool | ||
| 28 | } | 28 | } | ||
| 29 | 29 | ||||
| 30 | - | type resizePlan struct { | |||
| 31 | - | size domain.Size | |||
| 32 | - | tabs []*tab | |||
| 33 | - | members []resizeMember | |||
| 34 | - | } | |||
| 35 | - | ||||
| 36 | type preparedTabLayout struct { | 30 | type preparedTabLayout struct { | ||
| 37 | tab *tab | 31 | tab *tab | ||
| 38 | generation uint64 | 32 | generation uint64 | ||
| @@ -314,99 +308,6 @@ func (d *Daemon) scheduleAcceptedTabLayoutRetry(sess *session, tb *tab) { | |||||
| 314 | }() | 308 | }() | ||
| 315 | } | 309 | } | ||
| 316 | 310 | ||||
| 317 | - | func (d *Daemon) prepareResize(sess *session, size domain.Size) resizePlan { | |||
| 318 | - | plan := resizePlan{size: size} | |||
| 319 | - | sess.mu.Lock() | |||
| 320 | - | plan.tabs = append(plan.tabs, sess.tabs...) | |||
| 321 | - | active := sess.active | |||
| 322 | - | sess.mu.Unlock() | |||
| 323 | - | for i, tb := range plan.tabs { | |||
| 324 | - | tb.mu.Lock() | |||
| 325 | - | if tb.tree != nil && tb.tree.Root != nil { | |||
| 326 | - | ts := tabSize(size) | |||
| 327 | - | area := domain.Rect{Width: ts.Cols, Height: ts.Rows} | |||
| 328 | - | if placements, ok := layout.Solve(tb.tree.Root, area); ok { | |||
| 329 | - | for _, pl := range placements { | |||
| 330 | - | if !pl.Collapsed && pl.Content.Width > 0 && pl.Content.Height > 0 { | |||
| 331 | - | if p := tb.panes[pl.ID]; p != nil { | |||
| 332 | - | plan.members = append(plan.members, resizeMember{session: sess, tab: tb, pane: p, rect: pl.Content}) | |||
| 333 | - | } | |||
| 334 | - | } | |||
| 335 | - | } | |||
| 336 | - | } else if tb.tree.Root.Kind == layout.Leaf { | |||
| 337 | - | // A single pane remains a valid PTY target even when a tiny client | |||
| 338 | - | // is below the interactive layout minimum. Keeping it live avoids | |||
| 339 | - | // stale screen dimensions (and lost scrollback) until the client | |||
| 340 | - | // grows back into a solvable layout. | |||
| 341 | - | if p := tb.panes[tb.tree.Root.Leaf]; p != nil { | |||
| 342 | - | plan.members = append(plan.members, resizeMember{session: sess, tab: tb, pane: p, rect: area}) | |||
| 343 | - | } | |||
| 344 | - | } | |||
| 345 | - | } | |||
| 346 | - | // Hidden retained popups enter this same primitive when shown; only a | |||
| 347 | - | // visible popup participates in an unrelated client resize. | |||
| 348 | - | if i == active && tb.floating.state == floatingVisible && tb.floating.pane != nil { | |||
| 349 | - | g := calculateContentFloatingGeometry(tabSize(size), d.currentFloatingConfig()) | |||
| 350 | - | if g.valid() { | |||
| 351 | - | plan.members = append(plan.members, resizeMember{session: sess, tab: tb, pane: tb.floating.pane, rect: g.Inner, floating: g, isFloating: true}) | |||
| 352 | - | } | |||
| 353 | - | } | |||
| 354 | - | tb.mu.Unlock() | |||
| 355 | - | } | |||
| 356 | - | return plan | |||
| 357 | - | } | |||
| 358 | - | ||||
| 359 | - | // applyResize deliberately holds only a pane's resizeMu around its PTY call; | |||
| 360 | - | // no daemon, session, tab, or pane lock crosses the external boundary. | |||
| 361 | - | func (d *Daemon) applyResize(plan *resizePlan, current ...func() bool) bool { | |||
| 362 | - | // Collect degradation across the loop and report once from the explicit | |||
| 363 | - | // lock-free return points. notify acquires sess.mu/d.mu and repaints, so it | |||
| 364 | - | // must never run while a pane's resizeMu is held. | |||
| 365 | - | failed := 0 | |||
| 366 | - | var lastErr error | |||
| 367 | - | reportFailure := func() { | |||
| 368 | - | if failed == 0 { | |||
| 369 | - | return | |||
| 370 | - | } | |||
| 371 | - | // A failure can only come from a plan member, so members[0] is safe. | |||
| 372 | - | d.notify(plan.members[0].session, domain.NoticeWarn, domain.NoticeResizeFailed, | |||
| 373 | - | "pane resize failed; retrying in background", lastErr) | |||
| 374 | - | } | |||
| 375 | - | for i := range plan.members { | |||
| 376 | - | if len(current) != 0 && !current[0]() { | |||
| 377 | - | reportFailure() | |||
| 378 | - | return false | |||
| 379 | - | } | |||
| 380 | - | m := &plan.members[i] | |||
| 381 | - | m.pane.resizeMu.Lock() | |||
| 382 | - | m.pane.mu.Lock() | |||
| 383 | - | old, pty := m.pane.rect, m.pane.pty | |||
| 384 | - | screenSize := domain.Size{Cols: m.pane.screen.Frame.Width, Rows: m.pane.screen.Frame.Height} | |||
| 385 | - | needsApply := m.retry || old.Width != m.rect.Width || old.Height != m.rect.Height || screenSize != rectSize(m.rect) | |||
| 386 | - | if needsApply && pty != nil { | |||
| 387 | - | // Publish this gate before the external call. ptyReader keeps draining | |||
| 388 | - | // but does not parse bytes at the stale width while Resize is in flight. | |||
| 389 | - | m.pane.resizeApplying = true | |||
| 390 | - | } | |||
| 391 | - | m.pane.mu.Unlock() | |||
| 392 | - | if !needsApply || pty == nil { | |||
| 393 | - | m.ok = true | |||
| 394 | - | } else if err := pty.Resize(rectSize(m.rect)); err != nil { | |||
| 395 | - | d.log.Warn("pty resize failed", "err", err) | |||
| 396 | - | failed++ | |||
| 397 | - | lastErr = err | |||
| 398 | - | d.replayResizePending(m.session, m.tab, m.pane, false, m.rect) | |||
| 399 | - | } else { | |||
| 400 | - | m.ok = true | |||
| 401 | - | d.replayResizePending(m.session, m.tab, m.pane, true, m.rect) | |||
| 402 | - | m.screenResized = true | |||
| 403 | - | } | |||
| 404 | - | m.pane.resizeMu.Unlock() | |||
| 405 | - | } | |||
| 406 | - | reportFailure() | |||
| 407 | - | return true | |||
| 408 | - | } | |||
| 409 | - | ||||
| 410 | // replayResizePending completes a pane's apply epoch. It keeps the gate set | 311 | // replayResizePending completes a pane's apply epoch. It keeps the gate set | ||
| 411 | // while each buffered batch goes through the normal VT path, so reads arriving | 312 | // while each buffered batch goes through the normal VT path, so reads arriving | ||
| 412 | // during replay join the same ordered stream. Failure deliberately omits the | 313 | // during replay join the same ordered stream. Failure deliberately omits the | ||
| @@ -469,7 +370,7 @@ func (d *Daemon) applyVisibleFloatingLayoutForMember(sess *session, tb *tab, cur | |||||
| 469 | return nil, true | 370 | return nil, true | ||
| 470 | } | 371 | } | ||
| 471 | 372 | ||||
| 472 | - | // Unlike applyResize, this keeps successful PTY resizes gated until the | 373 | + | // This keeps successful PTY resizes gated until the |
| 473 | // floating slot and tab size are revalidated. A newer client resize may | 374 | // floating slot and tab size are revalidated. A newer client resize may | ||
| 474 | // otherwise publish this obsolete popup geometry after its PTY call returns. | 375 | // otherwise publish this obsolete popup geometry after its PTY call returns. | ||
| 475 | plan := preparedTabLayout{members: []resizeMember{{ | 376 | plan := preparedTabLayout{members: []resizeMember{{ | ||
| @@ -554,6 +455,9 @@ func (d *Daemon) applySessionLayout(sess *session, size domain.Size, current, ad | |||||
| 554 | 455 | ||||
| 555 | target := tabSize(size) | 456 | target := tabSize(size) | ||
| 556 | for { | 457 | for { | ||
| 458 | + | if sess.ctx != nil && sess.ctx.Err() != nil { | |||
| 459 | + | return nil, false | |||
| 460 | + | } | |||
| 557 | if current != nil && !current() { | 461 | if current != nil && !current() { | ||
| 558 | return nil, false | 462 | return nil, false | ||
| 559 | } | 463 | } | ||
M internal/usecase/daemon/transactional_resize_test.go +32 -95
| @@ -181,6 +181,21 @@ func TestProcessPTYDataRetainsCallbacksDuringResizeReplay(t *testing.T) { | |||
| 181 | 181 | // invoke PTY.Resize after all session/tab/pane locks are released. This is the | |
| 182 | 182 | // deadlock boundary with PTY readers and scripts which synchronously re-enter | |
| 183 | 183 | // daemon state. | |
| 184 | + | func TestApplySessionLayoutStopsRetryWhenSessionCanceled(t *testing.T) { | |
| 185 | + | pty := &transactionalResizePTY{} | |
| 186 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |
| 187 | + | admissions := 0 | |
| 188 | + | d.beforeSessionResizePublication = sess.cancel | |
| 189 | + | ||
| 190 | + | _, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, func() bool { | |
| 191 | + | admissions++ | |
| 192 | + | return admissions > 1 | |
| 193 | + | }) | |
| 194 | + | ||
| 195 | + | require.False(t, ok) | |
| 196 | + | require.Len(t, pty.requested(), 1, "cancellation must stop before a stale retry applies another PTY resize") | |
| 197 | + | } | |
| 198 | + | ||
| 184 | 199 | func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | |
| 185 | 200 | pty := &transactionalResizePTY{} | |
| 186 | 201 | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |
| @@ -204,105 +219,27 @@ func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | |||
| 204 | 219 | require.True(t, paneUnlocked, "PTY.Resize must run outside pane.mu") | |
| 205 | 220 | } | |
| 206 | 221 | ||
| 207 | - | // P3 degradation: a failing PTY resize is surfaced as exactly one warn notice | |
| 208 | - | // per applyResize call, collected inside the resizeMu loop and reported once | |
| 209 | - | // after it returns to lock-free code. The count is the meaningful assertion: | |
| 210 | - | // notify acquires sess.mu/d.mu and must never run while any resizeMu is held. | |
| 211 | - | func TestTransactionalResizeApplyReportsFailureOncePerCall(t *testing.T) { | |
| 212 | - | countResizeFailed := func(d *Daemon) (int, domain.Notification) { | |
| 213 | - | var last domain.Notification | |
| 214 | - | count := 0 | |
| 215 | - | for _, h := range d.notices.history() { | |
| 216 | - | if h.Code == domain.NoticeResizeFailed { | |
| 217 | - | count++ | |
| 218 | - | last = h | |
| 219 | - | } | |
| 222 | + | // S3 acceptance: a failed member never publishes speculative parser/screen or | |
| 223 | + | // rectangle state. Successful members and the client layout commit together; | |
| 224 | + | // composition clips/pads the retained failed screen against that new layout. | |
| 225 | + | 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++ | |
| 220 | 238 | } | |
| 221 | - | return count, last | |
| 222 | - | } | |
| 223 | - | ||
| 224 | - | t.Run("one failed member yields one notice after return", func(t *testing.T) { | |
| 225 | - | failing := &transactionalResizePTY{errs: []error{errors.New("scripted resize failure")}} | |
| 226 | - | d, sess, _, _ := newManualSessionWithPTYs(t, failing) | |
| 227 | - | tb := sess.activeTab() | |
| 228 | - | p := tb.focusedPane() | |
| 229 | - | plan := resizePlan{members: []resizeMember{{session: sess, tab: tb, pane: p, rect: domain.Rect{Width: 100, Height: 20}, retry: true}}} | |
| 230 | - | ||
| 231 | - | require.Empty(t, d.notices.history(), "no notice is recorded before applyResize") | |
| 232 | - | require.True(t, d.applyResize(&plan)) | |
| 233 | - | ||
| 234 | - | count, n := countResizeFailed(d) | |
| 235 | - | require.Equal(t, 1, count, "exactly one NoticeResizeFailed per applyResize call") | |
| 236 | - | require.Equal(t, domain.NoticeWarn, n.Severity) | |
| 237 | - | require.Equal(t, "pane resize failed; retrying in background", n.Message) | |
| 238 | - | require.False(t, plan.members[0].ok, "failed member is not marked ok") | |
| 239 | - | }) | |
| 240 | - | ||
| 241 | - | for _, tt := range []struct { | |
| 242 | - | name string | |
| 243 | - | firstErr error | |
| 244 | - | secondErr error | |
| 245 | - | current func() bool | |
| 246 | - | wantResult bool | |
| 247 | - | wantCount int | |
| 248 | - | }{ | |
| 249 | - | { | |
| 250 | - | name: "normal completion reports one combined failure", | |
| 251 | - | firstErr: errors.New("first fails"), | |
| 252 | - | secondErr: errors.New("second fails"), | |
| 253 | - | wantResult: true, | |
| 254 | - | wantCount: 1, | |
| 255 | - | }, | |
| 256 | - | { | |
| 257 | - | name: "stale current after failure still reports once", | |
| 258 | - | firstErr: errors.New("first fails"), | |
| 259 | - | secondErr: errors.New("must not be resized"), | |
| 260 | - | current: func() func() bool { | |
| 261 | - | checks := 0 | |
| 262 | - | return func() bool { | |
| 263 | - | checks++ | |
| 264 | - | return checks == 1 | |
| 265 | - | } | |
| 266 | - | }(), | |
| 267 | - | wantResult: false, | |
| 268 | - | wantCount: 1, | |
| 269 | - | }, | |
| 270 | - | { | |
| 271 | - | name: "normal completion without failure does not report", | |
| 272 | - | wantResult: true, | |
| 273 | - | wantCount: 0, | |
| 274 | - | }, | |
| 275 | - | } { | |
| 276 | - | t.Run(tt.name, func(t *testing.T) { | |
| 277 | - | first := &transactionalResizePTY{errs: []error{tt.firstErr}} | |
| 278 | - | second := &transactionalResizePTY{errs: []error{tt.secondErr}} | |
| 279 | - | d, sess, _, _ := newManualSessionWithPTYs(t, first, second) | |
| 280 | - | tabs := sess.tabs | |
| 281 | - | plan := resizePlan{members: []resizeMember{ | |
| 282 | - | {session: sess, tab: tabs[0], pane: tabs[0].focusedPane(), rect: domain.Rect{Width: 100, Height: 20}, retry: true}, | |
| 283 | - | {session: sess, tab: tabs[1], pane: tabs[1].focusedPane(), rect: domain.Rect{Width: 100, Height: 20}, retry: true}, | |
| 284 | - | }} | |
| 285 | - | ||
| 286 | - | var got bool | |
| 287 | - | if tt.current == nil { | |
| 288 | - | got = d.applyResize(&plan) | |
| 289 | - | } else { | |
| 290 | - | got = d.applyResize(&plan, tt.current) | |
| 291 | - | } | |
| 292 | - | require.Equal(t, tt.wantResult, got) | |
| 293 | - | ||
| 294 | - | count, _ := countResizeFailed(d) | |
| 295 | - | require.Equal(t, tt.wantCount, count, "one notice per applyResize call, including stale returns") | |
| 296 | - | if !tt.wantResult { | |
| 297 | - | require.Equal(t, []domain.Size(nil), second.requested(), "stale current check must stop before the next member") | |
| 298 | - | } | |
| 299 | - | }) | |
| 300 | 239 | } | |
| 240 | + | require.Equal(t, 1, count) | |
| 301 | 241 | } | |
| 302 | 242 | ||
| 303 | - | // S3 acceptance: a failed member never publishes speculative parser/screen or | |
| 304 | - | // rectangle state. Successful members and the client layout commit together; | |
| 305 | - | // composition clips/pads the retained failed screen against that new layout. | |
| 306 | 243 | func TestTransactionalResizePartialFailureCommitsOnlySuccessfulPTYState(t *testing.T) { | |
| 307 | 244 | ok := &transactionalResizePTY{} | |
| 308 | 245 | failed := &transactionalResizePTY{errs: []error{errors.New("scripted resize failure")}} | |
| @@ -181,6 +181,21 @@ func TestProcessPTYDataRetainsCallbacksDuringResizeReplay(t *testing.T) { | |||||
| 181 | // invoke PTY.Resize after all session/tab/pane locks are released. This is the | 181 | // invoke PTY.Resize after all session/tab/pane locks are released. This is the | ||
| 182 | // deadlock boundary with PTY readers and scripts which synchronously re-enter | 182 | // deadlock boundary with PTY readers and scripts which synchronously re-enter | ||
| 183 | // daemon state. | 183 | // daemon state. | ||
| 184 | + | func TestApplySessionLayoutStopsRetryWhenSessionCanceled(t *testing.T) { | |||
| 185 | + | pty := &transactionalResizePTY{} | |||
| 186 | + | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | |||
| 187 | + | admissions := 0 | |||
| 188 | + | d.beforeSessionResizePublication = sess.cancel | |||
| 189 | + | ||||
| 190 | + | _, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, func() bool { | |||
| 191 | + | admissions++ | |||
| 192 | + | return admissions > 1 | |||
| 193 | + | }) | |||
| 194 | + | ||||
| 195 | + | require.False(t, ok) | |||
| 196 | + | require.Len(t, pty.requested(), 1, "cancellation must stop before a stale retry applies another PTY resize") | |||
| 197 | + | } | |||
| 198 | + | ||||
| 184 | func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | 199 | func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | ||
| 185 | pty := &transactionalResizePTY{} | 200 | pty := &transactionalResizePTY{} | ||
| 186 | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | 201 | d, sess, _, _ := newManualSessionWithPTYs(t, pty) | ||
| @@ -204,105 +219,27 @@ func TestTransactionalResizeApplyDoesNotHoldTabOrPaneLocks(t *testing.T) { | |||||
| 204 | require.True(t, paneUnlocked, "PTY.Resize must run outside pane.mu") | 219 | require.True(t, paneUnlocked, "PTY.Resize must run outside pane.mu") | ||
| 205 | } | 220 | } | ||
| 206 | 221 | ||||
| 207 | - | // P3 degradation: a failing PTY resize is surfaced as exactly one warn notice | 222 | + | // S3 acceptance: a failed member never publishes speculative parser/screen or |
| 208 | - | // per applyResize call, collected inside the resizeMu loop and reported once | 223 | + | // rectangle state. Successful members and the client layout commit together; |
| 209 | - | // after it returns to lock-free code. The count is the meaningful assertion: | 224 | + | // composition clips/pads the retained failed screen against that new layout. |
| 210 | - | // notify acquires sess.mu/d.mu and must never run while any resizeMu is held. | 225 | + | func TestApplySessionLayoutReportsResizeFailuresOnce(t *testing.T) { |
| 211 | - | func TestTransactionalResizeApplyReportsFailureOncePerCall(t *testing.T) { | 226 | + | first := &transactionalResizePTY{errs: []error{errors.New("first failure")}} |
| 212 | - | countResizeFailed := func(d *Daemon) (int, domain.Notification) { | 227 | + | second := &transactionalResizePTY{errs: []error{errors.New("second failure")}} |
| 213 | - | var last domain.Notification | 228 | + | d, sess, _, _ := newManualSessionWithPTYs(t, first, second) |
| 214 | - | count := 0 | 229 | + | |
| 215 | - | for _, h := range d.notices.history() { | 230 | + | failed, ok := d.applySessionLayout(sess, domain.Size{Cols: 100, Rows: 30}, nil, nil) |
| 216 | - | if h.Code == domain.NoticeResizeFailed { | 231 | + | |
| 217 | - | count++ | 232 | + | require.True(t, ok) |
| 218 | - | last = h | 233 | + | require.Len(t, failed, 2) |
| 219 | - | } | 234 | + | count := 0 |
| 235 | + | for _, notification := range d.notices.history() { | |||
| 236 | + | if notification.Code == domain.NoticeResizeFailed { | |||
| 237 | + | count++ | |||
| 220 | } | 238 | } | ||
| 221 | - | return count, last | |||
| 222 | - | } | |||
| 223 | - | ||||
| 224 | - | t.Run("one failed member yields one notice after return", func(t *testing.T) { | |||
| 225 | - | failing := &transactionalResizePTY{errs: []error{errors.New("scripted resize failure")}} | |||
| 226 | - | d, sess, _, _ := newManualSessionWithPTYs(t, failing) | |||
| 227 | - | tb := sess.activeTab() | |||
| 228 | - | p := tb.focusedPane() | |||
| 229 | - | plan := resizePlan{members: []resizeMember{{session: sess, tab: tb, pane: p, rect: domain.Rect{Width: 100, Height: 20}, retry: true}}} | |||
| 230 | - | ||||
| 231 | - | require.Empty(t, d.notices.history(), "no notice is recorded before applyResize") | |||
| 232 | - | require.True(t, d.applyResize(&plan)) | |||
| 233 | - | ||||
| 234 | - | count, n := countResizeFailed(d) | |||
| 235 | - | require.Equal(t, 1, count, "exactly one NoticeResizeFailed per applyResize call") | |||
| 236 | - | require.Equal(t, domain.NoticeWarn, n.Severity) | |||
| 237 | - | require.Equal(t, "pane resize failed; retrying in background", n.Message) | |||
| 238 | - | require.False(t, plan.members[0].ok, "failed member is not marked ok") | |||
| 239 | - | }) | |||
| 240 | - | ||||
| 241 | - | for _, tt := range []struct { | |||
| 242 | - | name string | |||
| 243 | - | firstErr error | |||
| 244 | - | secondErr error | |||
| 245 | - | current func() bool | |||
| 246 | - | wantResult bool | |||
| 247 | - | wantCount int | |||
| 248 | - | }{ | |||
| 249 | - | { | |||
| 250 | - | name: "normal completion reports one combined failure", | |||
| 251 | - | firstErr: errors.New("first fails"), | |||
| 252 | - | secondErr: errors.New("second fails"), | |||
| 253 | - | wantResult: true, | |||
| 254 | - | wantCount: 1, | |||
| 255 | - | }, | |||
| 256 | - | { | |||
| 257 | - | name: "stale current after failure still reports once", | |||
| 258 | - | firstErr: errors.New("first fails"), | |||
| 259 | - | secondErr: errors.New("must not be resized"), | |||
| 260 | - | current: func() func() bool { | |||
| 261 | - | checks := 0 | |||
| 262 | - | return func() bool { | |||
| 263 | - | checks++ | |||
| 264 | - | return checks == 1 | |||
| 265 | - | } | |||
| 266 | - | }(), | |||
| 267 | - | wantResult: false, | |||
| 268 | - | wantCount: 1, | |||
| 269 | - | }, | |||
| 270 | - | { | |||
| 271 | - | name: "normal completion without failure does not report", | |||
| 272 | - | wantResult: true, | |||
| 273 | - | wantCount: 0, | |||
| 274 | - | }, | |||
| 275 | - | } { | |||
| 276 | - | t.Run(tt.name, func(t *testing.T) { | |||
| 277 | - | first := &transactionalResizePTY{errs: []error{tt.firstErr}} | |||
| 278 | - | second := &transactionalResizePTY{errs: []error{tt.secondErr}} | |||
| 279 | - | d, sess, _, _ := newManualSessionWithPTYs(t, first, second) | |||
| 280 | - | tabs := sess.tabs | |||
| 281 | - | plan := resizePlan{members: []resizeMember{ | |||
| 282 | - | {session: sess, tab: tabs[0], pane: tabs[0].focusedPane(), rect: domain.Rect{Width: 100, Height: 20}, retry: true}, | |||
| 283 | - | {session: sess, tab: tabs[1], pane: tabs[1].focusedPane(), rect: domain.Rect{Width: 100, Height: 20}, retry: true}, | |||
| 284 | - | }} | |||
| 285 | - | ||||
| 286 | - | var got bool | |||
| 287 | - | if tt.current == nil { | |||
| 288 | - | got = d.applyResize(&plan) | |||
| 289 | - | } else { | |||
| 290 | - | got = d.applyResize(&plan, tt.current) | |||
| 291 | - | } | |||
| 292 | - | require.Equal(t, tt.wantResult, got) | |||
| 293 | - | ||||
| 294 | - | count, _ := countResizeFailed(d) | |||
| 295 | - | require.Equal(t, tt.wantCount, count, "one notice per applyResize call, including stale returns") | |||
| 296 | - | if !tt.wantResult { | |||
| 297 | - | require.Equal(t, []domain.Size(nil), second.requested(), "stale current check must stop before the next member") | |||
| 298 | - | } | |||
| 299 | - | }) | |||
| 300 | } | 239 | } | ||
| 240 | + | require.Equal(t, 1, count) | |||
| 301 | } | 241 | } | ||
| 302 | 242 | ||||
| 303 | - | // S3 acceptance: a failed member never publishes speculative parser/screen or | |||
| 304 | - | // rectangle state. Successful members and the client layout commit together; | |||
| 305 | - | // composition clips/pads the retained failed screen against that new layout. | |||
| 306 | func TestTransactionalResizePartialFailureCommitsOnlySuccessfulPTYState(t *testing.T) { | 243 | func TestTransactionalResizePartialFailureCommitsOnlySuccessfulPTYState(t *testing.T) { | ||
| 307 | ok := &transactionalResizePTY{} | 244 | ok := &transactionalResizePTY{} | ||
| 308 | failed := &transactionalResizePTY{errs: []error{errors.New("scripted resize failure")}} | 245 | failed := &transactionalResizePTY{errs: []error{errors.New("scripted resize failure")}} | ||
M internal/usecase/layout/ops.go +6 -4
| @@ -2,6 +2,7 @@ package layout | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "errors" | |
| 5 | + | "math" | |
| 5 | 6 | "sort" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/vev/internal/domain" | |
| @@ -55,10 +56,11 @@ func (t *Tree) ResizeFocus(axis Axis, delta int, area domain.Rect) error { | |||
| 55 | 56 | ||
| 56 | 57 | requested := delta | |
| 57 | 58 | if requested < 0 { | |
| 58 | - | // -(MinInt) overflows. Splitting the negation keeps the magnitude | |
| 59 | - | // representable even for a deliberately hostile caller. | |
| 60 | - | requested = -(requested + 1) | |
| 61 | - | requested++ | |
| 59 | + | if requested == math.MinInt { | |
| 60 | + | requested = math.MaxInt | |
| 61 | + | } else { | |
| 62 | + | requested = -requested | |
| 63 | + | } | |
| 62 | 64 | } | |
| 63 | 65 | if requested == 0 { | |
| 64 | 66 | return ErrTooSmall | |
| @@ -2,6 +2,7 @@ package layout | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "errors" | 4 | "errors" | ||
| 5 | + | "math" | |||
| 5 | "sort" | 6 | "sort" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/vev/internal/domain" | 8 | "github.com/bnema/vev/internal/domain" | ||
| @@ -55,10 +56,11 @@ func (t *Tree) ResizeFocus(axis Axis, delta int, area domain.Rect) error { | |||||
| 55 | 56 | ||||
| 56 | requested := delta | 57 | requested := delta | ||
| 57 | if requested < 0 { | 58 | if requested < 0 { | ||
| 58 | - | // -(MinInt) overflows. Splitting the negation keeps the magnitude | 59 | + | if requested == math.MinInt { |
| 59 | - | // representable even for a deliberately hostile caller. | 60 | + | requested = math.MaxInt |
| 60 | - | requested = -(requested + 1) | 61 | + | } else { |
| 61 | - | requested++ | 62 | + | requested = -requested |
| 63 | + | } | |||
| 62 | } | 64 | } | ||
| 63 | if requested == 0 { | 65 | if requested == 0 { | ||
| 64 | return ErrTooSmall | 66 | return ErrTooSmall | ||
M internal/usecase/layout/resize_test.go +6 -5
| @@ -289,14 +289,15 @@ func TestEqualizeFailureIsTransactional(t *testing.T) { | |||
| 289 | 289 | require.Equal(t, before, tr) | |
| 290 | 290 | } | |
| 291 | 291 | ||
| 292 | - | func TestResizeFocusMinIntDoesNotOverflow(t *testing.T) { | |
| 292 | + | func TestResizeFocusMinIntClampsWithoutOverflow(t *testing.T) { | |
| 293 | 293 | t.Parallel() | |
| 294 | 294 | ||
| 295 | - | tr := &Tree{Root: split(Horizontal, NewLeaf("a"), NewLeaf("b")), Focus: "a"} | |
| 296 | - | before := tr.Clone() | |
| 295 | + | tr := &Tree{Root: split(Horizontal, weightedLeaf("a", 30), weightedLeaf("b", 20)), Focus: "a"} | |
| 297 | 296 | minInt := -int(^uint(0)>>1) - 1 | |
| 298 | - | require.ErrorIs(t, tr.ResizeFocus(Width, minInt, domain.Rect{Width: 41, Height: 2}), ErrTooSmall) | |
| 299 | - | require.Equal(t, before, tr, "an unrepresentable shrink magnitude must remain transactional") | |
| 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)) | |
| 300 | 301 | } | |
| 301 | 302 | ||
| 302 | 303 | func TestCanResize(t *testing.T) { | |
| @@ -289,14 +289,15 @@ func TestEqualizeFailureIsTransactional(t *testing.T) { | |||||
| 289 | require.Equal(t, before, tr) | 289 | require.Equal(t, before, tr) | ||
| 290 | } | 290 | } | ||
| 291 | 291 | ||||
| 292 | - | func TestResizeFocusMinIntDoesNotOverflow(t *testing.T) { | 292 | + | func TestResizeFocusMinIntClampsWithoutOverflow(t *testing.T) { |
| 293 | t.Parallel() | 293 | t.Parallel() | ||
| 294 | 294 | ||||
| 295 | - | tr := &Tree{Root: split(Horizontal, NewLeaf("a"), NewLeaf("b")), Focus: "a"} | 295 | + | tr := &Tree{Root: split(Horizontal, weightedLeaf("a", 30), weightedLeaf("b", 20)), Focus: "a"} |
| 296 | - | before := tr.Clone() | |||
| 297 | minInt := -int(^uint(0)>>1) - 1 | 296 | minInt := -int(^uint(0)>>1) - 1 | ||
| 298 | - | require.ErrorIs(t, tr.ResizeFocus(Width, minInt, domain.Rect{Width: 41, Height: 2}), ErrTooSmall) | 297 | + | require.NoError(t, tr.ResizeFocus(Width, minInt, domain.Rect{Width: 51, Height: 2})) |
| 299 | - | require.Equal(t, before, tr, "an unrepresentable shrink magnitude must remain transactional") | 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)) | |||
| 300 | } | 301 | } | ||
| 301 | 302 | ||||
| 302 | func TestCanResize(t *testing.T) { | 303 | func TestCanResize(t *testing.T) { | ||
M internal/usecase/snapshot/manifest.go +13 -3
| @@ -345,6 +345,14 @@ func writeManifestWeights(w *payloadWriter, nodes []*layout.Node) error { | |||
| 345 | 345 | } | |
| 346 | 346 | ||
| 347 | 347 | func readManifestWeights(r *payloadReader, nodes []*layout.Node) error { | |
| 348 | + | return readManifestWeightExtension(r, uint64(len(nodes)), func(i int, weight float64) { | |
| 349 | + | nodes[i].Weight = weight | |
| 350 | + | }) | |
| 351 | + | } | |
| 352 | + | ||
| 353 | + | // readManifestWeightExtension validates the optional weight extension and | |
| 354 | + | // calls assign for each decoded weight when assignment is required. | |
| 355 | + | func readManifestWeightExtension(r *payloadReader, wantNodes uint64, assign func(int, float64)) error { | |
| 348 | 356 | if len(r.b) == 0 { | |
| 349 | 357 | return nil | |
| 350 | 358 | } | |
| @@ -359,7 +367,7 @@ func readManifestWeights(r *payloadReader, nodes []*layout.Node) error { | |||
| 359 | 367 | if err != nil { | |
| 360 | 368 | return err | |
| 361 | 369 | } | |
| 362 | - | if uint64(count) != uint64(len(nodes)) { | |
| 370 | + | if uint64(count) != wantNodes { | |
| 363 | 371 | return fmt.Errorf("%w: weight node count", ErrInvalidData) | |
| 364 | 372 | } | |
| 365 | 373 | if uint64(count) > uint64(len(r.b))/manifestWeightSize { | |
| @@ -368,7 +376,7 @@ func readManifestWeights(r *payloadReader, nodes []*layout.Node) error { | |||
| 368 | 376 | if uint64(len(r.b)) != uint64(count)*manifestWeightSize { | |
| 369 | 377 | return ErrTrailingBytes | |
| 370 | 378 | } | |
| 371 | - | for i := range nodes { | |
| 379 | + | for i := range count { | |
| 372 | 380 | bits, err := r.getUint64() | |
| 373 | 381 | if err != nil { | |
| 374 | 382 | return err | |
| @@ -377,7 +385,9 @@ func readManifestWeights(r *payloadReader, nodes []*layout.Node) error { | |||
| 377 | 385 | if !validManifestWeight(weight) { | |
| 378 | 386 | return fmt.Errorf("%w: node weight", ErrInvalidData) | |
| 379 | 387 | } | |
| 380 | - | nodes[i].Weight = weight | |
| 388 | + | if assign != nil { | |
| 389 | + | assign(int(i), weight) | |
| 390 | + | } | |
| 381 | 391 | } | |
| 382 | 392 | return r.done() | |
| 383 | 393 | } | |
| @@ -345,6 +345,14 @@ func writeManifestWeights(w *payloadWriter, nodes []*layout.Node) error { | |||||
| 345 | } | 345 | } | ||
| 346 | 346 | ||||
| 347 | func readManifestWeights(r *payloadReader, nodes []*layout.Node) error { | 347 | func readManifestWeights(r *payloadReader, nodes []*layout.Node) error { | ||
| 348 | + | return readManifestWeightExtension(r, uint64(len(nodes)), func(i int, weight float64) { | |||
| 349 | + | nodes[i].Weight = weight | |||
| 350 | + | }) | |||
| 351 | + | } | |||
| 352 | + | ||||
| 353 | + | // readManifestWeightExtension validates the optional weight extension and | |||
| 354 | + | // calls assign for each decoded weight when assignment is required. | |||
| 355 | + | func readManifestWeightExtension(r *payloadReader, wantNodes uint64, assign func(int, float64)) error { | |||
| 348 | if len(r.b) == 0 { | 356 | if len(r.b) == 0 { | ||
| 349 | return nil | 357 | return nil | ||
| 350 | } | 358 | } | ||
| @@ -359,7 +367,7 @@ func readManifestWeights(r *payloadReader, nodes []*layout.Node) error { | |||||
| 359 | if err != nil { | 367 | if err != nil { | ||
| 360 | return err | 368 | return err | ||
| 361 | } | 369 | } | ||
| 362 | - | if uint64(count) != uint64(len(nodes)) { | 370 | + | if uint64(count) != wantNodes { |
| 363 | return fmt.Errorf("%w: weight node count", ErrInvalidData) | 371 | return fmt.Errorf("%w: weight node count", ErrInvalidData) | ||
| 364 | } | 372 | } | ||
| 365 | if uint64(count) > uint64(len(r.b))/manifestWeightSize { | 373 | if uint64(count) > uint64(len(r.b))/manifestWeightSize { | ||
| @@ -368,7 +376,7 @@ func readManifestWeights(r *payloadReader, nodes []*layout.Node) error { | |||||
| 368 | if uint64(len(r.b)) != uint64(count)*manifestWeightSize { | 376 | if uint64(len(r.b)) != uint64(count)*manifestWeightSize { | ||
| 369 | return ErrTrailingBytes | 377 | return ErrTrailingBytes | ||
| 370 | } | 378 | } | ||
| 371 | - | for i := range nodes { | 379 | + | for i := range count { |
| 372 | bits, err := r.getUint64() | 380 | bits, err := r.getUint64() | ||
| 373 | if err != nil { | 381 | if err != nil { | ||
| 374 | return err | 382 | return err | ||
| @@ -377,7 +385,9 @@ func readManifestWeights(r *payloadReader, nodes []*layout.Node) error { | |||||
| 377 | if !validManifestWeight(weight) { | 385 | if !validManifestWeight(weight) { | ||
| 378 | return fmt.Errorf("%w: node weight", ErrInvalidData) | 386 | return fmt.Errorf("%w: node weight", ErrInvalidData) | ||
| 379 | } | 387 | } | ||
| 380 | - | nodes[i].Weight = weight | 388 | + | if assign != nil { |
| 389 | + | assign(int(i), weight) | |||
| 390 | + | } | |||
| 381 | } | 391 | } | ||
| 382 | return r.done() | 392 | return r.done() | ||
| 383 | } | 393 | } | ||
M internal/usecase/snapshot/manifest_preflight.go +1 -34
| @@ -2,7 +2,6 @@ package snapshot | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "fmt" | |
| 5 | - | "math" | |
| 6 | 5 | ||
| 7 | 6 | "github.com/bnema/vev/internal/usecase/layout" | |
| 8 | 7 | ) | |
| @@ -206,39 +205,7 @@ func preflightManifestNode(r *payloadReader, depth int, root bool, budget *manif | |||
| 206 | 205 | } | |
| 207 | 206 | ||
| 208 | 207 | func preflightManifestWeights(r *payloadReader, wantNodes uint32) error { | |
| 209 | - | if len(r.b) == 0 { | |
| 210 | - | return nil | |
| 211 | - | } | |
| 212 | - | if len(r.b) < len(manifestWeightTag) { | |
| 213 | - | return ErrShortPayload | |
| 214 | - | } | |
| 215 | - | if string(r.b[:len(manifestWeightTag)]) != manifestWeightTag { | |
| 216 | - | return fmt.Errorf("%w: weight extension tag", ErrInvalidData) | |
| 217 | - | } | |
| 218 | - | r.b = r.b[len(manifestWeightTag):] | |
| 219 | - | count, err := r.getUint32() | |
| 220 | - | if err != nil { | |
| 221 | - | return err | |
| 222 | - | } | |
| 223 | - | if count != wantNodes { | |
| 224 | - | return fmt.Errorf("%w: weight node count", ErrInvalidData) | |
| 225 | - | } | |
| 226 | - | if uint64(count) > uint64(len(r.b))/manifestWeightSize { | |
| 227 | - | return ErrShortPayload | |
| 228 | - | } | |
| 229 | - | if uint64(len(r.b)) != uint64(count)*manifestWeightSize { | |
| 230 | - | return ErrTrailingBytes | |
| 231 | - | } | |
| 232 | - | for range count { | |
| 233 | - | bits, err := r.getUint64() | |
| 234 | - | if err != nil { | |
| 235 | - | return err | |
| 236 | - | } | |
| 237 | - | if !validManifestWeight(math.Float64frombits(bits)) { | |
| 238 | - | return fmt.Errorf("%w: node weight", ErrInvalidData) | |
| 239 | - | } | |
| 240 | - | } | |
| 241 | - | return r.done() | |
| 208 | + | return readManifestWeightExtension(r, uint64(wantNodes), nil) | |
| 242 | 209 | } | |
| 243 | 210 | ||
| 244 | 211 | func preflightManifestPane(r *payloadReader, budget *manifestPreflightBudget) (layout.PaneID, error) { | |
| @@ -2,7 +2,6 @@ package snapshot | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "fmt" | 4 | "fmt" | ||
| 5 | - | "math" | |||
| 6 | 5 | ||||
| 7 | "github.com/bnema/vev/internal/usecase/layout" | 6 | "github.com/bnema/vev/internal/usecase/layout" | ||
| 8 | ) | 7 | ) | ||
| @@ -206,39 +205,7 @@ func preflightManifestNode(r *payloadReader, depth int, root bool, budget *manif | |||||
| 206 | } | 205 | } | ||
| 207 | 206 | ||||
| 208 | func preflightManifestWeights(r *payloadReader, wantNodes uint32) error { | 207 | func preflightManifestWeights(r *payloadReader, wantNodes uint32) error { | ||
| 209 | - | if len(r.b) == 0 { | 208 | + | return readManifestWeightExtension(r, uint64(wantNodes), nil) |
| 210 | - | return nil | |||
| 211 | - | } | |||
| 212 | - | if len(r.b) < len(manifestWeightTag) { | |||
| 213 | - | return ErrShortPayload | |||
| 214 | - | } | |||
| 215 | - | if string(r.b[:len(manifestWeightTag)]) != manifestWeightTag { | |||
| 216 | - | return fmt.Errorf("%w: weight extension tag", ErrInvalidData) | |||
| 217 | - | } | |||
| 218 | - | r.b = r.b[len(manifestWeightTag):] | |||
| 219 | - | count, err := r.getUint32() | |||
| 220 | - | if err != nil { | |||
| 221 | - | return err | |||
| 222 | - | } | |||
| 223 | - | if count != wantNodes { | |||
| 224 | - | return fmt.Errorf("%w: weight node count", ErrInvalidData) | |||
| 225 | - | } | |||
| 226 | - | if uint64(count) > uint64(len(r.b))/manifestWeightSize { | |||
| 227 | - | return ErrShortPayload | |||
| 228 | - | } | |||
| 229 | - | if uint64(len(r.b)) != uint64(count)*manifestWeightSize { | |||
| 230 | - | return ErrTrailingBytes | |||
| 231 | - | } | |||
| 232 | - | for range count { | |||
| 233 | - | bits, err := r.getUint64() | |||
| 234 | - | if err != nil { | |||
| 235 | - | return err | |||
| 236 | - | } | |||
| 237 | - | if !validManifestWeight(math.Float64frombits(bits)) { | |||
| 238 | - | return fmt.Errorf("%w: node weight", ErrInvalidData) | |||
| 239 | - | } | |||
| 240 | - | } | |||
| 241 | - | return r.done() | |||
| 242 | } | 209 | } | ||
| 243 | 210 | ||||
| 244 | func preflightManifestPane(r *payloadReader, budget *manifestPreflightBudget) (layout.PaneID, error) { | 211 | func preflightManifestPane(r *payloadReader, budget *manifestPreflightBudget) (layout.PaneID, error) { | ||