detached c11de37

fix(daemon): refresh geometry before delayed resize retry

commit
c11de378a44f2f84f4866568316ff44a6c7b7bbe
author
brice · · 2026-07-25 05:22 UTC
parent
10f16b2
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
2 files · +123 -20
Split
UnifiedSplit
M internal/usecase/daemon/pane_actions_test.go +60 -0
@@ -111,6 +111,66 @@ func TestLayoutApplicationRetriesOnlyAcceptedFailedGeometry(t *testing.T) {
111111 }, time.Second, time.Millisecond, "successful retry did not publish its VT size")
112112 }
113113
114+func TestDelayedRetryUsesFreshLayoutAfterPaneResize(t *testing.T) {
115+ clock := newCoordinatorMockClock(t, 8)
116+ // The failed apply also posts its preserved degradation warning; its toast
117+ // lifetime is unrelated to the coordinator deadlines driven below.
118+ clock.clock.EXPECT().NewTimer(6 * time.Second).Return(stubTimer{}).Once()
119+ failed := &transactionalResizePTY{errs: []error{errors.New("first resize fails")}}
120+ d, sess, ac, _ := newManualSessionWithPTYs(t, failed)
121+ d.clock = clock.clock
122+ tb := sess.activeTab()
123+ first := tb.focusedPane()
124+ second := newPane("pane-2", nil, domain.Size{Cols: 80, Rows: 23})
125+ tb.mu.Lock()
126+ tb.tree = &layout.Tree{Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf(first.id), layout.NewLeaf(second.id)}}, Focus: first.id}
127+ tb.panes[second.id] = second
128+ tb.bumpLayoutGenerationLocked()
129+ tb.mu.Unlock()
130+
131+ // A client resize commits the new layout but leaves the failed PTY on the
132+ // coordinator retry lane. Drive both deadlines explicitly so this ordering
133+ // cannot depend on wall-clock scheduling.
134+ d.resize(sess, ac, domain.Size{Cols: 80, Rows: 24})
135+ rc := sess.renderCoordinator()
136+ resizeTimer := awaitCoordinatorScheduledTimer(t, clock)
137+ resizeDone := captureResizeCallbackDone(t, rc)
138+ resizeTimer.ch <- time.Time{}
139+ awaitTestCompletion(t, resizeDone, "failed client resize did not complete")
140+ var retryTimer *coordinatorMockTimer
141+ for retryTimer == nil {
142+ candidate := awaitCoordinatorScheduledTimer(t, clock)
143+ if candidate.duration == minOutputRenderDeadline {
144+ retryTimer = candidate
145+ }
146+ }
147+
148+ // A later pane-resize changes the solved rectangle and repairs the failed
149+ // PTY before the delayed callback fires. Retrying its old captured 40-column
150+ // rectangle would regress the PTY, VT, and published pane geometry.
151+ require.NoError(t, d.resizePane(daemonActionTarget{session: sess, tab: tb, pane: first}, layout.Width, resizeStepCols))
152+ tb.mu.Lock()
153+ placements, ok := layout.Solve(tb.tree.Root, domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows})
154+ currentFirst := placementContent(placements, first.id)
155+ currentSecond := placementContent(placements, second.id)
156+ tb.mu.Unlock()
157+ require.True(t, ok)
158+ require.Equal(t, currentFirst, first.rect)
159+ require.Equal(t, currentSecond, second.rect)
160+ require.Equal(t, rectSize(currentFirst), domain.Size{Cols: first.screen.Frame.Width, Rows: first.screen.Frame.Height})
161+ require.Equal(t, []domain.Size{{Cols: 40, Rows: 22}, rectSize(currentFirst)}, failed.requested())
162+
163+ rc.mu.Lock()
164+ retryDone := rc.retryLane.token.done
165+ rc.mu.Unlock()
166+ retryTimer.ch <- time.Time{}
167+ awaitTestCompletion(t, retryDone, "delayed resize retry did not complete")
168+
169+ require.Equal(t, []domain.Size{{Cols: 40, Rows: 22}, rectSize(currentFirst)}, failed.requested(), "delayed retry must not apply stale PTY geometry")
170+ require.Equal(t, currentFirst, first.rect, "delayed retry must not publish a stale rectangle")
171+ require.Equal(t, rectSize(currentFirst), domain.Size{Cols: first.screen.Frame.Width, Rows: first.screen.Frame.Height}, "delayed retry must not publish a stale VT size")
172+}
173+
114174 func TestLayoutApplicationDoesNotHoldTabLock(t *testing.T) {
115175 d, sess, pty, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10})
116176 tb := sess.activeTab()
@@ -111,6 +111,66 @@ func TestLayoutApplicationRetriesOnlyAcceptedFailedGeometry(t *testing.T) {
111 }, time.Second, time.Millisecond, "successful retry did not publish its VT size")111 }, time.Second, time.Millisecond, "successful retry did not publish its VT size")
112 }112 }
113 113
114+func TestDelayedRetryUsesFreshLayoutAfterPaneResize(t *testing.T) {
115+ clock := newCoordinatorMockClock(t, 8)
116+ // The failed apply also posts its preserved degradation warning; its toast
117+ // lifetime is unrelated to the coordinator deadlines driven below.
118+ clock.clock.EXPECT().NewTimer(6 * time.Second).Return(stubTimer{}).Once()
119+ failed := &transactionalResizePTY{errs: []error{errors.New("first resize fails")}}
120+ d, sess, ac, _ := newManualSessionWithPTYs(t, failed)
121+ d.clock = clock.clock
122+ tb := sess.activeTab()
123+ first := tb.focusedPane()
124+ second := newPane("pane-2", nil, domain.Size{Cols: 80, Rows: 23})
125+ tb.mu.Lock()
126+ tb.tree = &layout.Tree{Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf(first.id), layout.NewLeaf(second.id)}}, Focus: first.id}
127+ tb.panes[second.id] = second
128+ tb.bumpLayoutGenerationLocked()
129+ tb.mu.Unlock()
130+
131+ // A client resize commits the new layout but leaves the failed PTY on the
132+ // coordinator retry lane. Drive both deadlines explicitly so this ordering
133+ // cannot depend on wall-clock scheduling.
134+ d.resize(sess, ac, domain.Size{Cols: 80, Rows: 24})
135+ rc := sess.renderCoordinator()
136+ resizeTimer := awaitCoordinatorScheduledTimer(t, clock)
137+ resizeDone := captureResizeCallbackDone(t, rc)
138+ resizeTimer.ch <- time.Time{}
139+ awaitTestCompletion(t, resizeDone, "failed client resize did not complete")
140+ var retryTimer *coordinatorMockTimer
141+ for retryTimer == nil {
142+ candidate := awaitCoordinatorScheduledTimer(t, clock)
143+ if candidate.duration == minOutputRenderDeadline {
144+ retryTimer = candidate
145+ }
146+ }
147+
148+ // A later pane-resize changes the solved rectangle and repairs the failed
149+ // PTY before the delayed callback fires. Retrying its old captured 40-column
150+ // rectangle would regress the PTY, VT, and published pane geometry.
151+ require.NoError(t, d.resizePane(daemonActionTarget{session: sess, tab: tb, pane: first}, layout.Width, resizeStepCols))
152+ tb.mu.Lock()
153+ placements, ok := layout.Solve(tb.tree.Root, domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows})
154+ currentFirst := placementContent(placements, first.id)
155+ currentSecond := placementContent(placements, second.id)
156+ tb.mu.Unlock()
157+ require.True(t, ok)
158+ require.Equal(t, currentFirst, first.rect)
159+ require.Equal(t, currentSecond, second.rect)
160+ require.Equal(t, rectSize(currentFirst), domain.Size{Cols: first.screen.Frame.Width, Rows: first.screen.Frame.Height})
161+ require.Equal(t, []domain.Size{{Cols: 40, Rows: 22}, rectSize(currentFirst)}, failed.requested())
162+
163+ rc.mu.Lock()
164+ retryDone := rc.retryLane.token.done
165+ rc.mu.Unlock()
166+ retryTimer.ch <- time.Time{}
167+ awaitTestCompletion(t, retryDone, "delayed resize retry did not complete")
168+
169+ require.Equal(t, []domain.Size{{Cols: 40, Rows: 22}, rectSize(currentFirst)}, failed.requested(), "delayed retry must not apply stale PTY geometry")
170+ require.Equal(t, currentFirst, first.rect, "delayed retry must not publish a stale rectangle")
171+ require.Equal(t, rectSize(currentFirst), domain.Size{Cols: first.screen.Frame.Width, Rows: first.screen.Frame.Height}, "delayed retry must not publish a stale VT size")
172+}
173+
114 func TestLayoutApplicationDoesNotHoldTabLock(t *testing.T) {174 func TestLayoutApplicationDoesNotHoldTabLock(t *testing.T) {
115 d, sess, pty, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10})175 d, sess, pty, _ := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10})
116 tb := sess.activeTab()176 tb := sess.activeTab()
M internal/usecase/daemon/transactional_resize.go +63 -20
@@ -551,36 +551,79 @@ func (d *Daemon) runResizeTransaction(sess *session, ac *attachedClient, lease *
551551 return true
552552 }
553553
554-// retryResizeMembers retries only members which failed the committed epoch.
555-// It never republishes geometry: an intervening resize owns that publication.
554+// retryResizeMembers retries failed committed members through a freshly
555+// prepared tab transaction. The captured members identify retry candidates
556+// only: their rectangles must never cross this delayed boundary, because any
557+// later layout mutation may have changed their geometry or removed them.
556558 func (d *Daemon) retryResizeMembers(sess *session, ac *attachedClient, lease *attachmentLease, epoch uint64, members []resizeMember) {
557559 rc := sess.renderCoordinator()
558560 if rc == nil || !rc.retryCurrentForLease(epoch, ac, lease) {
559561 return
560562 }
561- plan := resizePlan{members: append([]resizeMember(nil), members...)}
562- for i := range plan.members {
563- plan.members[i].retry = true
564- }
565- if !d.applyResize(&plan, func() bool { return rc.retryCurrentForLease(epoch, ac, lease) }) || !rc.retryCurrentForLease(epoch, ac, lease) {
566- return
563+
564+ // Retain tab order for deterministic external PTY ordering while collapsing
565+ // several failed members from the same tab into one canonical transaction.
566+ tabs := make([]*tab, 0, len(members))
567+ seen := make(map[*tab]struct{}, len(members))
568+ for _, member := range members {
569+ if member.tab == nil {
570+ continue
571+ }
572+ if _, ok := seen[member.tab]; ok {
573+ continue
574+ }
575+ seen[member.tab] = struct{}{}
576+ tabs = append(tabs, member.tab)
567577 }
568- failed := make([]resizeMember, 0, len(plan.members))
578+
579+ failed := make([]resizeMember, 0, len(members))
569580 succeeded := false
570- for _, member := range plan.members {
571- if !member.ok {
572- failed = append(failed, member)
581+ for _, tb := range tabs {
582+ if !rc.retryCurrentForLease(epoch, ac, lease) {
583+ return
584+ }
585+ // A later accepted layout transaction clears resizeRetry. In that case
586+ // this delayed callback has no remaining work and, importantly, must not
587+ // replay the obsolete rectangle it captured before the mutation.
588+ retryPending := false
589+ tb.mu.Lock()
590+ for _, member := range members {
591+ if member.tab != tb || member.pane == nil || tb.panes[member.pane.id] != member.pane {
592+ continue
593+ }
594+ member.pane.mu.Lock()
595+ retryPending = retryPending || member.pane.resizeRetry
596+ member.pane.mu.Unlock()
597+ }
598+ tb.mu.Unlock()
599+ if !retryPending {
573600 continue
574601 }
575- member.pane.mu.Lock()
576- // The rect is the already committed layout target. A successful apply
577- // already resized before replaying buffered bytes; PTY-less members still
578- // need their VT resized here. Force a reset below in either case.
579- if !member.screenResized {
580- member.pane.screen.Resize(member.rect.Width, member.rect.Height)
602+
603+ // applyTabLayoutTransaction captures the current generation, pane
604+ // pointers, and solved rectangles, and validates them again before any
605+ // VT/rectangle publication. It also preserves the resize gate and
606+ // degradation notice behavior for another failed external attempt.
607+ freshFailed, ok := d.applyTabLayoutTransaction(sess, tb, func() bool {
608+ return rc.retryCurrentForLease(epoch, ac, lease)
609+ })
610+ if !ok || !rc.retryCurrentForLease(epoch, ac, lease) {
611+ return
581612 }
582- member.pane.mu.Unlock()
583- succeeded = true
613+ failed = append(failed, freshFailed...)
614+ // A retry success is a formerly failed, still-current target whose
615+ // canonical apply cleared its retry bit. A collapsed or removed target
616+ // is neither a retry completion nor a reason to publish a reset.
617+ tb.mu.Lock()
618+ for _, member := range members {
619+ if member.tab != tb || member.pane == nil || tb.panes[member.pane.id] != member.pane {
620+ continue
621+ }
622+ member.pane.mu.Lock()
623+ succeeded = succeeded || !member.pane.resizeRetry
624+ member.pane.mu.Unlock()
625+ }
626+ tb.mu.Unlock()
584627 }
585628 if len(failed) != 0 {
586629 rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { d.retryResizeMembers(sess, ac, lease, epoch, failed) })
@@ -551,36 +551,79 @@ func (d *Daemon) runResizeTransaction(sess *session, ac *attachedClient, lease *
551 return true551 return true
552 }552 }
553 553
554-// retryResizeMembers retries only members which failed the committed epoch.554+// retryResizeMembers retries failed committed members through a freshly
555-// It never republishes geometry: an intervening resize owns that publication.555+// prepared tab transaction. The captured members identify retry candidates
556+// only: their rectangles must never cross this delayed boundary, because any
557+// later layout mutation may have changed their geometry or removed them.
556 func (d *Daemon) retryResizeMembers(sess *session, ac *attachedClient, lease *attachmentLease, epoch uint64, members []resizeMember) {558 func (d *Daemon) retryResizeMembers(sess *session, ac *attachedClient, lease *attachmentLease, epoch uint64, members []resizeMember) {
557 rc := sess.renderCoordinator()559 rc := sess.renderCoordinator()
558 if rc == nil || !rc.retryCurrentForLease(epoch, ac, lease) {560 if rc == nil || !rc.retryCurrentForLease(epoch, ac, lease) {
559 return561 return
560 }562 }
561- plan := resizePlan{members: append([]resizeMember(nil), members...)}563+
562- for i := range plan.members {564+ // Retain tab order for deterministic external PTY ordering while collapsing
563- plan.members[i].retry = true565+ // several failed members from the same tab into one canonical transaction.
564- }566+ tabs := make([]*tab, 0, len(members))
565- if !d.applyResize(&plan, func() bool { return rc.retryCurrentForLease(epoch, ac, lease) }) || !rc.retryCurrentForLease(epoch, ac, lease) {567+ seen := make(map[*tab]struct{}, len(members))
566- return568+ for _, member := range members {
569+ if member.tab == nil {
570+ continue
571+ }
572+ if _, ok := seen[member.tab]; ok {
573+ continue
574+ }
575+ seen[member.tab] = struct{}{}
576+ tabs = append(tabs, member.tab)
567 }577 }
568- failed := make([]resizeMember, 0, len(plan.members))578+
579+ failed := make([]resizeMember, 0, len(members))
569 succeeded := false580 succeeded := false
570- for _, member := range plan.members {581+ for _, tb := range tabs {
571- if !member.ok {582+ if !rc.retryCurrentForLease(epoch, ac, lease) {
572- failed = append(failed, member)583+ return
584+ }
585+ // A later accepted layout transaction clears resizeRetry. In that case
586+ // this delayed callback has no remaining work and, importantly, must not
587+ // replay the obsolete rectangle it captured before the mutation.
588+ retryPending := false
589+ tb.mu.Lock()
590+ for _, member := range members {
591+ if member.tab != tb || member.pane == nil || tb.panes[member.pane.id] != member.pane {
592+ continue
593+ }
594+ member.pane.mu.Lock()
595+ retryPending = retryPending || member.pane.resizeRetry
596+ member.pane.mu.Unlock()
597+ }
598+ tb.mu.Unlock()
599+ if !retryPending {
573 continue600 continue
574 }601 }
575- member.pane.mu.Lock()602+
576- // The rect is the already committed layout target. A successful apply603+ // applyTabLayoutTransaction captures the current generation, pane
577- // already resized before replaying buffered bytes; PTY-less members still604+ // pointers, and solved rectangles, and validates them again before any
578- // need their VT resized here. Force a reset below in either case.605+ // VT/rectangle publication. It also preserves the resize gate and
579- if !member.screenResized {606+ // degradation notice behavior for another failed external attempt.
580- member.pane.screen.Resize(member.rect.Width, member.rect.Height)607+ freshFailed, ok := d.applyTabLayoutTransaction(sess, tb, func() bool {
608+ return rc.retryCurrentForLease(epoch, ac, lease)
609+ })
610+ if !ok || !rc.retryCurrentForLease(epoch, ac, lease) {
611+ return
581 }612 }
582- member.pane.mu.Unlock()613+ failed = append(failed, freshFailed...)
583- succeeded = true614+ // A retry success is a formerly failed, still-current target whose
615+ // canonical apply cleared its retry bit. A collapsed or removed target
616+ // is neither a retry completion nor a reason to publish a reset.
617+ tb.mu.Lock()
618+ for _, member := range members {
619+ if member.tab != tb || member.pane == nil || tb.panes[member.pane.id] != member.pane {
620+ continue
621+ }
622+ member.pane.mu.Lock()
623+ succeeded = succeeded || !member.pane.resizeRetry
624+ member.pane.mu.Unlock()
625+ }
626+ tb.mu.Unlock()
584 }627 }
585 if len(failed) != 0 {628 if len(failed) != 0 {
586 rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { d.retryResizeMembers(sess, ac, lease, epoch, failed) })629 rc.scheduleResizeRetryForLease(epoch, ac, lease, func() { d.retryResizeMembers(sess, ac, lease, epoch, failed) })

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close