fix(daemon): apply proxy screen review fixes
- commit
- 15846e648b6cef584e71b9ccaffb84bb51b76b35
- author
- brice · · 2026-08-02 10:48 UTC
- parent
- 4634437
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
14 files · +144 -83
M internal/usecase/daemon/output_state_stream.go +5 -14
| @@ -38,7 +38,7 @@ type preparedOutput struct { | |||
| 38 | 38 | next uint64 | |
| 39 | 39 | data []byte | |
| 40 | 40 | reset bool | |
| 41 | - | completed bool | |
| 41 | + | attempted bool | |
| 42 | 42 | } | |
| 43 | 43 | ||
| 44 | 44 | func (s *outputStateStream) prepare(frame renderer.Frame, damage []renderer.Damage, reset bool) (*preparedOutput, error) { | |
| @@ -57,10 +57,10 @@ func (s *outputStateStream) prepare(frame renderer.Frame, damage []renderer.Dama | |||
| 57 | 57 | } | |
| 58 | 58 | ||
| 59 | 59 | func (p *preparedOutput) send(data []byte, echoAck uint64, send func(ports.Frame) error) error { | |
| 60 | - | if p.completed { | |
| 60 | + | if p.attempted { | |
| 61 | 61 | return nil | |
| 62 | 62 | } | |
| 63 | - | p.completed = true | |
| 63 | + | p.attempted = true | |
| 64 | 64 | base := p.stream.next | |
| 65 | 65 | if p.reset { | |
| 66 | 66 | base = 0 | |
| @@ -76,10 +76,10 @@ func (p *preparedOutput) send(data []byte, echoAck uint64, send func(ports.Frame | |||
| 76 | 76 | } | |
| 77 | 77 | ||
| 78 | 78 | func (p *preparedOutput) commitNoSend() { | |
| 79 | - | if p.completed { | |
| 79 | + | if p == nil || p.attempted || len(p.data) != 0 { | |
| 80 | 80 | return | |
| 81 | 81 | } | |
| 82 | - | p.completed = true | |
| 82 | + | p.attempted = true | |
| 83 | 83 | p.commit() | |
| 84 | 84 | } | |
| 85 | 85 | ||
| @@ -88,15 +88,6 @@ func (p *preparedOutput) commit() { | |||
| 88 | 88 | p.stream.forceSnapshot = false | |
| 89 | 89 | } | |
| 90 | 90 | ||
| 91 | - | func (s *outputStateStream) frame(data []byte, reset bool, echoAck uint64) ports.Frame { | |
| 92 | - | s.next++ | |
| 93 | - | base := s.next - 1 | |
| 94 | - | if reset { | |
| 95 | - | base = 0 | |
| 96 | - | } | |
| 97 | - | return frameOutputState(data, base, s.next, echoAck) | |
| 98 | - | } | |
| 99 | - | ||
| 100 | 91 | func (s *outputStateStream) sideEffect(data []byte, echoAck uint64) ports.Frame { | |
| 101 | 92 | return frameOutputState(data, 0, 0, echoAck) | |
| 102 | 93 | } | |
| @@ -38,7 +38,7 @@ type preparedOutput struct { | |||||
| 38 | next uint64 | 38 | next uint64 | ||
| 39 | data []byte | 39 | data []byte | ||
| 40 | reset bool | 40 | reset bool | ||
| 41 | - | completed bool | 41 | + | attempted bool |
| 42 | } | 42 | } | ||
| 43 | 43 | ||||
| 44 | func (s *outputStateStream) prepare(frame renderer.Frame, damage []renderer.Damage, reset bool) (*preparedOutput, error) { | 44 | func (s *outputStateStream) prepare(frame renderer.Frame, damage []renderer.Damage, reset bool) (*preparedOutput, error) { | ||
| @@ -57,10 +57,10 @@ func (s *outputStateStream) prepare(frame renderer.Frame, damage []renderer.Dama | |||||
| 57 | } | 57 | } | ||
| 58 | 58 | ||||
| 59 | func (p *preparedOutput) send(data []byte, echoAck uint64, send func(ports.Frame) error) error { | 59 | func (p *preparedOutput) send(data []byte, echoAck uint64, send func(ports.Frame) error) error { | ||
| 60 | - | if p.completed { | 60 | + | if p.attempted { |
| 61 | return nil | 61 | return nil | ||
| 62 | } | 62 | } | ||
| 63 | - | p.completed = true | 63 | + | p.attempted = true |
| 64 | base := p.stream.next | 64 | base := p.stream.next | ||
| 65 | if p.reset { | 65 | if p.reset { | ||
| 66 | base = 0 | 66 | base = 0 | ||
| @@ -76,10 +76,10 @@ func (p *preparedOutput) send(data []byte, echoAck uint64, send func(ports.Frame | |||||
| 76 | } | 76 | } | ||
| 77 | 77 | ||||
| 78 | func (p *preparedOutput) commitNoSend() { | 78 | func (p *preparedOutput) commitNoSend() { | ||
| 79 | - | if p.completed { | 79 | + | if p == nil || p.attempted || len(p.data) != 0 { |
| 80 | return | 80 | return | ||
| 81 | } | 81 | } | ||
| 82 | - | p.completed = true | 82 | + | p.attempted = true |
| 83 | p.commit() | 83 | p.commit() | ||
| 84 | } | 84 | } | ||
| 85 | 85 | ||||
| @@ -88,15 +88,6 @@ func (p *preparedOutput) commit() { | |||||
| 88 | p.stream.forceSnapshot = false | 88 | p.stream.forceSnapshot = false | ||
| 89 | } | 89 | } | ||
| 90 | 90 | ||||
| 91 | - | func (s *outputStateStream) frame(data []byte, reset bool, echoAck uint64) ports.Frame { | |||
| 92 | - | s.next++ | |||
| 93 | - | base := s.next - 1 | |||
| 94 | - | if reset { | |||
| 95 | - | base = 0 | |||
| 96 | - | } | |||
| 97 | - | return frameOutputState(data, base, s.next, echoAck) | |||
| 98 | - | } | |||
| 99 | - | ||||
| 100 | func (s *outputStateStream) sideEffect(data []byte, echoAck uint64) ports.Frame { | 91 | func (s *outputStateStream) sideEffect(data []byte, echoAck uint64) ports.Frame { | ||
| 101 | return frameOutputState(data, 0, 0, echoAck) | 92 | return frameOutputState(data, 0, 0, echoAck) | ||
| 102 | } | 93 | } | ||
M internal/usecase/daemon/output_state_stream_test.go +31 -4
| @@ -141,17 +141,44 @@ func (s *outputStateStream) render(frame renderer.Frame, damage []renderer.Damag | |||
| 141 | 141 | if err != nil { | |
| 142 | 142 | return nil, err | |
| 143 | 143 | } | |
| 144 | - | prepared.commitNoSend() | |
| 144 | + | if len(prepared.data) == 0 { | |
| 145 | + | prepared.commitNoSend() | |
| 146 | + | return nil, nil | |
| 147 | + | } | |
| 148 | + | // This helper tests renderer replay bytes without publishing a wire frame. | |
| 149 | + | // Production state advancement happens only through preparedOutput.send. | |
| 150 | + | prepared.draw.Commit() | |
| 151 | + | s.forceSnapshot = false | |
| 145 | 152 | return prepared.data, nil | |
| 146 | 153 | } | |
| 147 | 154 | ||
| 155 | + | func outputStateFrame(stream *outputStateStream, data []byte, reset bool, echoAck uint64) ports.Frame { | |
| 156 | + | stream.next++ | |
| 157 | + | base := stream.next - 1 | |
| 158 | + | if reset { | |
| 159 | + | base = 0 | |
| 160 | + | } | |
| 161 | + | return frameOutputState(data, base, stream.next, echoAck) | |
| 162 | + | } | |
| 163 | + | ||
| 148 | 164 | func drawOutputState(t *testing.T, stream *outputStateStream, frame renderer.Frame, damage []renderer.Damage, reset bool, echoAck uint64) (ports.Frame, bool, error) { | |
| 149 | 165 | t.Helper() | |
| 150 | - | data, err := stream.render(frame, damage, reset) | |
| 151 | - | if err != nil || len(data) == 0 { | |
| 166 | + | prepared, err := stream.prepare(frame, damage, reset) | |
| 167 | + | if err != nil || len(prepared.data) == 0 { | |
| 168 | + | if prepared != nil { | |
| 169 | + | prepared.commitNoSend() | |
| 170 | + | } | |
| 171 | + | return ports.Frame{}, false, err | |
| 172 | + | } | |
| 173 | + | var output ports.Frame | |
| 174 | + | err = prepared.send(prepared.data, echoAck, func(frame ports.Frame) error { | |
| 175 | + | output = frame | |
| 176 | + | return nil | |
| 177 | + | }) | |
| 178 | + | if err != nil { | |
| 152 | 179 | return ports.Frame{}, false, err | |
| 153 | 180 | } | |
| 154 | - | return stream.frame(data, reset, echoAck), true, nil | |
| 181 | + | return output, true, nil | |
| 155 | 182 | } | |
| 156 | 183 | ||
| 157 | 184 | // A resize is a state reset, but it must not turn subsequent idle or damaged | |
| @@ -141,17 +141,44 @@ func (s *outputStateStream) render(frame renderer.Frame, damage []renderer.Damag | |||||
| 141 | if err != nil { | 141 | if err != nil { | ||
| 142 | return nil, err | 142 | return nil, err | ||
| 143 | } | 143 | } | ||
| 144 | - | prepared.commitNoSend() | 144 | + | if len(prepared.data) == 0 { |
| 145 | + | prepared.commitNoSend() | |||
| 146 | + | return nil, nil | |||
| 147 | + | } | |||
| 148 | + | // This helper tests renderer replay bytes without publishing a wire frame. | |||
| 149 | + | // Production state advancement happens only through preparedOutput.send. | |||
| 150 | + | prepared.draw.Commit() | |||
| 151 | + | s.forceSnapshot = false | |||
| 145 | return prepared.data, nil | 152 | return prepared.data, nil | ||
| 146 | } | 153 | } | ||
| 147 | 154 | ||||
| 155 | + | func outputStateFrame(stream *outputStateStream, data []byte, reset bool, echoAck uint64) ports.Frame { | |||
| 156 | + | stream.next++ | |||
| 157 | + | base := stream.next - 1 | |||
| 158 | + | if reset { | |||
| 159 | + | base = 0 | |||
| 160 | + | } | |||
| 161 | + | return frameOutputState(data, base, stream.next, echoAck) | |||
| 162 | + | } | |||
| 163 | + | ||||
| 148 | func drawOutputState(t *testing.T, stream *outputStateStream, frame renderer.Frame, damage []renderer.Damage, reset bool, echoAck uint64) (ports.Frame, bool, error) { | 164 | func drawOutputState(t *testing.T, stream *outputStateStream, frame renderer.Frame, damage []renderer.Damage, reset bool, echoAck uint64) (ports.Frame, bool, error) { | ||
| 149 | t.Helper() | 165 | t.Helper() | ||
| 150 | - | data, err := stream.render(frame, damage, reset) | 166 | + | prepared, err := stream.prepare(frame, damage, reset) |
| 151 | - | if err != nil || len(data) == 0 { | 167 | + | if err != nil || len(prepared.data) == 0 { |
| 168 | + | if prepared != nil { | |||
| 169 | + | prepared.commitNoSend() | |||
| 170 | + | } | |||
| 171 | + | return ports.Frame{}, false, err | |||
| 172 | + | } | |||
| 173 | + | var output ports.Frame | |||
| 174 | + | err = prepared.send(prepared.data, echoAck, func(frame ports.Frame) error { | |||
| 175 | + | output = frame | |||
| 176 | + | return nil | |||
| 177 | + | }) | |||
| 178 | + | if err != nil { | |||
| 152 | return ports.Frame{}, false, err | 179 | return ports.Frame{}, false, err | ||
| 153 | } | 180 | } | ||
| 154 | - | return stream.frame(data, reset, echoAck), true, nil | 181 | + | return output, true, nil |
| 155 | } | 182 | } | ||
| 156 | 183 | ||||
| 157 | // A resize is a state reset, but it must not turn subsequent idle or damaged | 184 | // A resize is a state reset, but it must not turn subsequent idle or damaged | ||
M internal/usecase/daemon/proxy_output.go +1 -2
| @@ -32,14 +32,13 @@ func (p *proxySession) applyScreenUpdateForGeneration(generation uint64, update | |||
| 32 | 32 | if update.NewStateNum <= p.appliedState { | |
| 33 | 33 | return 0, p.requestScreenResetLocked(), false | |
| 34 | 34 | } | |
| 35 | - | before := p.screen.cursorOut | |
| 36 | 35 | if err := p.screen.Apply(update); err != nil { | |
| 37 | 36 | return 0, p.requestScreenResetLocked(), false | |
| 38 | 37 | } | |
| 39 | 38 | p.appliedState = update.NewStateNum | |
| 40 | 39 | p.screenReady = true | |
| 41 | 40 | p.resetRequested = false | |
| 42 | - | return update.NewStateNum, false, update.Kind == ports.ScreenUpdateSnapshot || before != update.Cursor || len(update.Spans) != 0 | |
| 41 | + | return update.NewStateNum, false, true | |
| 43 | 42 | } | |
| 44 | 43 | ||
| 45 | 44 | if !p.screenReady || p.resetRequested || update.BaseStateNum != p.appliedState || update.NewStateNum != update.BaseStateNum+1 { | |
| @@ -32,14 +32,13 @@ func (p *proxySession) applyScreenUpdateForGeneration(generation uint64, update | |||||
| 32 | if update.NewStateNum <= p.appliedState { | 32 | if update.NewStateNum <= p.appliedState { | ||
| 33 | return 0, p.requestScreenResetLocked(), false | 33 | return 0, p.requestScreenResetLocked(), false | ||
| 34 | } | 34 | } | ||
| 35 | - | before := p.screen.cursorOut | |||
| 36 | if err := p.screen.Apply(update); err != nil { | 35 | if err := p.screen.Apply(update); err != nil { | ||
| 37 | return 0, p.requestScreenResetLocked(), false | 36 | return 0, p.requestScreenResetLocked(), false | ||
| 38 | } | 37 | } | ||
| 39 | p.appliedState = update.NewStateNum | 38 | p.appliedState = update.NewStateNum | ||
| 40 | p.screenReady = true | 39 | p.screenReady = true | ||
| 41 | p.resetRequested = false | 40 | p.resetRequested = false | ||
| 42 | - | return update.NewStateNum, false, update.Kind == ports.ScreenUpdateSnapshot || before != update.Cursor || len(update.Spans) != 0 | 41 | + | return update.NewStateNum, false, true |
| 43 | } | 42 | } | ||
| 44 | 43 | ||||
| 45 | if !p.screenReady || p.resetRequested || update.BaseStateNum != p.appliedState || update.NewStateNum != update.BaseStateNum+1 { | 44 | if !p.screenReady || p.resetRequested || update.BaseStateNum != p.appliedState || update.NewStateNum != update.BaseStateNum+1 { | ||
M internal/usecase/daemon/proxy_output_test.go +19 -14
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "io" | |
| 6 | 6 | "log/slog" | |
| 7 | + | "maps" | |
| 7 | 8 | "sync" | |
| 8 | 9 | "sync/atomic" | |
| 9 | 10 | "testing" | |
| @@ -182,11 +183,7 @@ func newProxyTestDaemon(t *testing.T, factory ports.RemoteDialerFactory, clk por | |||
| 182 | 183 | func proxySessionSnapshot(d *Daemon) map[domain.SessionID]attachmentSession { | |
| 183 | 184 | d.mu.Lock() | |
| 184 | 185 | defer d.mu.Unlock() | |
| 185 | - | clone := make(map[domain.SessionID]attachmentSession, len(d.sessions)) | |
| 186 | - | for id, entry := range d.sessions { | |
| 187 | - | clone[id] = entry | |
| 188 | - | } | |
| 189 | - | return clone | |
| 186 | + | return maps.Clone(d.sessions) | |
| 190 | 187 | } | |
| 191 | 188 | ||
| 192 | 189 | func proxyFactoryFor(t *testing.T, key domain.RemoteSessionKey, transports ...ports.Transport) ports.RemoteDialerFactory { | |
| @@ -246,7 +243,7 @@ func TestProxyHandshakePublishesOnlyAfterWelcomeAndMetadata(t *testing.T) { | |||
| 246 | 243 | require.Equal(t, ports.IntentAttach, hello.Intent) | |
| 247 | 244 | require.True(t, hello.Proxied) | |
| 248 | 245 | require.Equal(t, key.Name, hello.Name) | |
| 249 | - | require.Equal(t, domain.Size{Cols: 80, Rows: 22}, hello.Size) | |
| 246 | + | require.Equal(t, contentSize(domain.Size{Cols: 80, Rows: 24}, false), hello.Size) | |
| 250 | 247 | require.Equal(t, uint8(maxUnackedOutputStates), hello.MaxOutputInFlight) | |
| 251 | 248 | require.Empty(t, proxySessionSnapshot(d), "Hello alone must not publish a proxy") | |
| 252 | 249 | ||
| @@ -457,11 +454,9 @@ func TestProxySenderSerializesAllTransportWrites(t *testing.T) { | |||
| 457 | 454 | var wg sync.WaitGroup | |
| 458 | 455 | errs := make(chan error, 8) | |
| 459 | 456 | for range 8 { | |
| 460 | - | wg.Add(1) | |
| 461 | - | go func() { | |
| 462 | - | defer wg.Done() | |
| 457 | + | wg.Go(func() { | |
| 463 | 458 | errs <- proxy.sendCurrent(ports.Frame{Type: ports.MsgPing, Payload: ports.MarshalPing(ports.Ping{})}) | |
| 464 | - | }() | |
| 459 | + | }) | |
| 465 | 460 | } | |
| 466 | 461 | // Once the first sender enters Send, the others must remain behind sendMu. | |
| 467 | 462 | awaitTestCompletion(t, entered, "proxy sender did not enter transport") | |
| @@ -669,6 +664,15 @@ func screenSnapshotForProxy(t *testing.T, proxy *proxySession, state uint64) por | |||
| 669 | 664 | return snapshot | |
| 670 | 665 | } | |
| 671 | 666 | ||
| 667 | + | func TestProxyScreenSnapshotReportsChanged(t *testing.T) { | |
| 668 | + | _, proxy, _, generation := newProxyOutputSession(t) | |
| 669 | + | snapshot := screenSnapshotForProxy(t, proxy, 1) | |
| 670 | + | ack, reset, changed := proxy.applyScreenUpdateForGeneration(generation, snapshot) | |
| 671 | + | require.Equal(t, uint64(1), ack) | |
| 672 | + | require.False(t, reset) | |
| 673 | + | require.True(t, changed, "every accepted snapshot invalidates the composed frame") | |
| 674 | + | } | |
| 675 | + | ||
| 672 | 676 | func TestProxyScreenUpdate(t *testing.T) { | |
| 673 | 677 | t.Run("initial snapshot", func(t *testing.T) { | |
| 674 | 678 | d, proxy, transport, generation := newProxyOutputSession(t) | |
| @@ -812,19 +816,20 @@ func TestProxyScreenUpdate(t *testing.T) { | |||
| 812 | 816 | }() | |
| 813 | 817 | awaitTestCompletion(t, entered, "screen ACK sender did not enter transport") | |
| 814 | 818 | ||
| 815 | - | unlocked := make(chan struct{}) | |
| 819 | + | unlocked := make(chan ports.Transport, 1) | |
| 816 | 820 | go func() { | |
| 817 | 821 | proxy.mu.Lock() | |
| 818 | 822 | currentTransport := proxy.transport | |
| 819 | 823 | proxy.mu.Unlock() | |
| 820 | - | require.Same(t, transport, currentTransport) | |
| 821 | - | close(unlocked) | |
| 824 | + | unlocked <- currentTransport | |
| 822 | 825 | }() | |
| 826 | + | var currentTransport ports.Transport | |
| 823 | 827 | select { | |
| 824 | - | case <-unlocked: | |
| 828 | + | case currentTransport = <-unlocked: | |
| 825 | 829 | case <-time.After(time.Second): | |
| 826 | 830 | t.Fatal("proxy.mu was held while screen send was blocked") | |
| 827 | 831 | } | |
| 832 | + | require.Same(t, transport, currentTransport) | |
| 828 | 833 | require.NoError(t, transport.Close()) | |
| 829 | 834 | require.Error(t, awaitTestValue(t, result, "blocked screen send did not return")) | |
| 830 | 835 | }) | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "io" | 5 | "io" | ||
| 6 | "log/slog" | 6 | "log/slog" | ||
| 7 | + | "maps" | |||
| 7 | "sync" | 8 | "sync" | ||
| 8 | "sync/atomic" | 9 | "sync/atomic" | ||
| 9 | "testing" | 10 | "testing" | ||
| @@ -182,11 +183,7 @@ func newProxyTestDaemon(t *testing.T, factory ports.RemoteDialerFactory, clk por | |||||
| 182 | func proxySessionSnapshot(d *Daemon) map[domain.SessionID]attachmentSession { | 183 | func proxySessionSnapshot(d *Daemon) map[domain.SessionID]attachmentSession { | ||
| 183 | d.mu.Lock() | 184 | d.mu.Lock() | ||
| 184 | defer d.mu.Unlock() | 185 | defer d.mu.Unlock() | ||
| 185 | - | clone := make(map[domain.SessionID]attachmentSession, len(d.sessions)) | 186 | + | return maps.Clone(d.sessions) |
| 186 | - | for id, entry := range d.sessions { | |||
| 187 | - | clone[id] = entry | |||
| 188 | - | } | |||
| 189 | - | return clone | |||
| 190 | } | 187 | } | ||
| 191 | 188 | ||||
| 192 | func proxyFactoryFor(t *testing.T, key domain.RemoteSessionKey, transports ...ports.Transport) ports.RemoteDialerFactory { | 189 | func proxyFactoryFor(t *testing.T, key domain.RemoteSessionKey, transports ...ports.Transport) ports.RemoteDialerFactory { | ||
| @@ -246,7 +243,7 @@ func TestProxyHandshakePublishesOnlyAfterWelcomeAndMetadata(t *testing.T) { | |||||
| 246 | require.Equal(t, ports.IntentAttach, hello.Intent) | 243 | require.Equal(t, ports.IntentAttach, hello.Intent) | ||
| 247 | require.True(t, hello.Proxied) | 244 | require.True(t, hello.Proxied) | ||
| 248 | require.Equal(t, key.Name, hello.Name) | 245 | require.Equal(t, key.Name, hello.Name) | ||
| 249 | - | require.Equal(t, domain.Size{Cols: 80, Rows: 22}, hello.Size) | 246 | + | require.Equal(t, contentSize(domain.Size{Cols: 80, Rows: 24}, false), hello.Size) |
| 250 | require.Equal(t, uint8(maxUnackedOutputStates), hello.MaxOutputInFlight) | 247 | require.Equal(t, uint8(maxUnackedOutputStates), hello.MaxOutputInFlight) | ||
| 251 | require.Empty(t, proxySessionSnapshot(d), "Hello alone must not publish a proxy") | 248 | require.Empty(t, proxySessionSnapshot(d), "Hello alone must not publish a proxy") | ||
| 252 | 249 | ||||
| @@ -457,11 +454,9 @@ func TestProxySenderSerializesAllTransportWrites(t *testing.T) { | |||||
| 457 | var wg sync.WaitGroup | 454 | var wg sync.WaitGroup | ||
| 458 | errs := make(chan error, 8) | 455 | errs := make(chan error, 8) | ||
| 459 | for range 8 { | 456 | for range 8 { | ||
| 460 | - | wg.Add(1) | 457 | + | wg.Go(func() { |
| 461 | - | go func() { | |||
| 462 | - | defer wg.Done() | |||
| 463 | errs <- proxy.sendCurrent(ports.Frame{Type: ports.MsgPing, Payload: ports.MarshalPing(ports.Ping{})}) | 458 | errs <- proxy.sendCurrent(ports.Frame{Type: ports.MsgPing, Payload: ports.MarshalPing(ports.Ping{})}) | ||
| 464 | - | }() | 459 | + | }) |
| 465 | } | 460 | } | ||
| 466 | // Once the first sender enters Send, the others must remain behind sendMu. | 461 | // Once the first sender enters Send, the others must remain behind sendMu. | ||
| 467 | awaitTestCompletion(t, entered, "proxy sender did not enter transport") | 462 | awaitTestCompletion(t, entered, "proxy sender did not enter transport") | ||
| @@ -669,6 +664,15 @@ func screenSnapshotForProxy(t *testing.T, proxy *proxySession, state uint64) por | |||||
| 669 | return snapshot | 664 | return snapshot | ||
| 670 | } | 665 | } | ||
| 671 | 666 | ||||
| 667 | + | func TestProxyScreenSnapshotReportsChanged(t *testing.T) { | |||
| 668 | + | _, proxy, _, generation := newProxyOutputSession(t) | |||
| 669 | + | snapshot := screenSnapshotForProxy(t, proxy, 1) | |||
| 670 | + | ack, reset, changed := proxy.applyScreenUpdateForGeneration(generation, snapshot) | |||
| 671 | + | require.Equal(t, uint64(1), ack) | |||
| 672 | + | require.False(t, reset) | |||
| 673 | + | require.True(t, changed, "every accepted snapshot invalidates the composed frame") | |||
| 674 | + | } | |||
| 675 | + | ||||
| 672 | func TestProxyScreenUpdate(t *testing.T) { | 676 | func TestProxyScreenUpdate(t *testing.T) { | ||
| 673 | t.Run("initial snapshot", func(t *testing.T) { | 677 | t.Run("initial snapshot", func(t *testing.T) { | ||
| 674 | d, proxy, transport, generation := newProxyOutputSession(t) | 678 | d, proxy, transport, generation := newProxyOutputSession(t) | ||
| @@ -812,19 +816,20 @@ func TestProxyScreenUpdate(t *testing.T) { | |||||
| 812 | }() | 816 | }() | ||
| 813 | awaitTestCompletion(t, entered, "screen ACK sender did not enter transport") | 817 | awaitTestCompletion(t, entered, "screen ACK sender did not enter transport") | ||
| 814 | 818 | ||||
| 815 | - | unlocked := make(chan struct{}) | 819 | + | unlocked := make(chan ports.Transport, 1) |
| 816 | go func() { | 820 | go func() { | ||
| 817 | proxy.mu.Lock() | 821 | proxy.mu.Lock() | ||
| 818 | currentTransport := proxy.transport | 822 | currentTransport := proxy.transport | ||
| 819 | proxy.mu.Unlock() | 823 | proxy.mu.Unlock() | ||
| 820 | - | require.Same(t, transport, currentTransport) | 824 | + | unlocked <- currentTransport |
| 821 | - | close(unlocked) | |||
| 822 | }() | 825 | }() | ||
| 826 | + | var currentTransport ports.Transport | |||
| 823 | select { | 827 | select { | ||
| 824 | - | case <-unlocked: | 828 | + | case currentTransport = <-unlocked: |
| 825 | case <-time.After(time.Second): | 829 | case <-time.After(time.Second): | ||
| 826 | t.Fatal("proxy.mu was held while screen send was blocked") | 830 | t.Fatal("proxy.mu was held while screen send was blocked") | ||
| 827 | } | 831 | } | ||
| 832 | + | require.Same(t, transport, currentTransport) | |||
| 828 | require.NoError(t, transport.Close()) | 833 | require.NoError(t, transport.Close()) | ||
| 829 | require.Error(t, awaitTestValue(t, result, "blocked screen send did not return")) | 834 | require.Error(t, awaitTestValue(t, result, "blocked screen send did not return")) | ||
| 830 | }) | 835 | }) | ||
M internal/usecase/daemon/proxy_screen_state.go +12 -16
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "errors" | |
| 4 | 5 | "fmt" | |
| 5 | 6 | "math" | |
| 6 | 7 | "unicode/utf8" | |
| @@ -40,19 +41,20 @@ func newProxyScreenState(size domain.Size) *proxyScreenState { | |||
| 40 | 41 | if !validProxyScreenSize(size) { | |
| 41 | 42 | return &proxyScreenState{} | |
| 42 | 43 | } | |
| 43 | - | return &proxyScreenState{ | |
| 44 | + | state := &proxyScreenState{ | |
| 44 | 45 | frame: renderer.NewFrame(size.Cols, size.Rows), | |
| 45 | 46 | scratch: renderer.NewFrame(size.Cols, size.Rows), | |
| 46 | 47 | cursorOut: ports.ScreenCursor{Visible: true}, | |
| 47 | - | damage: []renderer.Damage{renderer.FullRedraw()}, | |
| 48 | 48 | generation: 1, | |
| 49 | 49 | } | |
| 50 | + | state.setFullRedraw() | |
| 51 | + | return state | |
| 50 | 52 | } | |
| 51 | 53 | ||
| 52 | 54 | // Apply validates the complete semantic update before mutating either frame. | |
| 53 | 55 | func (s *proxyScreenState) Apply(update ports.ScreenUpdate) error { | |
| 54 | 56 | if s == nil { | |
| 55 | - | return fmt.Errorf("proxy screen state is nil") | |
| 57 | + | return errors.New("proxy screen state is nil") | |
| 56 | 58 | } | |
| 57 | 59 | if err := s.validateUpdate(update); err != nil { | |
| 58 | 60 | return err | |
| @@ -173,7 +175,7 @@ func (s *proxyScreenState) validateStateNumbers(update ports.ScreenUpdate) error | |||
| 173 | 175 | func applyScreenUpdate(frame *renderer.Frame, update ports.ScreenUpdate) { | |
| 174 | 176 | if update.Scroll != nil { | |
| 175 | 177 | scroll := update.Scroll | |
| 176 | - | frame.ScrollUp(int(scroll.Top), int(scroll.Top+scroll.Height-1), int(scroll.Count)) | |
| 178 | + | frame.ScrollUp(int(scroll.Top), int(scroll.Top)+int(scroll.Height)-1, int(scroll.Count)) | |
| 177 | 179 | } | |
| 178 | 180 | for _, span := range update.Spans { | |
| 179 | 181 | copy(frame.Row(int(span.Y))[int(span.X):], span.Cells) | |
| @@ -266,7 +268,7 @@ func (s *proxyScreenState) AcknowledgeDamage(generation uint64) bool { | |||
| 266 | 268 | // replayed incrementally; snapshots, uncertain damage, and dimensions use a | |
| 267 | 269 | // complete clone. | |
| 268 | 270 | func (s *proxyScreenState) CaptureInto(dst *renderer.Frame) { | |
| 269 | - | if s == nil || dst == nil || !validProxyFrame(*s) { | |
| 271 | + | if s == nil || dst == nil || !validProxyFrame(s) { | |
| 270 | 272 | return | |
| 271 | 273 | } | |
| 272 | 274 | if dst.Width != s.frame.Width || dst.Height != s.frame.Height || dst.Validate() != nil { | |
| @@ -326,15 +328,9 @@ func (s *proxyScreenState) ResizePlaceholder(size domain.Size) bool { | |||
| 326 | 328 | } | |
| 327 | 329 | ||
| 328 | 330 | func copyFrameOverlap(dst *renderer.Frame, src renderer.Frame) { | |
| 329 | - | width := src.Width | |
| 330 | - | if dst.Width < width { | |
| 331 | - | width = dst.Width | |
| 332 | - | } | |
| 333 | - | height := src.Height | |
| 334 | - | if dst.Height < height { | |
| 335 | - | height = dst.Height | |
| 336 | - | } | |
| 337 | - | for y := 0; y < height; y++ { | |
| 331 | + | width := min(src.Width, dst.Width) | |
| 332 | + | height := min(src.Height, dst.Height) | |
| 333 | + | for y := range height { | |
| 338 | 334 | copy(dst.Row(y)[:width], src.Row(y)[:width]) | |
| 339 | 335 | } | |
| 340 | 336 | } | |
| @@ -346,8 +342,8 @@ func validProxyScreenSize(size domain.Size) bool { | |||
| 346 | 342 | return size.Rows <= maxProxyScreenCells/size.Cols | |
| 347 | 343 | } | |
| 348 | 344 | ||
| 349 | - | func validProxyFrame(s proxyScreenState) bool { | |
| 350 | - | return s.frame.Validate() == nil && s.scratch.Validate() == nil | |
| 345 | + | func validProxyFrame(s *proxyScreenState) bool { | |
| 346 | + | return s != nil && s.frame.Validate() == nil && s.scratch.Validate() == nil | |
| 351 | 347 | } | |
| 352 | 348 | ||
| 353 | 349 | func validProxyScreenStyle(style renderer.Style) bool { | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "errors" | |||
| 4 | "fmt" | 5 | "fmt" | ||
| 5 | "math" | 6 | "math" | ||
| 6 | "unicode/utf8" | 7 | "unicode/utf8" | ||
| @@ -40,19 +41,20 @@ func newProxyScreenState(size domain.Size) *proxyScreenState { | |||||
| 40 | if !validProxyScreenSize(size) { | 41 | if !validProxyScreenSize(size) { | ||
| 41 | return &proxyScreenState{} | 42 | return &proxyScreenState{} | ||
| 42 | } | 43 | } | ||
| 43 | - | return &proxyScreenState{ | 44 | + | state := &proxyScreenState{ |
| 44 | frame: renderer.NewFrame(size.Cols, size.Rows), | 45 | frame: renderer.NewFrame(size.Cols, size.Rows), | ||
| 45 | scratch: renderer.NewFrame(size.Cols, size.Rows), | 46 | scratch: renderer.NewFrame(size.Cols, size.Rows), | ||
| 46 | cursorOut: ports.ScreenCursor{Visible: true}, | 47 | cursorOut: ports.ScreenCursor{Visible: true}, | ||
| 47 | - | damage: []renderer.Damage{renderer.FullRedraw()}, | |||
| 48 | generation: 1, | 48 | generation: 1, | ||
| 49 | } | 49 | } | ||
| 50 | + | state.setFullRedraw() | |||
| 51 | + | return state | |||
| 50 | } | 52 | } | ||
| 51 | 53 | ||||
| 52 | // Apply validates the complete semantic update before mutating either frame. | 54 | // Apply validates the complete semantic update before mutating either frame. | ||
| 53 | func (s *proxyScreenState) Apply(update ports.ScreenUpdate) error { | 55 | func (s *proxyScreenState) Apply(update ports.ScreenUpdate) error { | ||
| 54 | if s == nil { | 56 | if s == nil { | ||
| 55 | - | return fmt.Errorf("proxy screen state is nil") | 57 | + | return errors.New("proxy screen state is nil") |
| 56 | } | 58 | } | ||
| 57 | if err := s.validateUpdate(update); err != nil { | 59 | if err := s.validateUpdate(update); err != nil { | ||
| 58 | return err | 60 | return err | ||
| @@ -173,7 +175,7 @@ func (s *proxyScreenState) validateStateNumbers(update ports.ScreenUpdate) error | |||||
| 173 | func applyScreenUpdate(frame *renderer.Frame, update ports.ScreenUpdate) { | 175 | func applyScreenUpdate(frame *renderer.Frame, update ports.ScreenUpdate) { | ||
| 174 | if update.Scroll != nil { | 176 | if update.Scroll != nil { | ||
| 175 | scroll := update.Scroll | 177 | scroll := update.Scroll | ||
| 176 | - | frame.ScrollUp(int(scroll.Top), int(scroll.Top+scroll.Height-1), int(scroll.Count)) | 178 | + | frame.ScrollUp(int(scroll.Top), int(scroll.Top)+int(scroll.Height)-1, int(scroll.Count)) |
| 177 | } | 179 | } | ||
| 178 | for _, span := range update.Spans { | 180 | for _, span := range update.Spans { | ||
| 179 | copy(frame.Row(int(span.Y))[int(span.X):], span.Cells) | 181 | copy(frame.Row(int(span.Y))[int(span.X):], span.Cells) | ||
| @@ -266,7 +268,7 @@ func (s *proxyScreenState) AcknowledgeDamage(generation uint64) bool { | |||||
| 266 | // replayed incrementally; snapshots, uncertain damage, and dimensions use a | 268 | // replayed incrementally; snapshots, uncertain damage, and dimensions use a | ||
| 267 | // complete clone. | 269 | // complete clone. | ||
| 268 | func (s *proxyScreenState) CaptureInto(dst *renderer.Frame) { | 270 | func (s *proxyScreenState) CaptureInto(dst *renderer.Frame) { | ||
| 269 | - | if s == nil || dst == nil || !validProxyFrame(*s) { | 271 | + | if s == nil || dst == nil || !validProxyFrame(s) { |
| 270 | return | 272 | return | ||
| 271 | } | 273 | } | ||
| 272 | if dst.Width != s.frame.Width || dst.Height != s.frame.Height || dst.Validate() != nil { | 274 | if dst.Width != s.frame.Width || dst.Height != s.frame.Height || dst.Validate() != nil { | ||
| @@ -326,15 +328,9 @@ func (s *proxyScreenState) ResizePlaceholder(size domain.Size) bool { | |||||
| 326 | } | 328 | } | ||
| 327 | 329 | ||||
| 328 | func copyFrameOverlap(dst *renderer.Frame, src renderer.Frame) { | 330 | func copyFrameOverlap(dst *renderer.Frame, src renderer.Frame) { | ||
| 329 | - | width := src.Width | 331 | + | width := min(src.Width, dst.Width) |
| 330 | - | if dst.Width < width { | 332 | + | height := min(src.Height, dst.Height) |
| 331 | - | width = dst.Width | 333 | + | for y := range height { |
| 332 | - | } | |||
| 333 | - | height := src.Height | |||
| 334 | - | if dst.Height < height { | |||
| 335 | - | height = dst.Height | |||
| 336 | - | } | |||
| 337 | - | for y := 0; y < height; y++ { | |||
| 338 | copy(dst.Row(y)[:width], src.Row(y)[:width]) | 334 | copy(dst.Row(y)[:width], src.Row(y)[:width]) | ||
| 339 | } | 335 | } | ||
| 340 | } | 336 | } | ||
| @@ -346,8 +342,8 @@ func validProxyScreenSize(size domain.Size) bool { | |||||
| 346 | return size.Rows <= maxProxyScreenCells/size.Cols | 342 | return size.Rows <= maxProxyScreenCells/size.Cols | ||
| 347 | } | 343 | } | ||
| 348 | 344 | ||||
| 349 | - | func validProxyFrame(s proxyScreenState) bool { | 345 | + | func validProxyFrame(s *proxyScreenState) bool { |
| 350 | - | return s.frame.Validate() == nil && s.scratch.Validate() == nil | 346 | + | return s != nil && s.frame.Validate() == nil && s.scratch.Validate() == nil |
| 351 | } | 347 | } | ||
| 352 | 348 | ||||
| 353 | func validProxyScreenStyle(style renderer.Style) bool { | 349 | func validProxyScreenStyle(style renderer.Style) bool { | ||
M internal/usecase/daemon/proxy_screen_state_test.go +11 -6
| @@ -80,16 +80,21 @@ func TestProxyScreenStateInvalidUpdateIsAtomic(t *testing.T) { | |||
| 80 | 80 | } | |
| 81 | 81 | ||
| 82 | 82 | func TestProxyScreenStateScreenAreaLimitIsAtomic(t *testing.T) { | |
| 83 | - | boundary := newProxyScreenState(domain.Size{Cols: 512, Rows: 512}) | |
| 84 | - | if boundary.frame.Width != 512 || boundary.frame.Height != 512 || boundary.scratch.Width != 512 || boundary.scratch.Height != 512 { | |
| 83 | + | const boundaryCols = 512 | |
| 84 | + | boundaryRows := maxProxyScreenCells / boundaryCols | |
| 85 | + | boundary := newProxyScreenState(domain.Size{Cols: boundaryCols, Rows: boundaryRows}) | |
| 86 | + | if boundary.frame.Width != boundaryCols || boundary.frame.Height != boundaryRows || boundary.scratch.Width != boundaryCols || boundary.scratch.Height != boundaryRows { | |
| 85 | 87 | t.Fatalf("boundary state dimensions = %dx%d and %dx%d", boundary.frame.Width, boundary.frame.Height, boundary.scratch.Width, boundary.scratch.Height) | |
| 86 | 88 | } | |
| 87 | - | if boundary.ResizePlaceholder(domain.Size{Cols: 513, Rows: 512}) { | |
| 89 | + | if boundary.ResizePlaceholder(domain.Size{Cols: boundaryCols, Rows: boundaryRows + 1}) { | |
| 88 | 90 | t.Fatal("over-cap resize was accepted") | |
| 89 | 91 | } | |
| 90 | - | if boundary.frame.Width != 512 || boundary.frame.Height != 512 || boundary.scratch.Width != 512 || boundary.scratch.Height != 512 { | |
| 92 | + | if boundary.frame.Width != boundaryCols || boundary.frame.Height != boundaryRows || boundary.scratch.Width != boundaryCols || boundary.scratch.Height != boundaryRows { | |
| 91 | 93 | t.Fatal("over-cap resize changed boundary state") | |
| 92 | 94 | } | |
| 95 | + | if !boundary.damageFullRedrawSticky { | |
| 96 | + | t.Fatal("initial full redraw must remain sticky until acknowledged") | |
| 97 | + | } | |
| 93 | 98 | ||
| 94 | 99 | s := newProxyScreenState(domain.Size{Cols: 2, Rows: 2}) | |
| 95 | 100 | if err := s.Apply(screenSnapshot(2, 2, "ab", "cd")); err != nil { | |
| @@ -337,7 +342,7 @@ func TestProxyScreenStateCursorOnlyAndDamageBound(t *testing.T) { | |||
| 337 | 342 | if s.cursorOut.Row != 1 || s.cursorOut.Col != 1 || len(s.damage) != 0 { | |
| 338 | 343 | t.Fatalf("cursor-only update = cursor=%+v damage=%+v", s.cursorOut, s.damage) | |
| 339 | 344 | } | |
| 340 | - | for i := 0; i < maxProxyScreenDamage; i++ { | |
| 345 | + | for range maxProxyScreenDamage { | |
| 341 | 346 | applyProxyDelta(t, s, ports.ScreenUpdate{ | |
| 342 | 347 | Size: domain.Size{Cols: 2, Rows: 2}, | |
| 343 | 348 | Spans: []ports.ScreenSpan{{Y: 0, Cells: cells("x")}}, | |
| @@ -384,7 +389,7 @@ func BenchmarkProxyScreenStateApply(b *testing.B) { | |||
| 384 | 389 | b.ResetTimer() | |
| 385 | 390 | b.ReportMetric(float64(tc.cells), "cells/op") | |
| 386 | 391 | b.ReportMetric(float64(tc.spans), "spans/op") | |
| 387 | - | for range b.N { | |
| 392 | + | for b.Loop() { | |
| 388 | 393 | if err := s.Apply(nextProxyDelta(s, tc.update)); err != nil { | |
| 389 | 394 | b.Fatal(err) | |
| 390 | 395 | } | |
| @@ -80,16 +80,21 @@ func TestProxyScreenStateInvalidUpdateIsAtomic(t *testing.T) { | |||||
| 80 | } | 80 | } | ||
| 81 | 81 | ||||
| 82 | func TestProxyScreenStateScreenAreaLimitIsAtomic(t *testing.T) { | 82 | func TestProxyScreenStateScreenAreaLimitIsAtomic(t *testing.T) { | ||
| 83 | - | boundary := newProxyScreenState(domain.Size{Cols: 512, Rows: 512}) | 83 | + | const boundaryCols = 512 |
| 84 | - | if boundary.frame.Width != 512 || boundary.frame.Height != 512 || boundary.scratch.Width != 512 || boundary.scratch.Height != 512 { | 84 | + | boundaryRows := maxProxyScreenCells / boundaryCols |
| 85 | + | boundary := newProxyScreenState(domain.Size{Cols: boundaryCols, Rows: boundaryRows}) | |||
| 86 | + | if boundary.frame.Width != boundaryCols || boundary.frame.Height != boundaryRows || boundary.scratch.Width != boundaryCols || boundary.scratch.Height != boundaryRows { | |||
| 85 | t.Fatalf("boundary state dimensions = %dx%d and %dx%d", boundary.frame.Width, boundary.frame.Height, boundary.scratch.Width, boundary.scratch.Height) | 87 | t.Fatalf("boundary state dimensions = %dx%d and %dx%d", boundary.frame.Width, boundary.frame.Height, boundary.scratch.Width, boundary.scratch.Height) | ||
| 86 | } | 88 | } | ||
| 87 | - | if boundary.ResizePlaceholder(domain.Size{Cols: 513, Rows: 512}) { | 89 | + | if boundary.ResizePlaceholder(domain.Size{Cols: boundaryCols, Rows: boundaryRows + 1}) { |
| 88 | t.Fatal("over-cap resize was accepted") | 90 | t.Fatal("over-cap resize was accepted") | ||
| 89 | } | 91 | } | ||
| 90 | - | if boundary.frame.Width != 512 || boundary.frame.Height != 512 || boundary.scratch.Width != 512 || boundary.scratch.Height != 512 { | 92 | + | if boundary.frame.Width != boundaryCols || boundary.frame.Height != boundaryRows || boundary.scratch.Width != boundaryCols || boundary.scratch.Height != boundaryRows { |
| 91 | t.Fatal("over-cap resize changed boundary state") | 93 | t.Fatal("over-cap resize changed boundary state") | ||
| 92 | } | 94 | } | ||
| 95 | + | if !boundary.damageFullRedrawSticky { | |||
| 96 | + | t.Fatal("initial full redraw must remain sticky until acknowledged") | |||
| 97 | + | } | |||
| 93 | 98 | ||||
| 94 | s := newProxyScreenState(domain.Size{Cols: 2, Rows: 2}) | 99 | s := newProxyScreenState(domain.Size{Cols: 2, Rows: 2}) | ||
| 95 | if err := s.Apply(screenSnapshot(2, 2, "ab", "cd")); err != nil { | 100 | if err := s.Apply(screenSnapshot(2, 2, "ab", "cd")); err != nil { | ||
| @@ -337,7 +342,7 @@ func TestProxyScreenStateCursorOnlyAndDamageBound(t *testing.T) { | |||||
| 337 | if s.cursorOut.Row != 1 || s.cursorOut.Col != 1 || len(s.damage) != 0 { | 342 | if s.cursorOut.Row != 1 || s.cursorOut.Col != 1 || len(s.damage) != 0 { | ||
| 338 | t.Fatalf("cursor-only update = cursor=%+v damage=%+v", s.cursorOut, s.damage) | 343 | t.Fatalf("cursor-only update = cursor=%+v damage=%+v", s.cursorOut, s.damage) | ||
| 339 | } | 344 | } | ||
| 340 | - | for i := 0; i < maxProxyScreenDamage; i++ { | 345 | + | for range maxProxyScreenDamage { |
| 341 | applyProxyDelta(t, s, ports.ScreenUpdate{ | 346 | applyProxyDelta(t, s, ports.ScreenUpdate{ | ||
| 342 | Size: domain.Size{Cols: 2, Rows: 2}, | 347 | Size: domain.Size{Cols: 2, Rows: 2}, | ||
| 343 | Spans: []ports.ScreenSpan{{Y: 0, Cells: cells("x")}}, | 348 | Spans: []ports.ScreenSpan{{Y: 0, Cells: cells("x")}}, | ||
| @@ -384,7 +389,7 @@ func BenchmarkProxyScreenStateApply(b *testing.B) { | |||||
| 384 | b.ResetTimer() | 389 | b.ResetTimer() | ||
| 385 | b.ReportMetric(float64(tc.cells), "cells/op") | 390 | b.ReportMetric(float64(tc.cells), "cells/op") | ||
| 386 | b.ReportMetric(float64(tc.spans), "spans/op") | 391 | b.ReportMetric(float64(tc.spans), "spans/op") | ||
| 387 | - | for range b.N { | 392 | + | for b.Loop() { |
| 388 | if err := s.Apply(nextProxyDelta(s, tc.update)); err != nil { | 393 | if err := s.Apply(nextProxyDelta(s, tc.update)); err != nil { | ||
| 389 | b.Fatal(err) | 394 | b.Fatal(err) | ||
| 390 | } | 395 | } | ||
M internal/usecase/daemon/proxy_session.go +2 -0
| @@ -27,6 +27,8 @@ type proxySession struct { | |||
| 27 | 27 | transport ports.Transport | |
| 28 | 28 | resumeToken uint64 | |
| 29 | 29 | appliedState uint64 | |
| 30 | + | // screenReady is true only after a moving snapshot establishes the | |
| 31 | + | // dependency required by subsequent deltas. | |
| 30 | 32 | screenReady bool | |
| 31 | 33 | resetRequested bool | |
| 32 | 34 | screen *proxyScreenState | |
| @@ -27,6 +27,8 @@ type proxySession struct { | |||||
| 27 | transport ports.Transport | 27 | transport ports.Transport | ||
| 28 | resumeToken uint64 | 28 | resumeToken uint64 | ||
| 29 | appliedState uint64 | 29 | appliedState uint64 | ||
| 30 | + | // screenReady is true only after a moving snapshot establishes the | |||
| 31 | + | // dependency required by subsequent deltas. | |||
| 30 | screenReady bool | 32 | screenReady bool | ||
| 31 | resetRequested bool | 33 | resetRequested bool | ||
| 32 | screen *proxyScreenState | 34 | screen *proxyScreenState | ||
M internal/usecase/daemon/render_pipeline_test.go +2 -1
| @@ -333,7 +333,8 @@ func TestComposeEmitExactReplayTiledFloatingBarsOverlayAndCursor(t *testing.T) { | |||
| 333 | 333 | client := vt.NewScreen(composed.frame.Width, composed.frame.Height) | |
| 334 | 334 | client.Write(terminalBytes) | |
| 335 | 335 | require.Equal(t, frameRows(composed.frame), frameRows(client.Frame)) | |
| 336 | - | prepared.commitNoSend() | |
| 336 | + | prepared.draw.Commit() | |
| 337 | + | stream.forceSnapshot = false | |
| 337 | 338 | again, err := stream.renderer.Draw(composed.frame, nil) | |
| 338 | 339 | require.NoError(t, err) | |
| 339 | 340 | require.Empty(t, again, "renderer shadow must exactly equal the composed frame") | |
| @@ -333,7 +333,8 @@ func TestComposeEmitExactReplayTiledFloatingBarsOverlayAndCursor(t *testing.T) { | |||||
| 333 | client := vt.NewScreen(composed.frame.Width, composed.frame.Height) | 333 | client := vt.NewScreen(composed.frame.Width, composed.frame.Height) | ||
| 334 | client.Write(terminalBytes) | 334 | client.Write(terminalBytes) | ||
| 335 | require.Equal(t, frameRows(composed.frame), frameRows(client.Frame)) | 335 | require.Equal(t, frameRows(composed.frame), frameRows(client.Frame)) | ||
| 336 | - | prepared.commitNoSend() | 336 | + | prepared.draw.Commit() |
| 337 | + | stream.forceSnapshot = false | |||
| 337 | again, err := stream.renderer.Draw(composed.frame, nil) | 338 | again, err := stream.renderer.Draw(composed.frame, nil) | ||
| 338 | require.NoError(t, err) | 339 | require.NoError(t, err) | ||
| 339 | require.Empty(t, again, "renderer shadow must exactly equal the composed frame") | 340 | require.Empty(t, again, "renderer shadow must exactly equal the composed frame") | ||
M internal/usecase/daemon/resume_test.go +2 -2
| @@ -1399,7 +1399,7 @@ func TestOutputAckLagAloneDoesNotForceFullStateRepaint(t *testing.T) { | |||
| 1399 | 1399 | reset := false | |
| 1400 | 1400 | require.False(t, reset, "reliable output ack lag alone must not force dependency-free full repaint") | |
| 1401 | 1401 | ||
| 1402 | - | f := ac.output.frame([]byte("incremental while reliable backlog drains"), reset, 0) | |
| 1402 | + | f := outputStateFrame(ac.output, []byte("incremental while reliable backlog drains"), reset, 0) | |
| 1403 | 1403 | ac.sendMu.Unlock() | |
| 1404 | 1404 | out, err := ports.UnmarshalOutput(f.Payload) | |
| 1405 | 1405 | require.NoError(t, err) | |
| @@ -1409,7 +1409,7 @@ func TestOutputAckLagAloneDoesNotForceFullStateRepaint(t *testing.T) { | |||
| 1409 | 1409 | ac.sendMu.Lock() | |
| 1410 | 1410 | reset = true | |
| 1411 | 1411 | require.True(t, reset, "explicit reset should still force full repaint") | |
| 1412 | - | full := ac.output.frame([]byte("explicit full repaint"), reset, 0) | |
| 1412 | + | full := outputStateFrame(ac.output, []byte("explicit full repaint"), reset, 0) | |
| 1413 | 1413 | ac.sendMu.Unlock() | |
| 1414 | 1414 | fullOut, err := ports.UnmarshalOutput(full.Payload) | |
| 1415 | 1415 | require.NoError(t, err) | |
| @@ -1399,7 +1399,7 @@ func TestOutputAckLagAloneDoesNotForceFullStateRepaint(t *testing.T) { | |||||
| 1399 | reset := false | 1399 | reset := false | ||
| 1400 | require.False(t, reset, "reliable output ack lag alone must not force dependency-free full repaint") | 1400 | require.False(t, reset, "reliable output ack lag alone must not force dependency-free full repaint") | ||
| 1401 | 1401 | ||||
| 1402 | - | f := ac.output.frame([]byte("incremental while reliable backlog drains"), reset, 0) | 1402 | + | f := outputStateFrame(ac.output, []byte("incremental while reliable backlog drains"), reset, 0) |
| 1403 | ac.sendMu.Unlock() | 1403 | ac.sendMu.Unlock() | ||
| 1404 | out, err := ports.UnmarshalOutput(f.Payload) | 1404 | out, err := ports.UnmarshalOutput(f.Payload) | ||
| 1405 | require.NoError(t, err) | 1405 | require.NoError(t, err) | ||
| @@ -1409,7 +1409,7 @@ func TestOutputAckLagAloneDoesNotForceFullStateRepaint(t *testing.T) { | |||||
| 1409 | ac.sendMu.Lock() | 1409 | ac.sendMu.Lock() | ||
| 1410 | reset = true | 1410 | reset = true | ||
| 1411 | require.True(t, reset, "explicit reset should still force full repaint") | 1411 | require.True(t, reset, "explicit reset should still force full repaint") | ||
| 1412 | - | full := ac.output.frame([]byte("explicit full repaint"), reset, 0) | 1412 | + | full := outputStateFrame(ac.output, []byte("explicit full repaint"), reset, 0) |
| 1413 | ac.sendMu.Unlock() | 1413 | ac.sendMu.Unlock() | ||
| 1414 | fullOut, err := ports.UnmarshalOutput(full.Payload) | 1414 | fullOut, err := ports.UnmarshalOutput(full.Payload) | ||
| 1415 | require.NoError(t, err) | 1415 | require.NoError(t, err) | ||
M internal/usecase/daemon/screen_output_stream.go +12 -10
| @@ -35,6 +35,10 @@ func (s *structuredOutputStream) prepare(frame renderer.Frame, damage []renderer | |||
| 35 | 35 | if err := frame.Validate(); err != nil { | |
| 36 | 36 | return nil, err | |
| 37 | 37 | } | |
| 38 | + | size := frameSize(frame) | |
| 39 | + | if !validProxyScreenSize(size) { | |
| 40 | + | return nil, errors.New("structured output: frame geometry is outside wire limits") | |
| 41 | + | } | |
| 38 | 42 | ||
| 39 | 43 | reset = reset || s.forceSnapshot || s.state.next == 0 || s.shadow.Validate() != nil || | |
| 40 | 44 | s.shadow.Width != frame.Width || s.shadow.Height != frame.Height | |
| @@ -91,7 +95,7 @@ func (s *structuredOutputStream) prepare(frame renderer.Frame, damage []renderer | |||
| 91 | 95 | } | |
| 92 | 96 | update.Spans = make([]ports.ScreenSpan, 0, len(plan.Spans)) | |
| 93 | 97 | for _, span := range plan.Spans { | |
| 94 | - | if span.Y < 0 || span.X < 0 || span.Width <= 0 || span.Y >= frame.Height || span.X+span.Width > frame.Width { | |
| 98 | + | if span.Y < 0 || span.X < 0 || span.Width <= 0 || span.Y >= frame.Height || span.X > frame.Width-span.Width { | |
| 95 | 99 | return nil, errors.New("structured output: invalid planned span") | |
| 96 | 100 | } | |
| 97 | 101 | update.Spans = append(update.Spans, ports.ScreenSpan{ | |
| @@ -156,43 +160,41 @@ type preparedStructuredOutput struct { | |||
| 156 | 160 | echoAck uint64 | |
| 157 | 161 | data []byte | |
| 158 | 162 | snapshot bool | |
| 159 | - | completed bool | |
| 163 | + | attempted bool | |
| 160 | 164 | committed bool | |
| 161 | 165 | } | |
| 162 | 166 | ||
| 163 | 167 | func (p *preparedStructuredOutput) send(sender func(ports.Frame) error) error { | |
| 164 | - | if p == nil || p.completed { | |
| 168 | + | if p == nil || p.attempted { | |
| 165 | 169 | return nil | |
| 166 | 170 | } | |
| 167 | 171 | if len(p.data) == 0 { | |
| 168 | - | p.completed = true | |
| 172 | + | p.attempted = true | |
| 169 | 173 | p.commit() | |
| 170 | 174 | return nil | |
| 171 | 175 | } | |
| 176 | + | p.attempted = true | |
| 172 | 177 | if sender == nil { | |
| 173 | - | p.completed = true | |
| 174 | 178 | p.stream.forceSnapshot = true | |
| 175 | 179 | return errors.New("structured output: nil sender") | |
| 176 | 180 | } | |
| 177 | 181 | if err := sender(ports.Frame{Type: ports.MsgScreenUpdate, Payload: p.data}); err != nil { | |
| 178 | - | p.completed = true | |
| 179 | 182 | p.stream.forceSnapshot = true | |
| 180 | 183 | return err | |
| 181 | 184 | } | |
| 182 | - | p.completed = true | |
| 183 | 185 | p.commit() | |
| 184 | 186 | p.stream.state.next = p.next | |
| 185 | 187 | return nil | |
| 186 | 188 | } | |
| 187 | 189 | ||
| 188 | 190 | // commitNoSend commits a prepared noop without adding a state-bearing frame. | |
| 189 | - | // A failed send is already completed, so it cannot accidentally commit after | |
| 191 | + | // A failed send is already attempted, so it cannot accidentally commit after | |
| 190 | 192 | // the failure and thereby destroy the required snapshot retry. | |
| 191 | 193 | func (p *preparedStructuredOutput) commitNoSend() { | |
| 192 | - | if p == nil || p.completed { | |
| 194 | + | if p == nil || p.attempted || len(p.data) != 0 { | |
| 193 | 195 | return | |
| 194 | 196 | } | |
| 195 | - | p.completed = true | |
| 197 | + | p.attempted = true | |
| 196 | 198 | p.commit() | |
| 197 | 199 | } | |
| 198 | 200 | ||
| @@ -35,6 +35,10 @@ func (s *structuredOutputStream) prepare(frame renderer.Frame, damage []renderer | |||||
| 35 | if err := frame.Validate(); err != nil { | 35 | if err := frame.Validate(); err != nil { | ||
| 36 | return nil, err | 36 | return nil, err | ||
| 37 | } | 37 | } | ||
| 38 | + | size := frameSize(frame) | |||
| 39 | + | if !validProxyScreenSize(size) { | |||
| 40 | + | return nil, errors.New("structured output: frame geometry is outside wire limits") | |||
| 41 | + | } | |||
| 38 | 42 | ||||
| 39 | reset = reset || s.forceSnapshot || s.state.next == 0 || s.shadow.Validate() != nil || | 43 | reset = reset || s.forceSnapshot || s.state.next == 0 || s.shadow.Validate() != nil || | ||
| 40 | s.shadow.Width != frame.Width || s.shadow.Height != frame.Height | 44 | s.shadow.Width != frame.Width || s.shadow.Height != frame.Height | ||
| @@ -91,7 +95,7 @@ func (s *structuredOutputStream) prepare(frame renderer.Frame, damage []renderer | |||||
| 91 | } | 95 | } | ||
| 92 | update.Spans = make([]ports.ScreenSpan, 0, len(plan.Spans)) | 96 | update.Spans = make([]ports.ScreenSpan, 0, len(plan.Spans)) | ||
| 93 | for _, span := range plan.Spans { | 97 | for _, span := range plan.Spans { | ||
| 94 | - | if span.Y < 0 || span.X < 0 || span.Width <= 0 || span.Y >= frame.Height || span.X+span.Width > frame.Width { | 98 | + | if span.Y < 0 || span.X < 0 || span.Width <= 0 || span.Y >= frame.Height || span.X > frame.Width-span.Width { |
| 95 | return nil, errors.New("structured output: invalid planned span") | 99 | return nil, errors.New("structured output: invalid planned span") | ||
| 96 | } | 100 | } | ||
| 97 | update.Spans = append(update.Spans, ports.ScreenSpan{ | 101 | update.Spans = append(update.Spans, ports.ScreenSpan{ | ||
| @@ -156,43 +160,41 @@ type preparedStructuredOutput struct { | |||||
| 156 | echoAck uint64 | 160 | echoAck uint64 | ||
| 157 | data []byte | 161 | data []byte | ||
| 158 | snapshot bool | 162 | snapshot bool | ||
| 159 | - | completed bool | 163 | + | attempted bool |
| 160 | committed bool | 164 | committed bool | ||
| 161 | } | 165 | } | ||
| 162 | 166 | ||||
| 163 | func (p *preparedStructuredOutput) send(sender func(ports.Frame) error) error { | 167 | func (p *preparedStructuredOutput) send(sender func(ports.Frame) error) error { | ||
| 164 | - | if p == nil || p.completed { | 168 | + | if p == nil || p.attempted { |
| 165 | return nil | 169 | return nil | ||
| 166 | } | 170 | } | ||
| 167 | if len(p.data) == 0 { | 171 | if len(p.data) == 0 { | ||
| 168 | - | p.completed = true | 172 | + | p.attempted = true |
| 169 | p.commit() | 173 | p.commit() | ||
| 170 | return nil | 174 | return nil | ||
| 171 | } | 175 | } | ||
| 176 | + | p.attempted = true | |||
| 172 | if sender == nil { | 177 | if sender == nil { | ||
| 173 | - | p.completed = true | |||
| 174 | p.stream.forceSnapshot = true | 178 | p.stream.forceSnapshot = true | ||
| 175 | return errors.New("structured output: nil sender") | 179 | return errors.New("structured output: nil sender") | ||
| 176 | } | 180 | } | ||
| 177 | if err := sender(ports.Frame{Type: ports.MsgScreenUpdate, Payload: p.data}); err != nil { | 181 | if err := sender(ports.Frame{Type: ports.MsgScreenUpdate, Payload: p.data}); err != nil { | ||
| 178 | - | p.completed = true | |||
| 179 | p.stream.forceSnapshot = true | 182 | p.stream.forceSnapshot = true | ||
| 180 | return err | 183 | return err | ||
| 181 | } | 184 | } | ||
| 182 | - | p.completed = true | |||
| 183 | p.commit() | 185 | p.commit() | ||
| 184 | p.stream.state.next = p.next | 186 | p.stream.state.next = p.next | ||
| 185 | return nil | 187 | return nil | ||
| 186 | } | 188 | } | ||
| 187 | 189 | ||||
| 188 | // commitNoSend commits a prepared noop without adding a state-bearing frame. | 190 | // commitNoSend commits a prepared noop without adding a state-bearing frame. | ||
| 189 | - | // A failed send is already completed, so it cannot accidentally commit after | 191 | + | // A failed send is already attempted, so it cannot accidentally commit after |
| 190 | // the failure and thereby destroy the required snapshot retry. | 192 | // the failure and thereby destroy the required snapshot retry. | ||
| 191 | func (p *preparedStructuredOutput) commitNoSend() { | 193 | func (p *preparedStructuredOutput) commitNoSend() { | ||
| 192 | - | if p == nil || p.completed { | 194 | + | if p == nil || p.attempted || len(p.data) != 0 { |
| 193 | return | 195 | return | ||
| 194 | } | 196 | } | ||
| 195 | - | p.completed = true | 197 | + | p.attempted = true |
| 196 | p.commit() | 198 | p.commit() | ||
| 197 | } | 199 | } | ||
| 198 | 200 | ||||
M internal/usecase/daemon/screen_output_stream_test.go +30 -3
| @@ -2,6 +2,7 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "errors" | |
| 5 | + | "math" | |
| 5 | 6 | "testing" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/vev/internal/ports" | |
| @@ -26,6 +27,10 @@ func TestStructuredOutputStreamSnapshotsThenDeltas(t *testing.T) { | |||
| 26 | 27 | require.Equal(t, uint16(y), span.Y) | |
| 27 | 28 | require.Zero(t, span.X) | |
| 28 | 29 | require.Len(t, span.Cells, frame.Width) | |
| 30 | + | for x, cell := range span.Cells { | |
| 31 | + | require.Equal(t, frame.At(x, y).Rune, cell.Rune) | |
| 32 | + | require.Equal(t, frame.At(x, y).Continuation, cell.Continuation) | |
| 33 | + | } | |
| 29 | 34 | } | |
| 30 | 35 | require.NoError(t, first.send(func(got ports.Frame) error { | |
| 31 | 36 | require.Equal(t, ports.MsgScreenUpdate, got.Type) | |
| @@ -210,6 +215,27 @@ func TestStructuredOutputStreamComparesCursorValidityAndHidden(t *testing.T) { | |||
| 210 | 215 | require.Empty(t, noop.data) | |
| 211 | 216 | } | |
| 212 | 217 | ||
| 218 | + | func TestStructuredOutputStreamRejectsInvalidCursorAndGeometry(t *testing.T) { | |
| 219 | + | cases := []struct { | |
| 220 | + | name string | |
| 221 | + | frame renderer.Frame | |
| 222 | + | cursor cursorOut | |
| 223 | + | }{ | |
| 224 | + | {name: "cursor row outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, row: 1}}, | |
| 225 | + | {name: "cursor col outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, col: 1}}, | |
| 226 | + | {name: "screen cell cap", frame: renderer.NewFrame(513, 512), cursor: cursorOut{valid: true}}, | |
| 227 | + | {name: "uint16 dimension cap", frame: renderer.NewFrame(math.MaxUint16+1, 1), cursor: cursorOut{valid: true}}, | |
| 228 | + | } | |
| 229 | + | for _, tc := range cases { | |
| 230 | + | t.Run(tc.name, func(t *testing.T) { | |
| 231 | + | stream := newStructuredOutputStream(newOutputStateStream()) | |
| 232 | + | _, err := stream.prepare(tc.frame, nil, tc.cursor, true, 0) | |
| 233 | + | require.Error(t, err) | |
| 234 | + | require.Zero(t, stream.state.next, "rejected geometry must not advance state") | |
| 235 | + | }) | |
| 236 | + | } | |
| 237 | + | } | |
| 238 | + | ||
| 213 | 239 | func TestStructuredOutputStreamPreservesExplicitZeroCursorStyle(t *testing.T) { | |
| 214 | 240 | state := newOutputStateStream() | |
| 215 | 241 | stream := newStructuredOutputStream(state) | |
| @@ -308,10 +334,11 @@ func BenchmarkStructuredOutputPrepare(b *testing.B) { | |||
| 308 | 334 | if err := initial.send(func(ports.Frame) error { return nil }); err != nil { | |
| 309 | 335 | b.Fatal(err) | |
| 310 | 336 | } | |
| 311 | - | var wireBytes, spans, snapshots int | |
| 337 | + | var wireBytes, spans, snapshots, mutation int | |
| 312 | 338 | b.ResetTimer() | |
| 313 | - | for i := 0; i < b.N; i++ { | |
| 314 | - | fixture.mutate(fixture.frame, i) | |
| 339 | + | for b.Loop() { | |
| 340 | + | fixture.mutate(fixture.frame, mutation) | |
| 341 | + | mutation++ | |
| 315 | 342 | prepared, err := stream.prepare(fixture.frame, fixture.damage, cursor, false, 0) | |
| 316 | 343 | if err != nil { | |
| 317 | 344 | b.Fatal(err) | |
| @@ -2,6 +2,7 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "errors" | 4 | "errors" | ||
| 5 | + | "math" | |||
| 5 | "testing" | 6 | "testing" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/vev/internal/ports" | 8 | "github.com/bnema/vev/internal/ports" | ||
| @@ -26,6 +27,10 @@ func TestStructuredOutputStreamSnapshotsThenDeltas(t *testing.T) { | |||||
| 26 | require.Equal(t, uint16(y), span.Y) | 27 | require.Equal(t, uint16(y), span.Y) | ||
| 27 | require.Zero(t, span.X) | 28 | require.Zero(t, span.X) | ||
| 28 | require.Len(t, span.Cells, frame.Width) | 29 | require.Len(t, span.Cells, frame.Width) | ||
| 30 | + | for x, cell := range span.Cells { | |||
| 31 | + | require.Equal(t, frame.At(x, y).Rune, cell.Rune) | |||
| 32 | + | require.Equal(t, frame.At(x, y).Continuation, cell.Continuation) | |||
| 33 | + | } | |||
| 29 | } | 34 | } | ||
| 30 | require.NoError(t, first.send(func(got ports.Frame) error { | 35 | require.NoError(t, first.send(func(got ports.Frame) error { | ||
| 31 | require.Equal(t, ports.MsgScreenUpdate, got.Type) | 36 | require.Equal(t, ports.MsgScreenUpdate, got.Type) | ||
| @@ -210,6 +215,27 @@ func TestStructuredOutputStreamComparesCursorValidityAndHidden(t *testing.T) { | |||||
| 210 | require.Empty(t, noop.data) | 215 | require.Empty(t, noop.data) | ||
| 211 | } | 216 | } | ||
| 212 | 217 | ||||
| 218 | + | func TestStructuredOutputStreamRejectsInvalidCursorAndGeometry(t *testing.T) { | |||
| 219 | + | cases := []struct { | |||
| 220 | + | name string | |||
| 221 | + | frame renderer.Frame | |||
| 222 | + | cursor cursorOut | |||
| 223 | + | }{ | |||
| 224 | + | {name: "cursor row outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, row: 1}}, | |||
| 225 | + | {name: "cursor col outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, col: 1}}, | |||
| 226 | + | {name: "screen cell cap", frame: renderer.NewFrame(513, 512), cursor: cursorOut{valid: true}}, | |||
| 227 | + | {name: "uint16 dimension cap", frame: renderer.NewFrame(math.MaxUint16+1, 1), cursor: cursorOut{valid: true}}, | |||
| 228 | + | } | |||
| 229 | + | for _, tc := range cases { | |||
| 230 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 231 | + | stream := newStructuredOutputStream(newOutputStateStream()) | |||
| 232 | + | _, err := stream.prepare(tc.frame, nil, tc.cursor, true, 0) | |||
| 233 | + | require.Error(t, err) | |||
| 234 | + | require.Zero(t, stream.state.next, "rejected geometry must not advance state") | |||
| 235 | + | }) | |||
| 236 | + | } | |||
| 237 | + | } | |||
| 238 | + | ||||
| 213 | func TestStructuredOutputStreamPreservesExplicitZeroCursorStyle(t *testing.T) { | 239 | func TestStructuredOutputStreamPreservesExplicitZeroCursorStyle(t *testing.T) { | ||
| 214 | state := newOutputStateStream() | 240 | state := newOutputStateStream() | ||
| 215 | stream := newStructuredOutputStream(state) | 241 | stream := newStructuredOutputStream(state) | ||
| @@ -308,10 +334,11 @@ func BenchmarkStructuredOutputPrepare(b *testing.B) { | |||||
| 308 | if err := initial.send(func(ports.Frame) error { return nil }); err != nil { | 334 | if err := initial.send(func(ports.Frame) error { return nil }); err != nil { | ||
| 309 | b.Fatal(err) | 335 | b.Fatal(err) | ||
| 310 | } | 336 | } | ||
| 311 | - | var wireBytes, spans, snapshots int | 337 | + | var wireBytes, spans, snapshots, mutation int |
| 312 | b.ResetTimer() | 338 | b.ResetTimer() | ||
| 313 | - | for i := 0; i < b.N; i++ { | 339 | + | for b.Loop() { |
| 314 | - | fixture.mutate(fixture.frame, i) | 340 | + | fixture.mutate(fixture.frame, mutation) |
| 341 | + | mutation++ | |||
| 315 | prepared, err := stream.prepare(fixture.frame, fixture.damage, cursor, false, 0) | 342 | prepared, err := stream.prepare(fixture.frame, fixture.damage, cursor, false, 0) | ||
| 316 | if err != nil { | 343 | if err != nil { | ||
| 317 | b.Fatal(err) | 344 | b.Fatal(err) | ||
M internal/usecase/daemon/session_meta_test.go +6 -1
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "errors" | |
| 5 | 5 | "io" | |
| 6 | 6 | "testing" | |
| 7 | + | "time" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/vev/internal/ports" | |
| 9 | 10 | "github.com/stretchr/testify/require" | |
| @@ -83,7 +84,11 @@ func TestProxiedMetadataSendsWithoutOutputBytes(t *testing.T) { | |||
| 83 | 84 | meta, err := ports.UnmarshalSessionMeta(metaFrame.Payload) | |
| 84 | 85 | require.NoError(t, err) | |
| 85 | 86 | require.Equal(t, "renamed", meta.Tabs[0].Name) | |
| 86 | - | requireNoOutputFrame(t, sends) | |
| 87 | + | select { | |
| 88 | + | case frame := <-sends: | |
| 89 | + | t.Fatalf("unexpected frame after metadata-only paint: %v", frame.Type) | |
| 90 | + | case <-time.After(50 * time.Millisecond): | |
| 91 | + | } | |
| 87 | 92 | } | |
| 88 | 93 | ||
| 89 | 94 | func TestSessionMetaSnapshotIsImmutable(t *testing.T) { | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "errors" | 4 | "errors" | ||
| 5 | "io" | 5 | "io" | ||
| 6 | "testing" | 6 | "testing" | ||
| 7 | + | "time" | |||
| 7 | 8 | ||||
| 8 | "github.com/bnema/vev/internal/ports" | 9 | "github.com/bnema/vev/internal/ports" | ||
| 9 | "github.com/stretchr/testify/require" | 10 | "github.com/stretchr/testify/require" | ||
| @@ -83,7 +84,11 @@ func TestProxiedMetadataSendsWithoutOutputBytes(t *testing.T) { | |||||
| 83 | meta, err := ports.UnmarshalSessionMeta(metaFrame.Payload) | 84 | meta, err := ports.UnmarshalSessionMeta(metaFrame.Payload) | ||
| 84 | require.NoError(t, err) | 85 | require.NoError(t, err) | ||
| 85 | require.Equal(t, "renamed", meta.Tabs[0].Name) | 86 | require.Equal(t, "renamed", meta.Tabs[0].Name) | ||
| 86 | - | requireNoOutputFrame(t, sends) | 87 | + | select { |
| 88 | + | case frame := <-sends: | |||
| 89 | + | t.Fatalf("unexpected frame after metadata-only paint: %v", frame.Type) | |||
| 90 | + | case <-time.After(50 * time.Millisecond): | |||
| 91 | + | } | |||
| 87 | } | 92 | } | ||
| 88 | 93 | ||||
| 89 | func TestSessionMetaSnapshotIsImmutable(t *testing.T) { | 94 | func TestSessionMetaSnapshotIsImmutable(t *testing.T) { | ||
M internal/usecase/daemon/snatched_render_test.go +2 -0
| @@ -142,6 +142,7 @@ func TestSendSnatchedPanelUsesStructuredSnapshotForProxied(t *testing.T) { | |||
| 142 | 142 | d := newTestDaemon(t, nil, stubClock{}) | |
| 143 | 143 | tr := &closeTrackingTransport{} | |
| 144 | 144 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), proxied: true, size: domain.Size{Cols: 80, Rows: 24}} | |
| 145 | + | ac.output.next = 7 | |
| 145 | 146 | ac.setAppliedTheme(appliedTheme{Resolved: themeui.Resolve(themeui.Theme{}, domain.ThemeAccent{Mode: domain.ThemeAccentAuto})}) | |
| 146 | 147 | ac.roleGeneration.Store(1) | |
| 147 | 148 | ||
| @@ -153,6 +154,7 @@ func TestSendSnatchedPanelUsesStructuredSnapshotForProxied(t *testing.T) { | |||
| 153 | 154 | require.NoError(t, err) | |
| 154 | 155 | require.Equal(t, ports.ScreenUpdateSnapshot, update.Kind) | |
| 155 | 156 | require.Zero(t, update.BaseStateNum) | |
| 157 | + | require.Equal(t, uint64(8), update.NewStateNum, "snatched panel must advance the shared output state") | |
| 156 | 158 | require.False(t, update.Cursor.Visible, "structured snatched panel must hide the cursor semantically") | |
| 157 | 159 | require.Len(t, update.Spans, ac.size.Rows, "structured snapshots must carry every full row") | |
| 158 | 160 | } | |
| @@ -142,6 +142,7 @@ func TestSendSnatchedPanelUsesStructuredSnapshotForProxied(t *testing.T) { | |||||
| 142 | d := newTestDaemon(t, nil, stubClock{}) | 142 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 143 | tr := &closeTrackingTransport{} | 143 | tr := &closeTrackingTransport{} | ||
| 144 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), proxied: true, size: domain.Size{Cols: 80, Rows: 24}} | 144 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), proxied: true, size: domain.Size{Cols: 80, Rows: 24}} | ||
| 145 | + | ac.output.next = 7 | |||
| 145 | ac.setAppliedTheme(appliedTheme{Resolved: themeui.Resolve(themeui.Theme{}, domain.ThemeAccent{Mode: domain.ThemeAccentAuto})}) | 146 | ac.setAppliedTheme(appliedTheme{Resolved: themeui.Resolve(themeui.Theme{}, domain.ThemeAccent{Mode: domain.ThemeAccentAuto})}) | ||
| 146 | ac.roleGeneration.Store(1) | 147 | ac.roleGeneration.Store(1) | ||
| 147 | 148 | ||||
| @@ -153,6 +154,7 @@ func TestSendSnatchedPanelUsesStructuredSnapshotForProxied(t *testing.T) { | |||||
| 153 | require.NoError(t, err) | 154 | require.NoError(t, err) | ||
| 154 | require.Equal(t, ports.ScreenUpdateSnapshot, update.Kind) | 155 | require.Equal(t, ports.ScreenUpdateSnapshot, update.Kind) | ||
| 155 | require.Zero(t, update.BaseStateNum) | 156 | require.Zero(t, update.BaseStateNum) | ||
| 157 | + | require.Equal(t, uint64(8), update.NewStateNum, "snatched panel must advance the shared output state") | |||
| 156 | require.False(t, update.Cursor.Visible, "structured snatched panel must hide the cursor semantically") | 158 | require.False(t, update.Cursor.Visible, "structured snatched panel must hide the cursor semantically") | ||
| 157 | require.Len(t, update.Spans, ac.size.Rows, "structured snapshots must carry every full row") | 159 | require.Len(t, update.Spans, ac.size.Rows, "structured snapshots must carry every full row") | ||
| 158 | } | 160 | } | ||
M internal/usecase/daemon/structured_diagnostics_test.go +9 -10
| @@ -40,7 +40,7 @@ func TestP4StructuredRuntimeDiagnosticsRecordAcceptedSnapshotAndDelta(t *testing | |||
| 40 | 40 | } | |
| 41 | 41 | ||
| 42 | 42 | snapshot := emit(true) | |
| 43 | - | marks := structuredScreenMarks(observer) | |
| 43 | + | marks := structuredScreenMarks(t, observer) | |
| 44 | 44 | require.Len(t, marks, 1) | |
| 45 | 45 | require.Equal(t, ports.RuntimeScreenSnapshot, marks[0].Kind) | |
| 46 | 46 | require.Equal(t, uint64(len(mustMarshalScreenUpdate(t, snapshot))), marks[0].Bytes) | |
| @@ -57,7 +57,7 @@ func TestP4StructuredRuntimeDiagnosticsRecordAcceptedSnapshotAndDelta(t *testing | |||
| 57 | 57 | observer.marks = nil | |
| 58 | 58 | ||
| 59 | 59 | delta := emit(false) | |
| 60 | - | marks = structuredScreenMarks(observer) | |
| 60 | + | marks = structuredScreenMarks(t, observer) | |
| 61 | 61 | require.Len(t, marks, 1) | |
| 62 | 62 | require.Equal(t, ports.RuntimeScreenDelta, marks[0].Kind) | |
| 63 | 63 | require.Equal(t, uint64(len(mustMarshalScreenUpdate(t, delta))), marks[0].Bytes) | |
| @@ -79,7 +79,7 @@ func TestP4StructuredRuntimeDiagnosticsDoNotRecordFailedSend(t *testing.T) { | |||
| 79 | 79 | require.True(t, ok) | |
| 80 | 80 | composed := composeFrame(*state, composeCacheInput{}) | |
| 81 | 81 | require.True(t, d.emitFrame(proxy, ac, state, composed)) | |
| 82 | - | require.Empty(t, structuredScreenMarks(observer)) | |
| 82 | + | require.Empty(t, structuredScreenMarks(t, observer)) | |
| 83 | 83 | } | |
| 84 | 84 | ||
| 85 | 85 | func TestP4StructuredRuntimeDiagnosticsResetOnlyAfterAcceptedRequest(t *testing.T) { | |
| @@ -91,7 +91,7 @@ func TestP4StructuredRuntimeDiagnosticsResetOnlyAfterAcceptedRequest(t *testing. | |||
| 91 | 91 | Size: domain.Size{Cols: 80, Rows: 22}, | |
| 92 | 92 | } | |
| 93 | 93 | require.NoError(t, d.handleProxyScreenUpdate(proxy, generation, invalid)) | |
| 94 | - | marks := structuredScreenMarks(observer) | |
| 94 | + | marks := structuredScreenMarks(t, observer) | |
| 95 | 95 | require.Len(t, marks, 1) | |
| 96 | 96 | require.Equal(t, ports.RuntimeScreenResetRequested, marks[0].Kind) | |
| 97 | 97 | require.Zero(t, marks[0].Bytes) | |
| @@ -100,13 +100,13 @@ func TestP4StructuredRuntimeDiagnosticsResetOnlyAfterAcceptedRequest(t *testing. | |||
| 100 | 100 | ||
| 101 | 101 | // The already-requested reset is not sent or measured a second time. | |
| 102 | 102 | require.NoError(t, d.handleProxyScreenUpdate(proxy, generation, invalid)) | |
| 103 | - | require.Len(t, structuredScreenMarks(observer), 1) | |
| 103 | + | require.Len(t, structuredScreenMarks(t, observer), 1) | |
| 104 | 104 | ||
| 105 | 105 | d, proxy, transport, generation = newProxyOutputSession(t) | |
| 106 | 106 | d.runtimeObserver = &daemonRuntimeObserver{} | |
| 107 | 107 | transport.sendFails.Store(true) | |
| 108 | 108 | require.Error(t, d.handleProxyScreenUpdate(proxy, generation, invalid)) | |
| 109 | - | require.Empty(t, structuredScreenMarks(d.runtimeObserver.(*daemonRuntimeObserver))) | |
| 109 | + | require.Empty(t, structuredScreenMarks(t, d.runtimeObserver)) | |
| 110 | 110 | } | |
| 111 | 111 | ||
| 112 | 112 | func TestP4StructuredRuntimeDiagnosticsObserverRunsOutsideAttachmentAndProxyLocks(t *testing.T) { | |
| @@ -155,11 +155,10 @@ func TestP4StructuredRuntimeDiagnosticsObserverRunsOutsideAttachmentAndProxyLock | |||
| 155 | 155 | awaitDaemonObserver(t, done, "structured render") | |
| 156 | 156 | } | |
| 157 | 157 | ||
| 158 | - | func structuredScreenMarks(observer ports.RuntimeObserver) []ports.RuntimeMark { | |
| 158 | + | func structuredScreenMarks(t *testing.T, observer ports.RuntimeObserver) []ports.RuntimeMark { | |
| 159 | + | t.Helper() | |
| 159 | 160 | collector, ok := observer.(*daemonRuntimeObserver) | |
| 160 | - | if !ok { | |
| 161 | - | return nil | |
| 162 | - | } | |
| 161 | + | require.True(t, ok, "runtime observer must be the daemon test collector") | |
| 163 | 162 | var marks []ports.RuntimeMark | |
| 164 | 163 | for _, mark := range collector.marks { | |
| 165 | 164 | switch mark.Kind { | |
| @@ -40,7 +40,7 @@ func TestP4StructuredRuntimeDiagnosticsRecordAcceptedSnapshotAndDelta(t *testing | |||||
| 40 | } | 40 | } | ||
| 41 | 41 | ||||
| 42 | snapshot := emit(true) | 42 | snapshot := emit(true) | ||
| 43 | - | marks := structuredScreenMarks(observer) | 43 | + | marks := structuredScreenMarks(t, observer) |
| 44 | require.Len(t, marks, 1) | 44 | require.Len(t, marks, 1) | ||
| 45 | require.Equal(t, ports.RuntimeScreenSnapshot, marks[0].Kind) | 45 | require.Equal(t, ports.RuntimeScreenSnapshot, marks[0].Kind) | ||
| 46 | require.Equal(t, uint64(len(mustMarshalScreenUpdate(t, snapshot))), marks[0].Bytes) | 46 | require.Equal(t, uint64(len(mustMarshalScreenUpdate(t, snapshot))), marks[0].Bytes) | ||
| @@ -57,7 +57,7 @@ func TestP4StructuredRuntimeDiagnosticsRecordAcceptedSnapshotAndDelta(t *testing | |||||
| 57 | observer.marks = nil | 57 | observer.marks = nil | ||
| 58 | 58 | ||||
| 59 | delta := emit(false) | 59 | delta := emit(false) | ||
| 60 | - | marks = structuredScreenMarks(observer) | 60 | + | marks = structuredScreenMarks(t, observer) |
| 61 | require.Len(t, marks, 1) | 61 | require.Len(t, marks, 1) | ||
| 62 | require.Equal(t, ports.RuntimeScreenDelta, marks[0].Kind) | 62 | require.Equal(t, ports.RuntimeScreenDelta, marks[0].Kind) | ||
| 63 | require.Equal(t, uint64(len(mustMarshalScreenUpdate(t, delta))), marks[0].Bytes) | 63 | require.Equal(t, uint64(len(mustMarshalScreenUpdate(t, delta))), marks[0].Bytes) | ||
| @@ -79,7 +79,7 @@ func TestP4StructuredRuntimeDiagnosticsDoNotRecordFailedSend(t *testing.T) { | |||||
| 79 | require.True(t, ok) | 79 | require.True(t, ok) | ||
| 80 | composed := composeFrame(*state, composeCacheInput{}) | 80 | composed := composeFrame(*state, composeCacheInput{}) | ||
| 81 | require.True(t, d.emitFrame(proxy, ac, state, composed)) | 81 | require.True(t, d.emitFrame(proxy, ac, state, composed)) | ||
| 82 | - | require.Empty(t, structuredScreenMarks(observer)) | 82 | + | require.Empty(t, structuredScreenMarks(t, observer)) |
| 83 | } | 83 | } | ||
| 84 | 84 | ||||
| 85 | func TestP4StructuredRuntimeDiagnosticsResetOnlyAfterAcceptedRequest(t *testing.T) { | 85 | func TestP4StructuredRuntimeDiagnosticsResetOnlyAfterAcceptedRequest(t *testing.T) { | ||
| @@ -91,7 +91,7 @@ func TestP4StructuredRuntimeDiagnosticsResetOnlyAfterAcceptedRequest(t *testing. | |||||
| 91 | Size: domain.Size{Cols: 80, Rows: 22}, | 91 | Size: domain.Size{Cols: 80, Rows: 22}, | ||
| 92 | } | 92 | } | ||
| 93 | require.NoError(t, d.handleProxyScreenUpdate(proxy, generation, invalid)) | 93 | require.NoError(t, d.handleProxyScreenUpdate(proxy, generation, invalid)) | ||
| 94 | - | marks := structuredScreenMarks(observer) | 94 | + | marks := structuredScreenMarks(t, observer) |
| 95 | require.Len(t, marks, 1) | 95 | require.Len(t, marks, 1) | ||
| 96 | require.Equal(t, ports.RuntimeScreenResetRequested, marks[0].Kind) | 96 | require.Equal(t, ports.RuntimeScreenResetRequested, marks[0].Kind) | ||
| 97 | require.Zero(t, marks[0].Bytes) | 97 | require.Zero(t, marks[0].Bytes) | ||
| @@ -100,13 +100,13 @@ func TestP4StructuredRuntimeDiagnosticsResetOnlyAfterAcceptedRequest(t *testing. | |||||
| 100 | 100 | ||||
| 101 | // The already-requested reset is not sent or measured a second time. | 101 | // The already-requested reset is not sent or measured a second time. | ||
| 102 | require.NoError(t, d.handleProxyScreenUpdate(proxy, generation, invalid)) | 102 | require.NoError(t, d.handleProxyScreenUpdate(proxy, generation, invalid)) | ||
| 103 | - | require.Len(t, structuredScreenMarks(observer), 1) | 103 | + | require.Len(t, structuredScreenMarks(t, observer), 1) |
| 104 | 104 | ||||
| 105 | d, proxy, transport, generation = newProxyOutputSession(t) | 105 | d, proxy, transport, generation = newProxyOutputSession(t) | ||
| 106 | d.runtimeObserver = &daemonRuntimeObserver{} | 106 | d.runtimeObserver = &daemonRuntimeObserver{} | ||
| 107 | transport.sendFails.Store(true) | 107 | transport.sendFails.Store(true) | ||
| 108 | require.Error(t, d.handleProxyScreenUpdate(proxy, generation, invalid)) | 108 | require.Error(t, d.handleProxyScreenUpdate(proxy, generation, invalid)) | ||
| 109 | - | require.Empty(t, structuredScreenMarks(d.runtimeObserver.(*daemonRuntimeObserver))) | 109 | + | require.Empty(t, structuredScreenMarks(t, d.runtimeObserver)) |
| 110 | } | 110 | } | ||
| 111 | 111 | ||||
| 112 | func TestP4StructuredRuntimeDiagnosticsObserverRunsOutsideAttachmentAndProxyLocks(t *testing.T) { | 112 | func TestP4StructuredRuntimeDiagnosticsObserverRunsOutsideAttachmentAndProxyLocks(t *testing.T) { | ||
| @@ -155,11 +155,10 @@ func TestP4StructuredRuntimeDiagnosticsObserverRunsOutsideAttachmentAndProxyLock | |||||
| 155 | awaitDaemonObserver(t, done, "structured render") | 155 | awaitDaemonObserver(t, done, "structured render") | ||
| 156 | } | 156 | } | ||
| 157 | 157 | ||||
| 158 | - | func structuredScreenMarks(observer ports.RuntimeObserver) []ports.RuntimeMark { | 158 | + | func structuredScreenMarks(t *testing.T, observer ports.RuntimeObserver) []ports.RuntimeMark { |
| 159 | + | t.Helper() | |||
| 159 | collector, ok := observer.(*daemonRuntimeObserver) | 160 | collector, ok := observer.(*daemonRuntimeObserver) | ||
| 160 | - | if !ok { | 161 | + | require.True(t, ok, "runtime observer must be the daemon test collector") |
| 161 | - | return nil | |||
| 162 | - | } | |||
| 163 | var marks []ports.RuntimeMark | 162 | var marks []ports.RuntimeMark | ||
| 164 | for _, mark := range collector.marks { | 163 | for _, mark := range collector.marks { | ||
| 165 | switch mark.Kind { | 164 | switch mark.Kind { | ||