fix(daemon): clear the palette before opening a client picker (#292)
* fix(daemon): send palette-opened pickers after the palette-erasing frame * fix(daemon): keep deferred picker opens reliable and command refusals synchronous * test(app): drive local identity retry with a sandbox clock
- commit
- 5334f3383c4bcbfe2985366a847d265f42bcd238
- author
- bnema · · 2026-09-26 05:17 UTC
- committer
- GitHub · · 2026-09-26 05:17 UTC
- parent
- 9386b87
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
12 files · +411 -11
M docs/protocol.md +12 -8
| @@ -168,14 +168,18 @@ adapter's queue. IPC reuses one grow-once read buffer internally. | |||
| 168 | 168 | ||
| 169 | 169 | ## Picker admission without physical write | |
| 170 | 170 | ||
| 171 | - | While a client-picker lease is acquiring/owned/releasing | |
| 172 | - | (`client/picker_lease.go`), the client still applies every accepted | |
| 173 | - | daemon frame to its output shadow and acknowledges it inside the bounded | |
| 174 | - | window, but suppresses the physical write until the barrier | |
| 175 | - | (epoch/state) is applied (acquire), an authoritative post-close full | |
| 176 | - | paint arrives (release), or the lease aborts (disconnect/generation | |
| 177 | - | change). The daemon side drops raw key/mouse input for that attachment | |
| 178 | - | while `pickerOpen`; only typed picker messages act. | |
| 171 | + | While a client picker overlay is open over a live attachment (session | |
| 172 | + | picker `PickerOffer`, or the first move `PickerSnapshot`), the client | |
| 173 | + | still applies and acknowledges every accepted daemon frame but does not | |
| 174 | + | write it. Closing the overlay sends `OutputResetRequest`, and the daemon | |
| 175 | + | answers with an authoritative full paint. | |
| 176 | + | ||
| 177 | + | The daemon sends a palette-opened picker only after a frame composed | |
| 178 | + | after the palette closed has been emitted, so the erased palette is on | |
| 179 | + | screen before output suppression starts. An ACK-blocked paint or a | |
| 180 | + | transient freeze delays the picker; a detach or transport change drops it. The daemon side drops raw | |
| 181 | + | key/mouse input for the attachment while a move picker is open; only | |
| 182 | + | typed picker messages act. | |
| 179 | 183 | ||
| 180 | 184 | ## Ordering, close, and admission semantics | |
| 181 | 185 | ||
| @@ -168,14 +168,18 @@ adapter's queue. IPC reuses one grow-once read buffer internally. | |||||
| 168 | 168 | ||||
| 169 | ## Picker admission without physical write | 169 | ## Picker admission without physical write | ||
| 170 | 170 | ||||
| 171 | - | While a client-picker lease is acquiring/owned/releasing | 171 | + | While a client picker overlay is open over a live attachment (session |
| 172 | - | (`client/picker_lease.go`), the client still applies every accepted | 172 | + | picker `PickerOffer`, or the first move `PickerSnapshot`), the client |
| 173 | - | daemon frame to its output shadow and acknowledges it inside the bounded | 173 | + | still applies and acknowledges every accepted daemon frame but does not |
| 174 | - | window, but suppresses the physical write until the barrier | 174 | + | write it. Closing the overlay sends `OutputResetRequest`, and the daemon |
| 175 | - | (epoch/state) is applied (acquire), an authoritative post-close full | 175 | + | answers with an authoritative full paint. |
| 176 | - | paint arrives (release), or the lease aborts (disconnect/generation | 176 | + | |
| 177 | - | change). The daemon side drops raw key/mouse input for that attachment | 177 | + | The daemon sends a palette-opened picker only after a frame composed |
| 178 | - | while `pickerOpen`; only typed picker messages act. | 178 | + | after the palette closed has been emitted, so the erased palette is on |
| 179 | + | screen before output suppression starts. An ACK-blocked paint or a | |||
| 180 | + | transient freeze delays the picker; a detach or transport change drops it. The daemon side drops raw | |||
| 181 | + | key/mouse input for the attachment while a move picker is open; only | |||
| 182 | + | typed picker messages act. | |||
| 179 | 183 | ||||
| 180 | ## Ordering, close, and admission semantics | 184 | ## Ordering, close, and admission semantics | ||
| 181 | 185 | ||||
M internal/app/broker_offline_local_test.go +7 -1
| @@ -581,7 +581,10 @@ func TestBrokerLocalObservationPublishesReachableCatalogueWhenIdentityAppears(t | |||
| 581 | 581 | store, err := brokerstore.Open(brokerstore.Options{Dir: stateDir}) | |
| 582 | 582 | require.NoError(t, err) | |
| 583 | 583 | t.Cleanup(func() { require.NoError(t, store.Close()) }) | |
| 584 | - | registry, err := broker.NewRegistryWithConfig(brokerLocalTestEpoch, store, nil, clock.New(), discardLog(), broker.RegistryConfig{Local: local, MembershipMode: broker.MembershipMutable}) | |
| 584 | + | // A deterministic clock: the identity retry is a jittered multi-second | |
| 585 | + | // delay, so wall time would race the test's wait bound. | |
| 586 | + | clk := newSandboxClock() | |
| 587 | + | registry, err := broker.NewRegistryWithConfig(brokerLocalTestEpoch, store, nil, clk, discardLog(), broker.RegistryConfig{Local: local, MembershipMode: broker.MembershipMutable}) | |
| 585 | 588 | require.NoError(t, err) | |
| 586 | 589 | startLocalRegistry(t, registry) | |
| 587 | 590 | ||
| @@ -589,6 +592,9 @@ func TestBrokerLocalObservationPublishesReachableCatalogueWhenIdentityAppears(t | |||
| 589 | 592 | require.Equal(t, int32(0), daemon.dials.Load(), "no dial happens while no identity exists") | |
| 590 | 593 | ||
| 591 | 594 | identity.Store(daemonIdentity) | |
| 595 | + | // Step past the longest jittered retry (retry base plus 10%) so the next | |
| 596 | + | // attempt reads the published identity. | |
| 597 | + | clk.Advance(6 * time.Second) | |
| 592 | 598 | observed := waitLocalEntry(t, registry, func(entry ports.BrokerDaemonObservation) bool { | |
| 593 | 599 | return entry.Availability == domain.RemoteAvailabilityReachable | |
| 594 | 600 | }) | |
| @@ -581,7 +581,10 @@ func TestBrokerLocalObservationPublishesReachableCatalogueWhenIdentityAppears(t | |||||
| 581 | store, err := brokerstore.Open(brokerstore.Options{Dir: stateDir}) | 581 | store, err := brokerstore.Open(brokerstore.Options{Dir: stateDir}) | ||
| 582 | require.NoError(t, err) | 582 | require.NoError(t, err) | ||
| 583 | t.Cleanup(func() { require.NoError(t, store.Close()) }) | 583 | t.Cleanup(func() { require.NoError(t, store.Close()) }) | ||
| 584 | - | registry, err := broker.NewRegistryWithConfig(brokerLocalTestEpoch, store, nil, clock.New(), discardLog(), broker.RegistryConfig{Local: local, MembershipMode: broker.MembershipMutable}) | 584 | + | // A deterministic clock: the identity retry is a jittered multi-second |
| 585 | + | // delay, so wall time would race the test's wait bound. | |||
| 586 | + | clk := newSandboxClock() | |||
| 587 | + | registry, err := broker.NewRegistryWithConfig(brokerLocalTestEpoch, store, nil, clk, discardLog(), broker.RegistryConfig{Local: local, MembershipMode: broker.MembershipMutable}) | |||
| 585 | require.NoError(t, err) | 588 | require.NoError(t, err) | ||
| 586 | startLocalRegistry(t, registry) | 589 | startLocalRegistry(t, registry) | ||
| 587 | 590 | ||||
| @@ -589,6 +592,9 @@ func TestBrokerLocalObservationPublishesReachableCatalogueWhenIdentityAppears(t | |||||
| 589 | require.Equal(t, int32(0), daemon.dials.Load(), "no dial happens while no identity exists") | 592 | require.Equal(t, int32(0), daemon.dials.Load(), "no dial happens while no identity exists") | ||
| 590 | 593 | ||||
| 591 | identity.Store(daemonIdentity) | 594 | identity.Store(daemonIdentity) | ||
| 595 | + | // Step past the longest jittered retry (retry base plus 10%) so the next | |||
| 596 | + | // attempt reads the published identity. | |||
| 597 | + | clk.Advance(6 * time.Second) | |||
| 592 | observed := waitLocalEntry(t, registry, func(entry ports.BrokerDaemonObservation) bool { | 598 | observed := waitLocalEntry(t, registry, func(entry ports.BrokerDaemonObservation) bool { | ||
| 593 | return entry.Availability == domain.RemoteAvailabilityReachable | 599 | return entry.Availability == domain.RemoteAvailabilityReachable | ||
| 594 | }) | 600 | }) | ||
A internal/usecase/daemon/after_frame.go +131 -0
| @@ -0,0 +1,131 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import "sync" | |
| 4 | + | ||
| 5 | + | // afterFrameQueue orders a control after the attachment's next composed frame. | |
| 6 | + | // | |
| 7 | + | // A client overlay (the session or move picker) suppresses attachment output | |
| 8 | + | // from the moment it opens, so the frame that erases a daemon overlay closed by | |
| 9 | + | // the same action must reach the client first. Queued work is released once a | |
| 10 | + | // frame captured after its registration has been emitted. The transport is one | |
| 11 | + | // ordered stream, so that frame is queued on the wire before anything the work | |
| 12 | + | // sends. | |
| 13 | + | type afterFrameQueue struct { | |
| 14 | + | mu sync.Mutex | |
| 15 | + | captures uint64 // captures started so far | |
| 16 | + | pending []afterFrameWork | |
| 17 | + | ready []func() | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | type afterFrameWork struct { | |
| 21 | + | minCapture uint64 // first capture that observes state at registration | |
| 22 | + | run func() | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | func (q *afterFrameQueue) add(run func()) { | |
| 26 | + | q.mu.Lock() | |
| 27 | + | q.pending = append(q.pending, afterFrameWork{minCapture: q.captures + 1, run: run}) | |
| 28 | + | q.mu.Unlock() | |
| 29 | + | } | |
| 30 | + | ||
| 31 | + | // beginCapture numbers one frame capture. Paint calls it under sendMu before | |
| 32 | + | // snapshotting overlays, so a capture numbered after a registration observes | |
| 33 | + | // every state change that preceded it. | |
| 34 | + | func (q *afterFrameQueue) beginCapture() uint64 { | |
| 35 | + | q.mu.Lock() | |
| 36 | + | defer q.mu.Unlock() | |
| 37 | + | q.captures++ | |
| 38 | + | return q.captures | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | // emitted releases the work satisfied by the emitted capture. | |
| 42 | + | func (q *afterFrameQueue) emitted(capture uint64) { | |
| 43 | + | if capture == 0 { | |
| 44 | + | return | |
| 45 | + | } | |
| 46 | + | q.mu.Lock() | |
| 47 | + | defer q.mu.Unlock() | |
| 48 | + | kept := q.pending[:0] | |
| 49 | + | for _, work := range q.pending { | |
| 50 | + | if work.minCapture <= capture { | |
| 51 | + | q.ready = append(q.ready, work.run) | |
| 52 | + | } else { | |
| 53 | + | kept = append(kept, work) | |
| 54 | + | } | |
| 55 | + | } | |
| 56 | + | clear(q.pending[len(kept):]) | |
| 57 | + | q.pending = kept | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | // runReady starts released work on its own goroutine: the painting goroutine | |
| 61 | + | // (often a render wake) must not wait on picker projections or a blocking | |
| 62 | + | // control send, and its caller may hold dispatch locks. | |
| 63 | + | func (q *afterFrameQueue) runReady() { | |
| 64 | + | q.mu.Lock() | |
| 65 | + | ready := q.ready | |
| 66 | + | q.ready = nil | |
| 67 | + | q.mu.Unlock() | |
| 68 | + | for _, run := range ready { | |
| 69 | + | go run() | |
| 70 | + | } | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | // afterNextFrame runs work with a fresh effect once a frame composed after this | |
| 74 | + | // call reached the client. A transient freeze of the same attachment delays the | |
| 75 | + | // work until the freeze ends (the frame is already on the wire); a changed | |
| 76 | + | // capability (detach, suspension, transport replacement, session switch) or the | |
| 77 | + | // session ending drops it. | |
| 78 | + | func (d *Daemon) afterNextFrame(sess *session, ac *attachedClient, effect *attachmentEffect, work func(*attachmentEffect)) { | |
| 79 | + | d.queueAfterNextFrame(ac, effect, work) | |
| 80 | + | d.invalidateRender(sess, ac, true, "after_frame.go") | |
| 81 | + | } | |
| 82 | + | ||
| 83 | + | // queueAfterNextFrame registers work without requesting the frame. | |
| 84 | + | func (d *Daemon) queueAfterNextFrame(ac *attachedClient, effect *attachmentEffect, work func(*attachmentEffect)) { | |
| 85 | + | capability := effect.capability() | |
| 86 | + | ac.afterFrame.add(func() { | |
| 87 | + | fresh, admitted := waitAttachmentEffect(capability) | |
| 88 | + | if !admitted { | |
| 89 | + | if d.log != nil { | |
| 90 | + | d.log.Debug("dropping after-frame work for a replaced attachment") | |
| 91 | + | } | |
| 92 | + | return | |
| 93 | + | } | |
| 94 | + | defer fresh.End() | |
| 95 | + | work(fresh) | |
| 96 | + | }) | |
| 97 | + | } | |
| 98 | + | ||
| 99 | + | // waitAttachmentEffect admits capability, waiting out a transient freeze while | |
| 100 | + | // it stays current. It gives up once the capability is replaced or its session | |
| 101 | + | // ends. | |
| 102 | + | func waitAttachmentEffect(capability attachmentCapability) (*attachmentEffect, bool) { | |
| 103 | + | ac := capability.ac | |
| 104 | + | if ac == nil || capability.sess == nil { | |
| 105 | + | return nil, false | |
| 106 | + | } | |
| 107 | + | var done <-chan struct{} | |
| 108 | + | if ctx := capability.sess.ctx; ctx != nil { | |
| 109 | + | done = ctx.Done() | |
| 110 | + | } | |
| 111 | + | for { | |
| 112 | + | // Take the change signal before trying, so a thaw between the failed | |
| 113 | + | // admission and the wait is never missed. | |
| 114 | + | g := &ac.lifecycle | |
| 115 | + | g.mu.Lock() | |
| 116 | + | g.initLocked() | |
| 117 | + | changed := g.changed | |
| 118 | + | g.mu.Unlock() | |
| 119 | + | if fresh, admitted := ac.beginAttachmentEffect(capability); admitted { | |
| 120 | + | return fresh, true | |
| 121 | + | } | |
| 122 | + | if !capability.current() { | |
| 123 | + | return nil, false | |
| 124 | + | } | |
| 125 | + | select { | |
| 126 | + | case <-changed: | |
| 127 | + | case <-done: | |
| 128 | + | return nil, false | |
| 129 | + | } | |
| 130 | + | } | |
| 131 | + | } | |
| @@ -0,0 +1,131 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import "sync" | |||
| 4 | + | ||||
| 5 | + | // afterFrameQueue orders a control after the attachment's next composed frame. | |||
| 6 | + | // | |||
| 7 | + | // A client overlay (the session or move picker) suppresses attachment output | |||
| 8 | + | // from the moment it opens, so the frame that erases a daemon overlay closed by | |||
| 9 | + | // the same action must reach the client first. Queued work is released once a | |||
| 10 | + | // frame captured after its registration has been emitted. The transport is one | |||
| 11 | + | // ordered stream, so that frame is queued on the wire before anything the work | |||
| 12 | + | // sends. | |||
| 13 | + | type afterFrameQueue struct { | |||
| 14 | + | mu sync.Mutex | |||
| 15 | + | captures uint64 // captures started so far | |||
| 16 | + | pending []afterFrameWork | |||
| 17 | + | ready []func() | |||
| 18 | + | } | |||
| 19 | + | ||||
| 20 | + | type afterFrameWork struct { | |||
| 21 | + | minCapture uint64 // first capture that observes state at registration | |||
| 22 | + | run func() | |||
| 23 | + | } | |||
| 24 | + | ||||
| 25 | + | func (q *afterFrameQueue) add(run func()) { | |||
| 26 | + | q.mu.Lock() | |||
| 27 | + | q.pending = append(q.pending, afterFrameWork{minCapture: q.captures + 1, run: run}) | |||
| 28 | + | q.mu.Unlock() | |||
| 29 | + | } | |||
| 30 | + | ||||
| 31 | + | // beginCapture numbers one frame capture. Paint calls it under sendMu before | |||
| 32 | + | // snapshotting overlays, so a capture numbered after a registration observes | |||
| 33 | + | // every state change that preceded it. | |||
| 34 | + | func (q *afterFrameQueue) beginCapture() uint64 { | |||
| 35 | + | q.mu.Lock() | |||
| 36 | + | defer q.mu.Unlock() | |||
| 37 | + | q.captures++ | |||
| 38 | + | return q.captures | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | // emitted releases the work satisfied by the emitted capture. | |||
| 42 | + | func (q *afterFrameQueue) emitted(capture uint64) { | |||
| 43 | + | if capture == 0 { | |||
| 44 | + | return | |||
| 45 | + | } | |||
| 46 | + | q.mu.Lock() | |||
| 47 | + | defer q.mu.Unlock() | |||
| 48 | + | kept := q.pending[:0] | |||
| 49 | + | for _, work := range q.pending { | |||
| 50 | + | if work.minCapture <= capture { | |||
| 51 | + | q.ready = append(q.ready, work.run) | |||
| 52 | + | } else { | |||
| 53 | + | kept = append(kept, work) | |||
| 54 | + | } | |||
| 55 | + | } | |||
| 56 | + | clear(q.pending[len(kept):]) | |||
| 57 | + | q.pending = kept | |||
| 58 | + | } | |||
| 59 | + | ||||
| 60 | + | // runReady starts released work on its own goroutine: the painting goroutine | |||
| 61 | + | // (often a render wake) must not wait on picker projections or a blocking | |||
| 62 | + | // control send, and its caller may hold dispatch locks. | |||
| 63 | + | func (q *afterFrameQueue) runReady() { | |||
| 64 | + | q.mu.Lock() | |||
| 65 | + | ready := q.ready | |||
| 66 | + | q.ready = nil | |||
| 67 | + | q.mu.Unlock() | |||
| 68 | + | for _, run := range ready { | |||
| 69 | + | go run() | |||
| 70 | + | } | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | // afterNextFrame runs work with a fresh effect once a frame composed after this | |||
| 74 | + | // call reached the client. A transient freeze of the same attachment delays the | |||
| 75 | + | // work until the freeze ends (the frame is already on the wire); a changed | |||
| 76 | + | // capability (detach, suspension, transport replacement, session switch) or the | |||
| 77 | + | // session ending drops it. | |||
| 78 | + | func (d *Daemon) afterNextFrame(sess *session, ac *attachedClient, effect *attachmentEffect, work func(*attachmentEffect)) { | |||
| 79 | + | d.queueAfterNextFrame(ac, effect, work) | |||
| 80 | + | d.invalidateRender(sess, ac, true, "after_frame.go") | |||
| 81 | + | } | |||
| 82 | + | ||||
| 83 | + | // queueAfterNextFrame registers work without requesting the frame. | |||
| 84 | + | func (d *Daemon) queueAfterNextFrame(ac *attachedClient, effect *attachmentEffect, work func(*attachmentEffect)) { | |||
| 85 | + | capability := effect.capability() | |||
| 86 | + | ac.afterFrame.add(func() { | |||
| 87 | + | fresh, admitted := waitAttachmentEffect(capability) | |||
| 88 | + | if !admitted { | |||
| 89 | + | if d.log != nil { | |||
| 90 | + | d.log.Debug("dropping after-frame work for a replaced attachment") | |||
| 91 | + | } | |||
| 92 | + | return | |||
| 93 | + | } | |||
| 94 | + | defer fresh.End() | |||
| 95 | + | work(fresh) | |||
| 96 | + | }) | |||
| 97 | + | } | |||
| 98 | + | ||||
| 99 | + | // waitAttachmentEffect admits capability, waiting out a transient freeze while | |||
| 100 | + | // it stays current. It gives up once the capability is replaced or its session | |||
| 101 | + | // ends. | |||
| 102 | + | func waitAttachmentEffect(capability attachmentCapability) (*attachmentEffect, bool) { | |||
| 103 | + | ac := capability.ac | |||
| 104 | + | if ac == nil || capability.sess == nil { | |||
| 105 | + | return nil, false | |||
| 106 | + | } | |||
| 107 | + | var done <-chan struct{} | |||
| 108 | + | if ctx := capability.sess.ctx; ctx != nil { | |||
| 109 | + | done = ctx.Done() | |||
| 110 | + | } | |||
| 111 | + | for { | |||
| 112 | + | // Take the change signal before trying, so a thaw between the failed | |||
| 113 | + | // admission and the wait is never missed. | |||
| 114 | + | g := &ac.lifecycle | |||
| 115 | + | g.mu.Lock() | |||
| 116 | + | g.initLocked() | |||
| 117 | + | changed := g.changed | |||
| 118 | + | g.mu.Unlock() | |||
| 119 | + | if fresh, admitted := ac.beginAttachmentEffect(capability); admitted { | |||
| 120 | + | return fresh, true | |||
| 121 | + | } | |||
| 122 | + | if !capability.current() { | |||
| 123 | + | return nil, false | |||
| 124 | + | } | |||
| 125 | + | select { | |||
| 126 | + | case <-changed: | |||
| 127 | + | case <-done: | |||
| 128 | + | return nil, false | |||
| 129 | + | } | |||
| 130 | + | } | |||
| 131 | + | } | |||
A internal/usecase/daemon/after_frame_test.go +107 -0
| @@ -0,0 +1,107 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | "time" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/require" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | func TestAfterFrameQueueReleasesOnlyAfterALaterCapture(t *testing.T) { | |
| 11 | + | tests := []struct { | |
| 12 | + | name string | |
| 13 | + | steps []string // "add:<id>", "capture", "emit:<n>" | |
| 14 | + | wantRan []string | |
| 15 | + | }{ | |
| 16 | + | {name: "nothing emitted", steps: []string{"add:a", "capture"}}, | |
| 17 | + | {name: "capture started before registration", steps: []string{"capture", "add:a", "capture", "emit:1"}}, | |
| 18 | + | {name: "capture started after registration", steps: []string{"capture", "add:a", "capture", "emit:2"}, wantRan: []string{"a"}}, | |
| 19 | + | {name: "unnumbered emission", steps: []string{"add:a", "capture", "emit:0"}}, | |
| 20 | + | {name: "partial release keeps later work", steps: []string{"add:a", "capture", "add:b", "capture", "emit:1"}, wantRan: []string{"a"}}, | |
| 21 | + | {name: "later emission releases the rest", steps: []string{"add:a", "capture", "add:b", "capture", "emit:1", "emit:2"}, wantRan: []string{"a", "b"}}, | |
| 22 | + | } | |
| 23 | + | for _, tt := range tests { | |
| 24 | + | t.Run(tt.name, func(t *testing.T) { | |
| 25 | + | var q afterFrameQueue | |
| 26 | + | ran := make(chan string, len(tt.steps)) | |
| 27 | + | for _, step := range tt.steps { | |
| 28 | + | switch { | |
| 29 | + | case step == "capture": | |
| 30 | + | q.beginCapture() | |
| 31 | + | case step[:4] == "add:": | |
| 32 | + | id := step[4:] | |
| 33 | + | q.add(func() { ran <- id }) | |
| 34 | + | case step[:5] == "emit:": | |
| 35 | + | q.emitted(uint64(step[5] - '0')) | |
| 36 | + | } | |
| 37 | + | } | |
| 38 | + | q.runReady() | |
| 39 | + | q.runReady() | |
| 40 | + | var got []string | |
| 41 | + | for range tt.wantRan { | |
| 42 | + | got = append(got, awaitTestValue(t, ran, "released work did not run")) | |
| 43 | + | } | |
| 44 | + | require.ElementsMatch(t, tt.wantRan, got) | |
| 45 | + | select { | |
| 46 | + | case extra := <-ran: | |
| 47 | + | t.Fatalf("unexpected work %q ran", extra) | |
| 48 | + | case <-time.After(50 * time.Millisecond): | |
| 49 | + | } | |
| 50 | + | }) | |
| 51 | + | } | |
| 52 | + | } | |
| 53 | + | ||
| 54 | + | // Work registered through afterNextFrame runs once a later capture is emitted | |
| 55 | + | // on the same attachment, and is dropped once the attachment reconnected. | |
| 56 | + | func TestAfterNextFrameRunsOrDropsByCapability(t *testing.T) { | |
| 57 | + | tests := []struct { | |
| 58 | + | name string | |
| 59 | + | reconnect bool | |
| 60 | + | freeze bool | |
| 61 | + | wantRun bool | |
| 62 | + | }{ | |
| 63 | + | {name: "same attachment runs", wantRun: true}, | |
| 64 | + | {name: "reconnected attachment drops", reconnect: true}, | |
| 65 | + | {name: "transient freeze delays until thaw", freeze: true, wantRun: true}, | |
| 66 | + | } | |
| 67 | + | for _, tt := range tests { | |
| 68 | + | t.Run(tt.name, func(t *testing.T) { | |
| 69 | + | d, source, ac, _, _, releases, _ := setupMovePickerSessionsCore(t, stubClock{}, 0) | |
| 70 | + | defer releaseAll(releases) | |
| 71 | + | effect, admitted := ac.beginAttachmentEffect(captureAttachmentCapability(source, ac, ac.transport())) | |
| 72 | + | require.True(t, admitted) | |
| 73 | + | ran := make(chan struct{}, 1) | |
| 74 | + | d.queueAfterNextFrame(ac, effect, func(*attachmentEffect) { ran <- struct{}{} }) | |
| 75 | + | effect.End() | |
| 76 | + | if tt.reconnect { | |
| 77 | + | ac.replaceTransport(ac.transport()) | |
| 78 | + | ac.installTestAttachmentCapability(captureAttachmentCapability(source, ac, ac.transport())) | |
| 79 | + | } | |
| 80 | + | var frozen attachmentTransitionGuard | |
| 81 | + | if tt.freeze { | |
| 82 | + | frozen = freezeAttachmentEffectGates(ac) | |
| 83 | + | require.True(t, frozen.acquired) | |
| 84 | + | } | |
| 85 | + | ac.afterFrame.emitted(ac.afterFrame.beginCapture()) | |
| 86 | + | ac.afterFrame.runReady() | |
| 87 | + | if tt.freeze { | |
| 88 | + | // Released while frozen: it waits, with no further paint needed. | |
| 89 | + | select { | |
| 90 | + | case <-ran: | |
| 91 | + | t.Fatal("work ran while the attachment was frozen") | |
| 92 | + | case <-time.After(50 * time.Millisecond): | |
| 93 | + | } | |
| 94 | + | frozen.unfreeze() | |
| 95 | + | } | |
| 96 | + | if tt.wantRun { | |
| 97 | + | awaitTestValue(t, ran, "queued work did not run after the frame") | |
| 98 | + | return | |
| 99 | + | } | |
| 100 | + | select { | |
| 101 | + | case <-ran: | |
| 102 | + | t.Fatal("work ran for a reconnected attachment") | |
| 103 | + | case <-time.After(100 * time.Millisecond): | |
| 104 | + | } | |
| 105 | + | }) | |
| 106 | + | } | |
| 107 | + | } | |
| @@ -0,0 +1,107 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | "time" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/require" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | func TestAfterFrameQueueReleasesOnlyAfterALaterCapture(t *testing.T) { | |||
| 11 | + | tests := []struct { | |||
| 12 | + | name string | |||
| 13 | + | steps []string // "add:<id>", "capture", "emit:<n>" | |||
| 14 | + | wantRan []string | |||
| 15 | + | }{ | |||
| 16 | + | {name: "nothing emitted", steps: []string{"add:a", "capture"}}, | |||
| 17 | + | {name: "capture started before registration", steps: []string{"capture", "add:a", "capture", "emit:1"}}, | |||
| 18 | + | {name: "capture started after registration", steps: []string{"capture", "add:a", "capture", "emit:2"}, wantRan: []string{"a"}}, | |||
| 19 | + | {name: "unnumbered emission", steps: []string{"add:a", "capture", "emit:0"}}, | |||
| 20 | + | {name: "partial release keeps later work", steps: []string{"add:a", "capture", "add:b", "capture", "emit:1"}, wantRan: []string{"a"}}, | |||
| 21 | + | {name: "later emission releases the rest", steps: []string{"add:a", "capture", "add:b", "capture", "emit:1", "emit:2"}, wantRan: []string{"a", "b"}}, | |||
| 22 | + | } | |||
| 23 | + | for _, tt := range tests { | |||
| 24 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 25 | + | var q afterFrameQueue | |||
| 26 | + | ran := make(chan string, len(tt.steps)) | |||
| 27 | + | for _, step := range tt.steps { | |||
| 28 | + | switch { | |||
| 29 | + | case step == "capture": | |||
| 30 | + | q.beginCapture() | |||
| 31 | + | case step[:4] == "add:": | |||
| 32 | + | id := step[4:] | |||
| 33 | + | q.add(func() { ran <- id }) | |||
| 34 | + | case step[:5] == "emit:": | |||
| 35 | + | q.emitted(uint64(step[5] - '0')) | |||
| 36 | + | } | |||
| 37 | + | } | |||
| 38 | + | q.runReady() | |||
| 39 | + | q.runReady() | |||
| 40 | + | var got []string | |||
| 41 | + | for range tt.wantRan { | |||
| 42 | + | got = append(got, awaitTestValue(t, ran, "released work did not run")) | |||
| 43 | + | } | |||
| 44 | + | require.ElementsMatch(t, tt.wantRan, got) | |||
| 45 | + | select { | |||
| 46 | + | case extra := <-ran: | |||
| 47 | + | t.Fatalf("unexpected work %q ran", extra) | |||
| 48 | + | case <-time.After(50 * time.Millisecond): | |||
| 49 | + | } | |||
| 50 | + | }) | |||
| 51 | + | } | |||
| 52 | + | } | |||
| 53 | + | ||||
| 54 | + | // Work registered through afterNextFrame runs once a later capture is emitted | |||
| 55 | + | // on the same attachment, and is dropped once the attachment reconnected. | |||
| 56 | + | func TestAfterNextFrameRunsOrDropsByCapability(t *testing.T) { | |||
| 57 | + | tests := []struct { | |||
| 58 | + | name string | |||
| 59 | + | reconnect bool | |||
| 60 | + | freeze bool | |||
| 61 | + | wantRun bool | |||
| 62 | + | }{ | |||
| 63 | + | {name: "same attachment runs", wantRun: true}, | |||
| 64 | + | {name: "reconnected attachment drops", reconnect: true}, | |||
| 65 | + | {name: "transient freeze delays until thaw", freeze: true, wantRun: true}, | |||
| 66 | + | } | |||
| 67 | + | for _, tt := range tests { | |||
| 68 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 69 | + | d, source, ac, _, _, releases, _ := setupMovePickerSessionsCore(t, stubClock{}, 0) | |||
| 70 | + | defer releaseAll(releases) | |||
| 71 | + | effect, admitted := ac.beginAttachmentEffect(captureAttachmentCapability(source, ac, ac.transport())) | |||
| 72 | + | require.True(t, admitted) | |||
| 73 | + | ran := make(chan struct{}, 1) | |||
| 74 | + | d.queueAfterNextFrame(ac, effect, func(*attachmentEffect) { ran <- struct{}{} }) | |||
| 75 | + | effect.End() | |||
| 76 | + | if tt.reconnect { | |||
| 77 | + | ac.replaceTransport(ac.transport()) | |||
| 78 | + | ac.installTestAttachmentCapability(captureAttachmentCapability(source, ac, ac.transport())) | |||
| 79 | + | } | |||
| 80 | + | var frozen attachmentTransitionGuard | |||
| 81 | + | if tt.freeze { | |||
| 82 | + | frozen = freezeAttachmentEffectGates(ac) | |||
| 83 | + | require.True(t, frozen.acquired) | |||
| 84 | + | } | |||
| 85 | + | ac.afterFrame.emitted(ac.afterFrame.beginCapture()) | |||
| 86 | + | ac.afterFrame.runReady() | |||
| 87 | + | if tt.freeze { | |||
| 88 | + | // Released while frozen: it waits, with no further paint needed. | |||
| 89 | + | select { | |||
| 90 | + | case <-ran: | |||
| 91 | + | t.Fatal("work ran while the attachment was frozen") | |||
| 92 | + | case <-time.After(50 * time.Millisecond): | |||
| 93 | + | } | |||
| 94 | + | frozen.unfreeze() | |||
| 95 | + | } | |||
| 96 | + | if tt.wantRun { | |||
| 97 | + | awaitTestValue(t, ran, "queued work did not run after the frame") | |||
| 98 | + | return | |||
| 99 | + | } | |||
| 100 | + | select { | |||
| 101 | + | case <-ran: | |||
| 102 | + | t.Fatal("work ran for a reconnected attachment") | |||
| 103 | + | case <-time.After(100 * time.Millisecond): | |||
| 104 | + | } | |||
| 105 | + | }) | |||
| 106 | + | } | |||
| 107 | + | } | |||
M internal/usecase/daemon/client.go +1 -0
| @@ -101,6 +101,7 @@ type attachedClient struct { | |||
| 101 | 101 | renderStages renderStageHooks // optional render and handoff observability hooks | |
| 102 | 102 | linkMu sync.Mutex | |
| 103 | 103 | sendMu sync.Mutex | |
| 104 | + | afterFrame afterFrameQueue | |
| 104 | 105 | routeMu sync.RWMutex | |
| 105 | 106 | routeSnapshot protocol.RecentRouteSnapshot | |
| 106 | 107 | pendingRouteIdentity bool | |
| @@ -101,6 +101,7 @@ type attachedClient struct { | |||||
| 101 | renderStages renderStageHooks // optional render and handoff observability hooks | 101 | renderStages renderStageHooks // optional render and handoff observability hooks | ||
| 102 | linkMu sync.Mutex | 102 | linkMu sync.Mutex | ||
| 103 | sendMu sync.Mutex | 103 | sendMu sync.Mutex | ||
| 104 | + | afterFrame afterFrameQueue | |||
| 104 | routeMu sync.RWMutex | 105 | routeMu sync.RWMutex | ||
| 105 | routeSnapshot protocol.RecentRouteSnapshot | 106 | routeSnapshot protocol.RecentRouteSnapshot | ||
| 106 | pendingRouteIdentity bool | 107 | pendingRouteIdentity bool | ||
M internal/usecase/daemon/move_picker_test.go +4 -0
| @@ -137,6 +137,8 @@ func TestPaletteMovePaneCapturesSourceAndOpensPicker(t *testing.T) { | |||
| 137 | 137 | d.handleInput(source, ac, []byte("MFP\r")) | |
| 138 | 138 | require.False(t, ac.overlays.paletteActive()) | |
| 139 | 139 | ||
| 140 | + | // The picker opens after the frame erasing the palette. | |
| 141 | + | require.Eventually(t, ac.overlays.pickerClientActive, testWaitTimeout, time.Millisecond) | |
| 140 | 142 | ac.overlays.pickerMu.Lock() | |
| 141 | 143 | open := ac.overlays.pickerOpen | |
| 142 | 144 | intent, captured := ac.overlays.pickerIntent, ac.overlays.pickerMoveSource | |
| @@ -158,6 +160,8 @@ func TestPaletteMoveTabCapturesActiveTabAndOpensPicker(t *testing.T) { | |||
| 158 | 160 | d.handleInput(source, ac, []byte("\x1b ")) | |
| 159 | 161 | d.handleInput(source, ac, []byte("MAT\r")) | |
| 160 | 162 | ||
| 163 | + | // The picker opens after the frame erasing the palette. | |
| 164 | + | require.Eventually(t, ac.overlays.pickerClientActive, testWaitTimeout, time.Millisecond) | |
| 161 | 165 | ac.overlays.pickerMu.Lock() | |
| 162 | 166 | open := ac.overlays.pickerOpen | |
| 163 | 167 | intent, captured := ac.overlays.pickerIntent, ac.overlays.pickerMoveSource | |
| @@ -137,6 +137,8 @@ func TestPaletteMovePaneCapturesSourceAndOpensPicker(t *testing.T) { | |||||
| 137 | d.handleInput(source, ac, []byte("MFP\r")) | 137 | d.handleInput(source, ac, []byte("MFP\r")) | ||
| 138 | require.False(t, ac.overlays.paletteActive()) | 138 | require.False(t, ac.overlays.paletteActive()) | ||
| 139 | 139 | ||||
| 140 | + | // The picker opens after the frame erasing the palette. | |||
| 141 | + | require.Eventually(t, ac.overlays.pickerClientActive, testWaitTimeout, time.Millisecond) | |||
| 140 | ac.overlays.pickerMu.Lock() | 142 | ac.overlays.pickerMu.Lock() | ||
| 141 | open := ac.overlays.pickerOpen | 143 | open := ac.overlays.pickerOpen | ||
| 142 | intent, captured := ac.overlays.pickerIntent, ac.overlays.pickerMoveSource | 144 | intent, captured := ac.overlays.pickerIntent, ac.overlays.pickerMoveSource | ||
| @@ -158,6 +160,8 @@ func TestPaletteMoveTabCapturesActiveTabAndOpensPicker(t *testing.T) { | |||||
| 158 | d.handleInput(source, ac, []byte("\x1b ")) | 160 | d.handleInput(source, ac, []byte("\x1b ")) | ||
| 159 | d.handleInput(source, ac, []byte("MAT\r")) | 161 | d.handleInput(source, ac, []byte("MAT\r")) | ||
| 160 | 162 | ||||
| 163 | + | // The picker opens after the frame erasing the palette. | |||
| 164 | + | require.Eventually(t, ac.overlays.pickerClientActive, testWaitTimeout, time.Millisecond) | |||
| 161 | ac.overlays.pickerMu.Lock() | 165 | ac.overlays.pickerMu.Lock() | ||
| 162 | open := ac.overlays.pickerOpen | 166 | open := ac.overlays.pickerOpen | ||
| 163 | intent, captured := ac.overlays.pickerIntent, ac.overlays.pickerMoveSource | 167 | intent, captured := ac.overlays.pickerIntent, ac.overlays.pickerMoveSource | ||
M internal/usecase/daemon/palette.go +27 -2
| @@ -971,7 +971,21 @@ func (e paletteExec) openMovePicker(intent protocol.PickerIntent, source moveSou | |||
| 971 | 971 | defer fresh.End() | |
| 972 | 972 | effect = fresh | |
| 973 | 973 | } | |
| 974 | - | return e.d.openPickerForAttachment(e.ac, effect, intent, source, 0) | |
| 974 | + | if !e.redrawClosedPalette { | |
| 975 | + | return e.d.openPickerForAttachment(e.ac, effect, intent, source, 0) | |
| 976 | + | } | |
| 977 | + | // The client overlay suppresses attachment output once the first move | |
| 978 | + | // snapshot arrives, so open only after the frame erasing the palette. | |
| 979 | + | // Refusals known now still fail the command synchronously. | |
| 980 | + | if err := e.d.movePickerPrecheck(e.sess, e.ac, intent, source); err != nil { | |
| 981 | + | return err | |
| 982 | + | } | |
| 983 | + | e.d.afterNextFrame(e.sess, e.ac, effect, func(fresh *attachmentEffect) { | |
| 984 | + | if err := e.d.openPickerForAttachment(e.ac, fresh, intent, source, 0); err != nil && !errors.Is(err, errAttachmentTransition) { | |
| 985 | + | e.d.reportError(e.sess, movePickerUserError(err)) | |
| 986 | + | } | |
| 987 | + | }) | |
| 988 | + | return nil | |
| 975 | 989 | } | |
| 976 | 990 | ||
| 977 | 991 | func (e paletteExec) focus(direction layout.Direction) error { | |
| @@ -1098,7 +1112,18 @@ func (e paletteExec) OpenSessionPicker() error { | |||
| 1098 | 1112 | // over this attachment and owns its input until the user commits or | |
| 1099 | 1113 | // cancels. Detach-to-picker is reserved for attachment loss, session end, | |
| 1100 | 1114 | // and explicit detach. | |
| 1101 | - | return e.d.offerClientNavigationPicker(e.ac, effect) | |
| 1115 | + | if !e.redrawClosedPalette { | |
| 1116 | + | return e.d.offerClientNavigationPicker(e.ac, effect) | |
| 1117 | + | } | |
| 1118 | + | // The overlay suppresses attachment output, so the offer follows the | |
| 1119 | + | // frame erasing the palette. | |
| 1120 | + | e.d.afterNextFrame(e.sess, e.ac, effect, func(fresh *attachmentEffect) { | |
| 1121 | + | if err := e.d.offerClientNavigationPicker(e.ac, fresh); err != nil && !errors.Is(err, errAttachmentTransition) { | |
| 1122 | + | e.d.log.Error("palette command failed", "err", err, "command", "SSP") | |
| 1123 | + | e.d.reportError(e.sess, err) | |
| 1124 | + | } | |
| 1125 | + | }) | |
| 1126 | + | return nil | |
| 1102 | 1127 | } | |
| 1103 | 1128 | ||
| 1104 | 1129 | func (e paletteExec) OpenNotifications() error { | |
| @@ -971,7 +971,21 @@ func (e paletteExec) openMovePicker(intent protocol.PickerIntent, source moveSou | |||||
| 971 | defer fresh.End() | 971 | defer fresh.End() | ||
| 972 | effect = fresh | 972 | effect = fresh | ||
| 973 | } | 973 | } | ||
| 974 | - | return e.d.openPickerForAttachment(e.ac, effect, intent, source, 0) | 974 | + | if !e.redrawClosedPalette { |
| 975 | + | return e.d.openPickerForAttachment(e.ac, effect, intent, source, 0) | |||
| 976 | + | } | |||
| 977 | + | // The client overlay suppresses attachment output once the first move | |||
| 978 | + | // snapshot arrives, so open only after the frame erasing the palette. | |||
| 979 | + | // Refusals known now still fail the command synchronously. | |||
| 980 | + | if err := e.d.movePickerPrecheck(e.sess, e.ac, intent, source); err != nil { | |||
| 981 | + | return err | |||
| 982 | + | } | |||
| 983 | + | e.d.afterNextFrame(e.sess, e.ac, effect, func(fresh *attachmentEffect) { | |||
| 984 | + | if err := e.d.openPickerForAttachment(e.ac, fresh, intent, source, 0); err != nil && !errors.Is(err, errAttachmentTransition) { | |||
| 985 | + | e.d.reportError(e.sess, movePickerUserError(err)) | |||
| 986 | + | } | |||
| 987 | + | }) | |||
| 988 | + | return nil | |||
| 975 | } | 989 | } | ||
| 976 | 990 | ||||
| 977 | func (e paletteExec) focus(direction layout.Direction) error { | 991 | func (e paletteExec) focus(direction layout.Direction) error { | ||
| @@ -1098,7 +1112,18 @@ func (e paletteExec) OpenSessionPicker() error { | |||||
| 1098 | // over this attachment and owns its input until the user commits or | 1112 | // over this attachment and owns its input until the user commits or | ||
| 1099 | // cancels. Detach-to-picker is reserved for attachment loss, session end, | 1113 | // cancels. Detach-to-picker is reserved for attachment loss, session end, | ||
| 1100 | // and explicit detach. | 1114 | // and explicit detach. | ||
| 1101 | - | return e.d.offerClientNavigationPicker(e.ac, effect) | 1115 | + | if !e.redrawClosedPalette { |
| 1116 | + | return e.d.offerClientNavigationPicker(e.ac, effect) | |||
| 1117 | + | } | |||
| 1118 | + | // The overlay suppresses attachment output, so the offer follows the | |||
| 1119 | + | // frame erasing the palette. | |||
| 1120 | + | e.d.afterNextFrame(e.sess, e.ac, effect, func(fresh *attachmentEffect) { | |||
| 1121 | + | if err := e.d.offerClientNavigationPicker(e.ac, fresh); err != nil && !errors.Is(err, errAttachmentTransition) { | |||
| 1122 | + | e.d.log.Error("palette command failed", "err", err, "command", "SSP") | |||
| 1123 | + | e.d.reportError(e.sess, err) | |||
| 1124 | + | } | |||
| 1125 | + | }) | |||
| 1126 | + | return nil | |||
| 1102 | } | 1127 | } | ||
| 1103 | 1128 | ||||
| 1104 | func (e paletteExec) OpenNotifications() error { | 1129 | func (e paletteExec) OpenNotifications() error { | ||
M internal/usecase/daemon/picker_interaction.go +15 -0
| @@ -201,6 +201,21 @@ func (d *Daemon) openPickerForAttachment(ac *attachedClient, effect *attachmentE | |||
| 201 | 201 | return nil | |
| 202 | 202 | } | |
| 203 | 203 | ||
| 204 | + | // movePickerPrecheck reports the move-picker refusals that are known before | |
| 205 | + | // the picker opens, so a deferred open still fails its command synchronously. | |
| 206 | + | // openPickerForAttachment re-checks both when it actually opens. | |
| 207 | + | func (d *Daemon) movePickerPrecheck(sess *session, ac *attachedClient, intent protocol.PickerIntent, source moveSourceLocator) error { | |
| 208 | + | if sess == nil || !sess.capabilities().yieldsMoves() { | |
| 209 | + | return errSessionCannotYieldMoves | |
| 210 | + | } | |
| 211 | + | _, groupedViews, current := d.pickerViewProjections(sess, ac) | |
| 212 | + | filter := pickerSourceFilter{Session: source.Session.ID, Incarnation: source.Session.Incarnation, TabID: source.TabID} | |
| 213 | + | if !pickerLineSetHasMove(pickerLineSetFor(groupedViews, intent, filter, current)) { | |
| 214 | + | return errNoMoveDestination | |
| 215 | + | } | |
| 216 | + | return nil | |
| 217 | + | } | |
| 218 | + | ||
| 204 | 219 | // pickerMoveSourceKey names the captured move source in the offer. It is | |
| 205 | 220 | // opaque and never a route: only the daemon resolves it, and only for the | |
| 206 | 221 | // interaction that captured it. | |
| @@ -201,6 +201,21 @@ func (d *Daemon) openPickerForAttachment(ac *attachedClient, effect *attachmentE | |||||
| 201 | return nil | 201 | return nil | ||
| 202 | } | 202 | } | ||
| 203 | 203 | ||||
| 204 | + | // movePickerPrecheck reports the move-picker refusals that are known before | |||
| 205 | + | // the picker opens, so a deferred open still fails its command synchronously. | |||
| 206 | + | // openPickerForAttachment re-checks both when it actually opens. | |||
| 207 | + | func (d *Daemon) movePickerPrecheck(sess *session, ac *attachedClient, intent protocol.PickerIntent, source moveSourceLocator) error { | |||
| 208 | + | if sess == nil || !sess.capabilities().yieldsMoves() { | |||
| 209 | + | return errSessionCannotYieldMoves | |||
| 210 | + | } | |||
| 211 | + | _, groupedViews, current := d.pickerViewProjections(sess, ac) | |||
| 212 | + | filter := pickerSourceFilter{Session: source.Session.ID, Incarnation: source.Session.Incarnation, TabID: source.TabID} | |||
| 213 | + | if !pickerLineSetHasMove(pickerLineSetFor(groupedViews, intent, filter, current)) { | |||
| 214 | + | return errNoMoveDestination | |||
| 215 | + | } | |||
| 216 | + | return nil | |||
| 217 | + | } | |||
| 218 | + | ||||
| 204 | // pickerMoveSourceKey names the captured move source in the offer. It is | 219 | // pickerMoveSourceKey names the captured move source in the offer. It is | ||
| 205 | // opaque and never a route: only the daemon resolves it, and only for the | 220 | // opaque and never a route: only the daemon resolves it, and only for the | ||
| 206 | // interaction that captured it. | 221 | // interaction that captured it. | ||
M internal/usecase/daemon/render.go +3 -0
| @@ -389,6 +389,7 @@ func (d *Daemon) paintWithActivationEffect(entry *session, ac *attachedClient, r | |||
| 389 | 389 | // under that same ownership so concurrent fallback paints cannot observe a | |
| 390 | 390 | // partially published runtime. | |
| 391 | 391 | ac.initOverlays() | |
| 392 | + | frameCapture := ac.afterFrame.beginCapture() | |
| 392 | 393 | overlays := ac.overlays.SnapshotForRender() | |
| 393 | 394 | repaintAttachedClients := false | |
| 394 | 395 | defer func() { | |
| @@ -397,6 +398,7 @@ func (d *Daemon) paintWithActivationEffect(entry *session, ac *attachedClient, r | |||
| 397 | 398 | // Emit the captured sequence before any follow-up repaint can introduce | |
| 398 | 399 | // another boundary, while no attachment, session, tab, or pane lock is held. | |
| 399 | 400 | marks.flush() | |
| 401 | + | ac.afterFrame.runReady() | |
| 400 | 402 | if repaintAttachedClients { | |
| 401 | 403 | d.repaintAllAttachedClients() | |
| 402 | 404 | } | |
| @@ -464,6 +466,7 @@ func (d *Daemon) paintWithActivationEffect(entry *session, ac *attachedClient, r | |||
| 464 | 466 | if ac.renderStages.capture != nil { | |
| 465 | 467 | ac.renderStages.capture() | |
| 466 | 468 | } | |
| 469 | + | state.frameCapture = frameCapture | |
| 467 | 470 | captureOverlayLayers(state, overlays, paletteCfg) | |
| 468 | 471 | endCompose := marks.span(ports.RuntimeComposeStart, ports.RuntimeComposeEnd, 0) | |
| 469 | 472 | composed := composeFrame(*state, ac.pipelineCache, ac.pipelineScratch) | |
| @@ -389,6 +389,7 @@ func (d *Daemon) paintWithActivationEffect(entry *session, ac *attachedClient, r | |||||
| 389 | // under that same ownership so concurrent fallback paints cannot observe a | 389 | // under that same ownership so concurrent fallback paints cannot observe a | ||
| 390 | // partially published runtime. | 390 | // partially published runtime. | ||
| 391 | ac.initOverlays() | 391 | ac.initOverlays() | ||
| 392 | + | frameCapture := ac.afterFrame.beginCapture() | |||
| 392 | overlays := ac.overlays.SnapshotForRender() | 393 | overlays := ac.overlays.SnapshotForRender() | ||
| 393 | repaintAttachedClients := false | 394 | repaintAttachedClients := false | ||
| 394 | defer func() { | 395 | defer func() { | ||
| @@ -397,6 +398,7 @@ func (d *Daemon) paintWithActivationEffect(entry *session, ac *attachedClient, r | |||||
| 397 | // Emit the captured sequence before any follow-up repaint can introduce | 398 | // Emit the captured sequence before any follow-up repaint can introduce | ||
| 398 | // another boundary, while no attachment, session, tab, or pane lock is held. | 399 | // another boundary, while no attachment, session, tab, or pane lock is held. | ||
| 399 | marks.flush() | 400 | marks.flush() | ||
| 401 | + | ac.afterFrame.runReady() | |||
| 400 | if repaintAttachedClients { | 402 | if repaintAttachedClients { | ||
| 401 | d.repaintAllAttachedClients() | 403 | d.repaintAllAttachedClients() | ||
| 402 | } | 404 | } | ||
| @@ -464,6 +466,7 @@ func (d *Daemon) paintWithActivationEffect(entry *session, ac *attachedClient, r | |||||
| 464 | if ac.renderStages.capture != nil { | 466 | if ac.renderStages.capture != nil { | ||
| 465 | ac.renderStages.capture() | 467 | ac.renderStages.capture() | ||
| 466 | } | 468 | } | ||
| 469 | + | state.frameCapture = frameCapture | |||
| 467 | captureOverlayLayers(state, overlays, paletteCfg) | 470 | captureOverlayLayers(state, overlays, paletteCfg) | ||
| 468 | endCompose := marks.span(ports.RuntimeComposeStart, ports.RuntimeComposeEnd, 0) | 471 | endCompose := marks.span(ports.RuntimeComposeStart, ports.RuntimeComposeEnd, 0) | ||
| 469 | composed := composeFrame(*state, ac.pipelineCache, ac.pipelineScratch) | 472 | composed := composeFrame(*state, ac.pipelineCache, ac.pipelineScratch) | ||
M internal/usecase/daemon/render_capture.go +1 -0
| @@ -48,6 +48,7 @@ type capturedRenderState struct { | |||
| 48 | 48 | incarnation domain.IncarnationID | |
| 49 | 49 | lease *attachmentLease | |
| 50 | 50 | uiFence uint64 | |
| 51 | + | frameCapture uint64 // numbers this capture in the attachment's afterFrame queue | |
| 51 | 52 | view attachmentView | |
| 52 | 53 | focusedPaneID domain.PaneStableID | |
| 53 | 54 | window domain.Size | |
| @@ -48,6 +48,7 @@ type capturedRenderState struct { | |||||
| 48 | incarnation domain.IncarnationID | 48 | incarnation domain.IncarnationID | ||
| 49 | lease *attachmentLease | 49 | lease *attachmentLease | ||
| 50 | uiFence uint64 | 50 | uiFence uint64 | ||
| 51 | + | frameCapture uint64 // numbers this capture in the attachment's afterFrame queue | |||
| 51 | view attachmentView | 52 | view attachmentView | ||
| 52 | focusedPaneID domain.PaneStableID | 53 | focusedPaneID domain.PaneStableID | ||
| 53 | window domain.Size | 54 | window domain.Size | ||
M internal/usecase/daemon/render_pipeline.go +1 -0
| @@ -574,6 +574,7 @@ func (d *Daemon) emitFrame(entry *session, ac *attachedClient, state *capturedRe | |||
| 574 | 574 | return true | |
| 575 | 575 | } | |
| 576 | 576 | } | |
| 577 | + | ac.afterFrame.emitted(state.frameCapture) | |
| 577 | 578 | // Publish only after output preparation and transport emission both | |
| 578 | 579 | // succeed. A cross-session transition may publish concurrently, but its | |
| 579 | 580 | // mandatory first-paint rebase waits for sendMu and therefore follows this | |
| @@ -574,6 +574,7 @@ func (d *Daemon) emitFrame(entry *session, ac *attachedClient, state *capturedRe | |||||
| 574 | return true | 574 | return true | ||
| 575 | } | 575 | } | ||
| 576 | } | 576 | } | ||
| 577 | + | ac.afterFrame.emitted(state.frameCapture) | |||
| 577 | // Publish only after output preparation and transport emission both | 578 | // Publish only after output preparation and transport emission both | ||
| 578 | // succeed. A cross-session transition may publish concurrently, but its | 579 | // succeed. A cross-session transition may publish concurrently, but its | ||
| 579 | // mandatory first-paint rebase waits for sendMu and therefore follows this | 580 | // mandatory first-paint rebase waits for sendMu and therefore follows this | ||
M internal/usecase/daemon/session_picker_offer_test.go +102 -0
| @@ -50,3 +50,105 @@ func TestSessionPickerCommandOffersClientPickerOverLiveAttachment(t *testing.T) | |||
| 50 | 50 | require.Same(t, source, ac.currentAttachmentSession(), "SSP must keep the attachment live") | |
| 51 | 51 | assertNoFurtherFrame(t, sends, "Detached", "SSP must not detach the attachment") | |
| 52 | 52 | } | |
| 53 | + | ||
| 54 | + | // TestPaletteClientPickerFollowsPaletteErasingFrame pins the ordering the | |
| 55 | + | // client overlay depends on: it suppresses attachment output once the picker | |
| 56 | + | // opens, so the frame erasing the palette must reach it before the offer (SSP) | |
| 57 | + | // or the first move snapshot (MFP), even while output is ACK-blocked. | |
| 58 | + | func TestPaletteClientPickerFollowsPaletteErasingFrame(t *testing.T) { | |
| 59 | + | tests := []struct { | |
| 60 | + | name string | |
| 61 | + | command string | |
| 62 | + | saturated bool | |
| 63 | + | opens func(protocol.ServerMessage) bool | |
| 64 | + | }{ | |
| 65 | + | {name: "session picker", command: "SSP", opens: isPickerOffer}, | |
| 66 | + | {name: "session picker with full output window", command: "SSP", saturated: true, opens: isPickerOffer}, | |
| 67 | + | {name: "move picker", command: "MFP", opens: isPickerSnapshot}, | |
| 68 | + | {name: "move picker with full output window", command: "MFP", saturated: true, opens: isPickerSnapshot}, | |
| 69 | + | } | |
| 70 | + | for _, tt := range tests { | |
| 71 | + | t.Run(tt.name, func(t *testing.T) { | |
| 72 | + | d, source, ac, _, _, releases, sends := setupMovePickerSessionsCore(t, stubClock{}, 0) | |
| 73 | + | defer releaseAll(releases) | |
| 74 | + | // Production paints through the coordinator, which retries an | |
| 75 | + | // ACK-blocked paint once the client acknowledges. | |
| 76 | + | rc := d.attachCoordinator(source, nil, ac, true) | |
| 77 | + | t.Cleanup(func() { rc.beginSessionTeardown().finish(); rc.waitForTimerWorkers() }) | |
| 78 | + | d.handleInput(source, ac, []byte("\x1b ")) | |
| 79 | + | d.handleInput(source, ac, []byte(tt.command)) | |
| 80 | + | awaitFrame(t, sends, "Output") | |
| 81 | + | var blocking protocol.Output | |
| 82 | + | for _, frame := range drainAllFrames(sends) { | |
| 83 | + | if output, ok := decodeServerMessage(t, frame).(protocol.Output); ok { | |
| 84 | + | blocking = output | |
| 85 | + | } | |
| 86 | + | } | |
| 87 | + | if tt.saturated { | |
| 88 | + | // The palette frame is still unacknowledged: a one-frame window | |
| 89 | + | // blocks every further paint until the client ACKs it. | |
| 90 | + | require.NotZero(t, blocking.New) | |
| 91 | + | ac.output.setWindow(1) | |
| 92 | + | require.True(t, ac.output.atCapacity()) | |
| 93 | + | } | |
| 94 | + | ||
| 95 | + | d.handleInput(source, ac, []byte("\r")) | |
| 96 | + | require.False(t, ac.overlays.paletteActive()) | |
| 97 | + | if tt.saturated { | |
| 98 | + | // The work runs asynchronously once released, so give a wrong | |
| 99 | + | // early release time to reach the wire before asserting. | |
| 100 | + | assertNoSentServerMessage(t, sends, 200*time.Millisecond, tt.opens, "picker opened before the palette-erasing frame could be sent") | |
| 101 | + | d.handleAttachmentClientMessage(captureAttachmentCapability(source, ac, ac.transport()), protocol.Ack{Epoch: blocking.Epoch, State: blocking.New}) | |
| 102 | + | } | |
| 103 | + | ||
| 104 | + | first := awaitSentServerMessage(t, sends, "Output or picker", func(message protocol.ServerMessage) bool { | |
| 105 | + | _, output := message.(protocol.Output) | |
| 106 | + | return output || tt.opens(message) | |
| 107 | + | }) | |
| 108 | + | require.IsType(t, protocol.Output{}, first, "the palette-erasing frame must precede the picker") | |
| 109 | + | awaitSentServerMessage(t, sends, "picker open", tt.opens) | |
| 110 | + | }) | |
| 111 | + | } | |
| 112 | + | } | |
| 113 | + | ||
| 114 | + | func isPickerOffer(message protocol.ServerMessage) bool { | |
| 115 | + | _, ok := message.(protocol.PickerOffer) | |
| 116 | + | return ok | |
| 117 | + | } | |
| 118 | + | ||
| 119 | + | func isPickerSnapshot(message protocol.ServerMessage) bool { | |
| 120 | + | _, ok := message.(protocol.PickerSnapshot) | |
| 121 | + | return ok | |
| 122 | + | } | |
| 123 | + | ||
| 124 | + | // assertNoSentServerMessage fails if a matching message is sent within wait. | |
| 125 | + | func assertNoSentServerMessage(t *testing.T, sends <-chan wire.Envelope, wait time.Duration, match func(protocol.ServerMessage) bool, failure string) { | |
| 126 | + | t.Helper() | |
| 127 | + | deadline := time.After(wait) | |
| 128 | + | for { | |
| 129 | + | select { | |
| 130 | + | case frame := <-sends: | |
| 131 | + | require.False(t, match(decodeServerMessage(t, frame)), failure) | |
| 132 | + | case <-deadline: | |
| 133 | + | return | |
| 134 | + | } | |
| 135 | + | } | |
| 136 | + | } | |
| 137 | + | ||
| 138 | + | // A scripted CommandRequest has no palette to erase: its refusals stay | |
| 139 | + | // synchronous command results instead of a false success. | |
| 140 | + | func TestCommandRequestMovePickerReportsRefusalSynchronously(t *testing.T) { | |
| 141 | + | d, source, ac, _, _, releases, sends := setupMovePickerSessionsCore(t, stubClock{}, 0) | |
| 142 | + | defer releaseAll(releases) | |
| 143 | + | d.mu.Lock() | |
| 144 | + | delete(d.sessions, "destination") | |
| 145 | + | d.mu.Unlock() | |
| 146 | + | token := captureAttachmentCapability(source, ac, ac.transport()) | |
| 147 | + | require.False(t, d.handleAttachmentClientMessage(token, protocol.CommandRequest{Version: protocol.Version, RequestID: 5, Attached: true, Slug: "move-pane"})) | |
| 148 | + | result := awaitSentServerMessage(t, sends, "CommandResult", func(message protocol.ServerMessage) bool { | |
| 149 | + | _, ok := message.(protocol.CommandResult) | |
| 150 | + | return ok | |
| 151 | + | }).(protocol.CommandResult) | |
| 152 | + | require.NotEqual(t, protocol.CommandSucceeded, result.Outcome, "a refused move must not report success") | |
| 153 | + | require.False(t, ac.overlays.pickerClientActive()) | |
| 154 | + | } | |
| @@ -50,3 +50,105 @@ func TestSessionPickerCommandOffersClientPickerOverLiveAttachment(t *testing.T) | |||||
| 50 | require.Same(t, source, ac.currentAttachmentSession(), "SSP must keep the attachment live") | 50 | require.Same(t, source, ac.currentAttachmentSession(), "SSP must keep the attachment live") | ||
| 51 | assertNoFurtherFrame(t, sends, "Detached", "SSP must not detach the attachment") | 51 | assertNoFurtherFrame(t, sends, "Detached", "SSP must not detach the attachment") | ||
| 52 | } | 52 | } | ||
| 53 | + | ||||
| 54 | + | // TestPaletteClientPickerFollowsPaletteErasingFrame pins the ordering the | |||
| 55 | + | // client overlay depends on: it suppresses attachment output once the picker | |||
| 56 | + | // opens, so the frame erasing the palette must reach it before the offer (SSP) | |||
| 57 | + | // or the first move snapshot (MFP), even while output is ACK-blocked. | |||
| 58 | + | func TestPaletteClientPickerFollowsPaletteErasingFrame(t *testing.T) { | |||
| 59 | + | tests := []struct { | |||
| 60 | + | name string | |||
| 61 | + | command string | |||
| 62 | + | saturated bool | |||
| 63 | + | opens func(protocol.ServerMessage) bool | |||
| 64 | + | }{ | |||
| 65 | + | {name: "session picker", command: "SSP", opens: isPickerOffer}, | |||
| 66 | + | {name: "session picker with full output window", command: "SSP", saturated: true, opens: isPickerOffer}, | |||
| 67 | + | {name: "move picker", command: "MFP", opens: isPickerSnapshot}, | |||
| 68 | + | {name: "move picker with full output window", command: "MFP", saturated: true, opens: isPickerSnapshot}, | |||
| 69 | + | } | |||
| 70 | + | for _, tt := range tests { | |||
| 71 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 72 | + | d, source, ac, _, _, releases, sends := setupMovePickerSessionsCore(t, stubClock{}, 0) | |||
| 73 | + | defer releaseAll(releases) | |||
| 74 | + | // Production paints through the coordinator, which retries an | |||
| 75 | + | // ACK-blocked paint once the client acknowledges. | |||
| 76 | + | rc := d.attachCoordinator(source, nil, ac, true) | |||
| 77 | + | t.Cleanup(func() { rc.beginSessionTeardown().finish(); rc.waitForTimerWorkers() }) | |||
| 78 | + | d.handleInput(source, ac, []byte("\x1b ")) | |||
| 79 | + | d.handleInput(source, ac, []byte(tt.command)) | |||
| 80 | + | awaitFrame(t, sends, "Output") | |||
| 81 | + | var blocking protocol.Output | |||
| 82 | + | for _, frame := range drainAllFrames(sends) { | |||
| 83 | + | if output, ok := decodeServerMessage(t, frame).(protocol.Output); ok { | |||
| 84 | + | blocking = output | |||
| 85 | + | } | |||
| 86 | + | } | |||
| 87 | + | if tt.saturated { | |||
| 88 | + | // The palette frame is still unacknowledged: a one-frame window | |||
| 89 | + | // blocks every further paint until the client ACKs it. | |||
| 90 | + | require.NotZero(t, blocking.New) | |||
| 91 | + | ac.output.setWindow(1) | |||
| 92 | + | require.True(t, ac.output.atCapacity()) | |||
| 93 | + | } | |||
| 94 | + | ||||
| 95 | + | d.handleInput(source, ac, []byte("\r")) | |||
| 96 | + | require.False(t, ac.overlays.paletteActive()) | |||
| 97 | + | if tt.saturated { | |||
| 98 | + | // The work runs asynchronously once released, so give a wrong | |||
| 99 | + | // early release time to reach the wire before asserting. | |||
| 100 | + | assertNoSentServerMessage(t, sends, 200*time.Millisecond, tt.opens, "picker opened before the palette-erasing frame could be sent") | |||
| 101 | + | d.handleAttachmentClientMessage(captureAttachmentCapability(source, ac, ac.transport()), protocol.Ack{Epoch: blocking.Epoch, State: blocking.New}) | |||
| 102 | + | } | |||
| 103 | + | ||||
| 104 | + | first := awaitSentServerMessage(t, sends, "Output or picker", func(message protocol.ServerMessage) bool { | |||
| 105 | + | _, output := message.(protocol.Output) | |||
| 106 | + | return output || tt.opens(message) | |||
| 107 | + | }) | |||
| 108 | + | require.IsType(t, protocol.Output{}, first, "the palette-erasing frame must precede the picker") | |||
| 109 | + | awaitSentServerMessage(t, sends, "picker open", tt.opens) | |||
| 110 | + | }) | |||
| 111 | + | } | |||
| 112 | + | } | |||
| 113 | + | ||||
| 114 | + | func isPickerOffer(message protocol.ServerMessage) bool { | |||
| 115 | + | _, ok := message.(protocol.PickerOffer) | |||
| 116 | + | return ok | |||
| 117 | + | } | |||
| 118 | + | ||||
| 119 | + | func isPickerSnapshot(message protocol.ServerMessage) bool { | |||
| 120 | + | _, ok := message.(protocol.PickerSnapshot) | |||
| 121 | + | return ok | |||
| 122 | + | } | |||
| 123 | + | ||||
| 124 | + | // assertNoSentServerMessage fails if a matching message is sent within wait. | |||
| 125 | + | func assertNoSentServerMessage(t *testing.T, sends <-chan wire.Envelope, wait time.Duration, match func(protocol.ServerMessage) bool, failure string) { | |||
| 126 | + | t.Helper() | |||
| 127 | + | deadline := time.After(wait) | |||
| 128 | + | for { | |||
| 129 | + | select { | |||
| 130 | + | case frame := <-sends: | |||
| 131 | + | require.False(t, match(decodeServerMessage(t, frame)), failure) | |||
| 132 | + | case <-deadline: | |||
| 133 | + | return | |||
| 134 | + | } | |||
| 135 | + | } | |||
| 136 | + | } | |||
| 137 | + | ||||
| 138 | + | // A scripted CommandRequest has no palette to erase: its refusals stay | |||
| 139 | + | // synchronous command results instead of a false success. | |||
| 140 | + | func TestCommandRequestMovePickerReportsRefusalSynchronously(t *testing.T) { | |||
| 141 | + | d, source, ac, _, _, releases, sends := setupMovePickerSessionsCore(t, stubClock{}, 0) | |||
| 142 | + | defer releaseAll(releases) | |||
| 143 | + | d.mu.Lock() | |||
| 144 | + | delete(d.sessions, "destination") | |||
| 145 | + | d.mu.Unlock() | |||
| 146 | + | token := captureAttachmentCapability(source, ac, ac.transport()) | |||
| 147 | + | require.False(t, d.handleAttachmentClientMessage(token, protocol.CommandRequest{Version: protocol.Version, RequestID: 5, Attached: true, Slug: "move-pane"})) | |||
| 148 | + | result := awaitSentServerMessage(t, sends, "CommandResult", func(message protocol.ServerMessage) bool { | |||
| 149 | + | _, ok := message.(protocol.CommandResult) | |||
| 150 | + | return ok | |||
| 151 | + | }).(protocol.CommandResult) | |||
| 152 | + | require.NotEqual(t, protocol.CommandSucceeded, result.Outcome, "a refused move must not report success") | |||
| 153 | + | require.False(t, ac.overlays.pickerClientActive()) | |||
| 154 | + | } | |||