detached e15d552

refactor(daemon): remove the test-only render ackReady gate

The render coordinator now probes output readiness only per attachment
through ackReadyFor. Without attachment leases nothing gates output, which
matches production where ackReady was never set.
commit
e15d552f697222749ef14384178ea60178105985
author
brice · · 2026-10-08 05:04 UTC
parent
6fb053b
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +32 -47
M internal/usecase/daemon/render_coordinator.go +10 -24
@@ -991,9 +991,6 @@ func (c *renderCoordinator) attachmentAckReady(ac *attachedClient) bool {
991991 if c.opts.ackReadyFor != nil {
992992 return c.opts.ackReadyFor(ac)
993993 }
994- if c.opts.ackReady != nil {
995- return c.opts.ackReady()
996- }
997994 return true
998995 }
999996
@@ -1014,25 +1011,15 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui
10141011 leases[ac] = lease
10151012 }
10161013 }
1017- previewOnly := len(leases) == 0 && len(c.previewWakes) != 0
1018- legacyAckReady := c.opts.ackReady
10191014 c.mu.Unlock()
10201015
10211016 ready := make(map[*attachmentLease]bool, len(leases))
1022- if len(leases) != 0 {
1023- for ac, lease := range leases {
1024- // Welcome holds attachment.sendMu while its transport Send is in
1025- // flight. Do not probe an incarnation before it is ready.
1026- if lease.ready {
1027- ready[lease] = c.attachmentAckReady(ac)
1028- }
1017+ for ac, lease := range leases {
1018+ // Welcome holds attachment.sendMu while its transport Send is in
1019+ // flight. Do not probe an incarnation before it is ready.
1020+ if lease.ready {
1021+ ready[lease] = c.attachmentAckReady(ac)
10291022 }
1030- } else if !previewOnly && legacyAckReady != nil {
1031- // Keep coordinator-only/headless callers compatible; production always
1032- // has attachment leases and uses the per-attachment callback above.
1033- ready[nil] = legacyAckReady()
1034- } else {
1035- ready[nil] = true
10361023 }
10371024
10381025 // The readiness callback can detach, replace, or publish a sync batch.
@@ -1098,12 +1085,11 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui
10981085 }
10991086 c.ackDeferredFor[ac] = true
11001087 }
1101- } else if !ready[nil] {
1102- blockedByAck = true
1103- allDelivered = false
11041088 }
1105- legacyOutput := len(leases) == 0 && ready[nil] && (c.pendingShared || len(c.pendingTargets) == 0)
1106- outputReady := len(attachmentLeases) != 0 || legacyOutput
1089+ // Without any attachment lease (headless sessions and coordinator-only
1090+ // callers) nothing gates output, so a shared invalidation renders directly.
1091+ leaselessOutput := len(leases) == 0 && (c.pendingShared || len(c.pendingTargets) == 0)
1092+ outputReady := len(attachmentLeases) != 0 || leaselessOutput
11071093 if len(leases) != 0 {
11081094 for ac, lease := range leases {
11091095 if !c.pendingShared && len(c.pendingTargets) != 0 {
@@ -1118,7 +1104,7 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui
11181104 }
11191105 }
11201106 }
1121- if len(leases) == 0 && legacyOutput {
1107+ if leaselessOutput {
11221108 allDelivered = true
11231109 }
11241110 wakeGeneration := uint64(0)
@@ -991,9 +991,6 @@ func (c *renderCoordinator) attachmentAckReady(ac *attachedClient) bool {
991 if c.opts.ackReadyFor != nil {991 if c.opts.ackReadyFor != nil {
992 return c.opts.ackReadyFor(ac)992 return c.opts.ackReadyFor(ac)
993 }993 }
994- if c.opts.ackReady != nil {
995- return c.opts.ackReady()
996- }
997 return true994 return true
998 }995 }
999 996
@@ -1014,25 +1011,15 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui
1014 leases[ac] = lease1011 leases[ac] = lease
1015 }1012 }
1016 }1013 }
1017- previewOnly := len(leases) == 0 && len(c.previewWakes) != 0
1018- legacyAckReady := c.opts.ackReady
1019 c.mu.Unlock()1014 c.mu.Unlock()
1020 1015
1021 ready := make(map[*attachmentLease]bool, len(leases))1016 ready := make(map[*attachmentLease]bool, len(leases))
1022- if len(leases) != 0 {1017+ for ac, lease := range leases {
1023- for ac, lease := range leases {1018+ // Welcome holds attachment.sendMu while its transport Send is in
1024- // Welcome holds attachment.sendMu while its transport Send is in1019+ // flight. Do not probe an incarnation before it is ready.
1025- // flight. Do not probe an incarnation before it is ready.1020+ if lease.ready {
1026- if lease.ready {1021+ ready[lease] = c.attachmentAckReady(ac)
1027- ready[lease] = c.attachmentAckReady(ac)
1028- }
1029 }1022 }
1030- } else if !previewOnly && legacyAckReady != nil {
1031- // Keep coordinator-only/headless callers compatible; production always
1032- // has attachment leases and uses the per-attachment callback above.
1033- ready[nil] = legacyAckReady()
1034- } else {
1035- ready[nil] = true
1036 }1023 }
1037 1024
1038 // The readiness callback can detach, replace, or publish a sync batch.1025 // The readiness callback can detach, replace, or publish a sync batch.
@@ -1098,12 +1085,11 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui
1098 }1085 }
1099 c.ackDeferredFor[ac] = true1086 c.ackDeferredFor[ac] = true
1100 }1087 }
1101- } else if !ready[nil] {
1102- blockedByAck = true
1103- allDelivered = false
1104 }1088 }
1105- legacyOutput := len(leases) == 0 && ready[nil] && (c.pendingShared || len(c.pendingTargets) == 0)1089+ // Without any attachment lease (headless sessions and coordinator-only
1106- outputReady := len(attachmentLeases) != 0 || legacyOutput1090+ // callers) nothing gates output, so a shared invalidation renders directly.
1091+ leaselessOutput := len(leases) == 0 && (c.pendingShared || len(c.pendingTargets) == 0)
1092+ outputReady := len(attachmentLeases) != 0 || leaselessOutput
1107 if len(leases) != 0 {1093 if len(leases) != 0 {
1108 for ac, lease := range leases {1094 for ac, lease := range leases {
1109 if !c.pendingShared && len(c.pendingTargets) != 0 {1095 if !c.pendingShared && len(c.pendingTargets) != 0 {
@@ -1118,7 +1104,7 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui
1118 }1104 }
1119 }1105 }
1120 }1106 }
1121- if len(leases) == 0 && legacyOutput {1107+ if leaselessOutput {
1122 allDelivered = true1108 allDelivered = true
1123 }1109 }
1124 wakeGeneration := uint64(0)1110 wakeGeneration := uint64(0)
M internal/usecase/daemon/render_coordinator_test.go +9 -9
@@ -97,7 +97,7 @@ func TestRenderCoordinatorDetachDoesNotJoinSelectedDeadlineWorker(t *testing.T)
9797 release := make(chan struct{})
9898 rc := newRenderCoordinator(renderCoordinatorOptions{
9999 clock: clock,
100- ackReady: func() bool {
100+ ackReadyFor: func(*attachedClient) bool {
101101 close(entered)
102102 <-release
103103 return true
@@ -166,10 +166,10 @@ func newCoordinatorHarness(t *testing.T) *coordinatorHarness {
166166 }
167167 h.ackReady.Store(true)
168168 h.rc = newRenderCoordinator(renderCoordinatorOptions{
169- clock: h.clk.clock,
170- wake: func(w renderWake) { h.wakes <- w },
171- ackReady: func() bool { return h.ackReady.Load() },
172- syncActive: func() bool { return h.syncActive.Load() },
169+ clock: h.clk.clock,
170+ wake: func(w renderWake) { h.wakes <- w },
171+ ackReadyFor: func(*attachedClient) bool { return h.ackReady.Load() },
172+ syncActive: func() bool { return h.syncActive.Load() },
173173 })
174174 return h
175175 }
@@ -449,7 +449,7 @@ func TestRenderCoordinatorAckReadinessReentersWithoutBlockingResize(t *testing.T
449449
450450 var rc *renderCoordinator
451451 rc = newRenderCoordinator(renderCoordinatorOptions{
452- ackReady: func() bool {
452+ ackReadyFor: func(*attachedClient) bool {
453453 close(ackEntered)
454454 // The readiness probe models the output send path reading coordinator
455455 // metadata after sendMu is held. It must never run under c.mu.
@@ -1451,9 +1451,9 @@ func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T
14511451
14521452 wakes := make(chan renderWake, 1)
14531453 rc := newRenderCoordinator(renderCoordinatorOptions{
1454- clock: clock,
1455- ackReady: func() bool { return true },
1456- wake: func(w renderWake) { wakes <- w },
1454+ clock: clock,
1455+ ackReadyFor: func(*attachedClient) bool { return true },
1456+ wake: func(w renderWake) { wakes <- w },
14571457 })
14581458 rc.invalidate(renderInvalidation{class: invalidateOutput})
14591459
@@ -97,7 +97,7 @@ func TestRenderCoordinatorDetachDoesNotJoinSelectedDeadlineWorker(t *testing.T)
97 release := make(chan struct{})97 release := make(chan struct{})
98 rc := newRenderCoordinator(renderCoordinatorOptions{98 rc := newRenderCoordinator(renderCoordinatorOptions{
99 clock: clock,99 clock: clock,
100- ackReady: func() bool {100+ ackReadyFor: func(*attachedClient) bool {
101 close(entered)101 close(entered)
102 <-release102 <-release
103 return true103 return true
@@ -166,10 +166,10 @@ func newCoordinatorHarness(t *testing.T) *coordinatorHarness {
166 }166 }
167 h.ackReady.Store(true)167 h.ackReady.Store(true)
168 h.rc = newRenderCoordinator(renderCoordinatorOptions{168 h.rc = newRenderCoordinator(renderCoordinatorOptions{
169- clock: h.clk.clock,169+ clock: h.clk.clock,
170- wake: func(w renderWake) { h.wakes <- w },170+ wake: func(w renderWake) { h.wakes <- w },
171- ackReady: func() bool { return h.ackReady.Load() },171+ ackReadyFor: func(*attachedClient) bool { return h.ackReady.Load() },
172- syncActive: func() bool { return h.syncActive.Load() },172+ syncActive: func() bool { return h.syncActive.Load() },
173 })173 })
174 return h174 return h
175 }175 }
@@ -449,7 +449,7 @@ func TestRenderCoordinatorAckReadinessReentersWithoutBlockingResize(t *testing.T
449 449
450 var rc *renderCoordinator450 var rc *renderCoordinator
451 rc = newRenderCoordinator(renderCoordinatorOptions{451 rc = newRenderCoordinator(renderCoordinatorOptions{
452- ackReady: func() bool {452+ ackReadyFor: func(*attachedClient) bool {
453 close(ackEntered)453 close(ackEntered)
454 // The readiness probe models the output send path reading coordinator454 // The readiness probe models the output send path reading coordinator
455 // metadata after sendMu is held. It must never run under c.mu.455 // metadata after sendMu is held. It must never run under c.mu.
@@ -1451,9 +1451,9 @@ func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T
1451 1451
1452 wakes := make(chan renderWake, 1)1452 wakes := make(chan renderWake, 1)
1453 rc := newRenderCoordinator(renderCoordinatorOptions{1453 rc := newRenderCoordinator(renderCoordinatorOptions{
1454- clock: clock,1454+ clock: clock,
1455- ackReady: func() bool { return true },1455+ ackReadyFor: func(*attachedClient) bool { return true },
1456- wake: func(w renderWake) { wakes <- w },1456+ wake: func(w renderWake) { wakes <- w },
1457 })1457 })
1458 rc.invalidate(renderInvalidation{class: invalidateOutput})1458 rc.invalidate(renderInvalidation{class: invalidateOutput})
1459 1459
M internal/usecase/daemon/render_coordinator_types.go +3 -5
@@ -105,11 +105,9 @@ type renderCoordinatorOptions struct {
105105 observer ports.RuntimeObserver
106106 // wake composes a coalesced render request.
107107 wake func(renderWake)
108- // ackReady reports whether the attachment may compose another output
109- // state (the attachmentOutput window has capacity). It is retained for
110- // coordinator-only tests and headless callers; production uses ackReadyFor
111- // so one attachment cannot gate another.
112- ackReady func() bool
108+ // ackReadyFor reports whether one attachment may compose another output
109+ // state (its attachmentOutput window has capacity), so one attachment
110+ // cannot gate another. Nil means every attachment is ready.
113111 ackReadyFor func(*attachedClient) bool
114112 // syncRenderable reports whether a pane's synchronized batch currently
115113 // contributes to the attached composition. It must not acquire c.mu.
@@ -105,11 +105,9 @@ type renderCoordinatorOptions struct {
105 observer ports.RuntimeObserver105 observer ports.RuntimeObserver
106 // wake composes a coalesced render request.106 // wake composes a coalesced render request.
107 wake func(renderWake)107 wake func(renderWake)
108- // ackReady reports whether the attachment may compose another output108+ // ackReadyFor reports whether one attachment may compose another output
109- // state (the attachmentOutput window has capacity). It is retained for109+ // state (its attachmentOutput window has capacity), so one attachment
110- // coordinator-only tests and headless callers; production uses ackReadyFor110+ // cannot gate another. Nil means every attachment is ready.
111- // so one attachment cannot gate another.
112- ackReady func() bool
113 ackReadyFor func(*attachedClient) bool111 ackReadyFor func(*attachedClient) bool
114 // syncRenderable reports whether a pane's synchronized batch currently112 // syncRenderable reports whether a pane's synchronized batch currently
115 // contributes to the attached composition. It must not acquire c.mu.113 // contributes to the attached composition. It must not acquire c.mu.
M internal/usecase/daemon/transport_observability_test.go +10 -9
@@ -188,8 +188,8 @@ func TestTransportObservabilityACKBlockedSpansEndExactlyOnce(t *testing.T) {
188188 observer := &daemonRuntimeObserver{}
189189 ready := false
190190 rc := newRenderCoordinator(renderCoordinatorOptions{
191- observer: observer,
192- ackReady: func() bool { return ready },
191+ observer: observer,
192+ ackReadyFor: func(*attachedClient) bool { return ready },
193193 })
194194 rc.attach(&attachedClient{})
195195 rc.invalidate(renderInvalidation{class: invalidateOutput})
@@ -233,9 +233,9 @@ func TestTransportObservabilityBlockedACKEndDoesNotDelayNotifyAck(t *testing.T)
233233 wakes := make(chan renderWake, 1)
234234 var ready atomic.Bool
235235 rc := newRenderCoordinator(renderCoordinatorOptions{
236- observer: d.runtimeObserver,
237- ackReady: ready.Load,
238- wake: func(w renderWake) { wakes <- w },
236+ observer: d.runtimeObserver,
237+ ackReadyFor: func(*attachedClient) bool { return ready.Load() },
238+ wake: func(w renderWake) { wakes <- w },
239239 })
240240 rc.attach(&attachedClient{})
241241 rc.invalidate(renderInvalidation{class: invalidateOutput})
@@ -284,8 +284,8 @@ func (o *blockingACKEndObserver) ObserveRuntime(mark ports.RuntimeMark) {
284284 func TestTransportObservabilityBlockedCoordinatorObserverReleasesLock(t *testing.T) {
285285 observer := newBlockingDaemonRuntimeObserver()
286286 rc := newRenderCoordinator(renderCoordinatorOptions{
287- observer: observer,
288- ackReady: func() bool { return false },
287+ observer: observer,
288+ ackReadyFor: func(*attachedClient) bool { return false },
289289 })
290290 rc.attach(&attachedClient{})
291291 rc.mu.Lock()
@@ -347,9 +347,10 @@ func TestTransportObservabilityACKBlockedStartPublishesBeforeEnd(t *testing.T) {
347347 observer := newACKSpanInterleavingObserver()
348348 ready := false
349349 rc := newRenderCoordinator(renderCoordinatorOptions{
350- observer: observer,
351- ackReady: func() bool { return ready },
350+ observer: observer,
351+ ackReadyFor: func(*attachedClient) bool { return ready },
352352 })
353+ rc.attach(&attachedClient{})
353354 rc.mu.Lock()
354355 rc.pending = true
355356 gen := rc.normalLane.generation
@@ -188,8 +188,8 @@ func TestTransportObservabilityACKBlockedSpansEndExactlyOnce(t *testing.T) {
188 observer := &daemonRuntimeObserver{}188 observer := &daemonRuntimeObserver{}
189 ready := false189 ready := false
190 rc := newRenderCoordinator(renderCoordinatorOptions{190 rc := newRenderCoordinator(renderCoordinatorOptions{
191- observer: observer,191+ observer: observer,
192- ackReady: func() bool { return ready },192+ ackReadyFor: func(*attachedClient) bool { return ready },
193 })193 })
194 rc.attach(&attachedClient{})194 rc.attach(&attachedClient{})
195 rc.invalidate(renderInvalidation{class: invalidateOutput})195 rc.invalidate(renderInvalidation{class: invalidateOutput})
@@ -233,9 +233,9 @@ func TestTransportObservabilityBlockedACKEndDoesNotDelayNotifyAck(t *testing.T)
233 wakes := make(chan renderWake, 1)233 wakes := make(chan renderWake, 1)
234 var ready atomic.Bool234 var ready atomic.Bool
235 rc := newRenderCoordinator(renderCoordinatorOptions{235 rc := newRenderCoordinator(renderCoordinatorOptions{
236- observer: d.runtimeObserver,236+ observer: d.runtimeObserver,
237- ackReady: ready.Load,237+ ackReadyFor: func(*attachedClient) bool { return ready.Load() },
238- wake: func(w renderWake) { wakes <- w },238+ wake: func(w renderWake) { wakes <- w },
239 })239 })
240 rc.attach(&attachedClient{})240 rc.attach(&attachedClient{})
241 rc.invalidate(renderInvalidation{class: invalidateOutput})241 rc.invalidate(renderInvalidation{class: invalidateOutput})
@@ -284,8 +284,8 @@ func (o *blockingACKEndObserver) ObserveRuntime(mark ports.RuntimeMark) {
284 func TestTransportObservabilityBlockedCoordinatorObserverReleasesLock(t *testing.T) {284 func TestTransportObservabilityBlockedCoordinatorObserverReleasesLock(t *testing.T) {
285 observer := newBlockingDaemonRuntimeObserver()285 observer := newBlockingDaemonRuntimeObserver()
286 rc := newRenderCoordinator(renderCoordinatorOptions{286 rc := newRenderCoordinator(renderCoordinatorOptions{
287- observer: observer,287+ observer: observer,
288- ackReady: func() bool { return false },288+ ackReadyFor: func(*attachedClient) bool { return false },
289 })289 })
290 rc.attach(&attachedClient{})290 rc.attach(&attachedClient{})
291 rc.mu.Lock()291 rc.mu.Lock()
@@ -347,9 +347,10 @@ func TestTransportObservabilityACKBlockedStartPublishesBeforeEnd(t *testing.T) {
347 observer := newACKSpanInterleavingObserver()347 observer := newACKSpanInterleavingObserver()
348 ready := false348 ready := false
349 rc := newRenderCoordinator(renderCoordinatorOptions{349 rc := newRenderCoordinator(renderCoordinatorOptions{
350- observer: observer,350+ observer: observer,
351- ackReady: func() bool { return ready },351+ ackReadyFor: func(*attachedClient) bool { return ready },
352 })352 })
353+ rc.attach(&attachedClient{})
353 rc.mu.Lock()354 rc.mu.Lock()
354 rc.pending = true355 rc.pending = true
355 gen := rc.normalLane.generation356 gen := rc.normalLane.generation

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