fix(daemon): repaint remote metadata updates
- commit
- c48b08e66e40d84f80616a81e6b99a98d1399ddd
- author
- brice · · 2026-08-06 20:12 UTC
- committer
- brice · · 2026-08-06 20:14 UTC
- parent
- 8d7a354
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +236 -18
M internal/usecase/daemon/remote_link.go +59 -18
| @@ -397,27 +397,54 @@ func cloneRemoteSessionMeta(metadata ports.SessionMeta) ports.SessionMeta { | |||
| 397 | 397 | return metadata | |
| 398 | 398 | } | |
| 399 | 399 | ||
| 400 | - | // applyRemoteMetadata commits one validated update only for the exact current | |
| 401 | - | // link. The view lock protects content state only; no renderer or callback runs | |
| 402 | - | // before it is released. | |
| 400 | + | type remoteMetadataResult uint8 | |
| 401 | + | ||
| 402 | + | const ( | |
| 403 | + | remoteMetadataInvalid remoteMetadataResult = iota | |
| 404 | + | remoteMetadataStale | |
| 405 | + | remoteMetadataAccepted | |
| 406 | + | ) | |
| 407 | + | ||
| 408 | + | // applyRemoteMetadata retains the boolean helper used by initial handshake | |
| 409 | + | // callers. The detailed result is used by the live-link frame handler so a | |
| 410 | + | // valid stale revision can be ignored without stopping the link. | |
| 403 | 411 | func (v *remoteView) applyRemoteMetadata(link *remoteLink, metadata ports.SessionMeta, initial bool) bool { | |
| 404 | - | if v == nil || link == nil { | |
| 405 | - | return false | |
| 412 | + | result, _ := v.applyRemoteMetadataWithAttachments(link, metadata, initial) | |
| 413 | + | return result == remoteMetadataAccepted | |
| 414 | + | } | |
| 415 | + | ||
| 416 | + | // applyRemoteMetadataWithAttachments commits one validated update only for the | |
| 417 | + | // exact current link. The view lock protects content state only; no renderer or | |
| 418 | + | // callback runs before it is released. Accepted updates return an attachment | |
| 419 | + | // snapshot for repainting after the lock is released. | |
| 420 | + | func (v *remoteView) applyRemoteMetadataWithAttachments(link *remoteLink, metadata ports.SessionMeta, initial bool) (remoteMetadataResult, []*attachedClient) { | |
| 421 | + | if v == nil || link == nil || ports.ValidateSessionMeta(metadata) != nil { | |
| 422 | + | return remoteMetadataInvalid, nil | |
| 423 | + | } | |
| 424 | + | if metadata.LifecycleID != link.target.LifecycleID || metadata.SessionName != link.target.SessionName { | |
| 425 | + | return remoteMetadataInvalid, nil | |
| 406 | 426 | } | |
| 407 | 427 | v.mu.Lock() | |
| 408 | 428 | defer v.mu.Unlock() | |
| 409 | 429 | if v.closed || v.link != link || v.linkGeneration != link.generation { | |
| 410 | - | return false | |
| 430 | + | return remoteMetadataInvalid, nil | |
| 411 | 431 | } | |
| 412 | 432 | if initial { | |
| 413 | - | if v.metadata.Revision != 0 { | |
| 414 | - | return false | |
| 433 | + | if v.metadata.Revision != 0 || metadata.Revision != 1 { | |
| 434 | + | return remoteMetadataInvalid, nil | |
| 415 | 435 | } | |
| 416 | 436 | } else if metadata.Revision <= v.metadata.Revision { | |
| 417 | - | return false | |
| 437 | + | return remoteMetadataStale, nil | |
| 418 | 438 | } | |
| 419 | 439 | v.metadata = cloneRemoteSessionMeta(metadata) | |
| 420 | - | return true | |
| 440 | + | if initial { | |
| 441 | + | return remoteMetadataAccepted, nil | |
| 442 | + | } | |
| 443 | + | attachments := make([]*attachedClient, 0, len(v.attachments)) | |
| 444 | + | for attachment := range v.attachments { | |
| 445 | + | attachments = append(attachments, attachment) | |
| 446 | + | } | |
| 447 | + | return remoteMetadataAccepted, attachments | |
| 421 | 448 | } | |
| 422 | 449 | ||
| 423 | 450 | // applyRemoteOutput validates and applies state-bearing ANSI to the persistent | |
| @@ -516,22 +543,23 @@ func (d *Daemon) handleRemoteLinkFrame(link *remoteLink, frame ports.Frame) erro | |||
| 516 | 543 | return fmt.Errorf("remote view: acknowledge output: %w", err) | |
| 517 | 544 | } | |
| 518 | 545 | } | |
| 519 | - | for _, attachment := range attachments { | |
| 520 | - | token := attachmentOwnerToken(link.view, attachment, attachment.transport()) | |
| 521 | - | if token.ac != nil { | |
| 522 | - | go d.paintRemoteView(link.view, attachment, false, token) | |
| 523 | - | } | |
| 524 | - | } | |
| 546 | + | d.repaintRemoteViewAttachments(link.view, attachments) | |
| 525 | 547 | return nil | |
| 526 | 548 | case ports.MsgSessionMeta: | |
| 527 | 549 | metadata, err := ports.UnmarshalSessionMeta(frame.Payload) | |
| 528 | 550 | if err != nil { | |
| 529 | 551 | return fmt.Errorf("remote view: decode metadata: %w", err) | |
| 530 | 552 | } | |
| 531 | - | if metadata.LifecycleID != link.target.LifecycleID || metadata.SessionName != link.target.SessionName || !link.view.applyRemoteMetadata(link, metadata, false) { | |
| 553 | + | result, attachments := link.view.applyRemoteMetadataWithAttachments(link, metadata, false) | |
| 554 | + | switch result { | |
| 555 | + | case remoteMetadataAccepted: | |
| 556 | + | d.repaintRemoteViewAttachments(link.view, attachments) | |
| 557 | + | return nil | |
| 558 | + | case remoteMetadataStale: | |
| 559 | + | return nil | |
| 560 | + | default: | |
| 532 | 561 | return errors.New("remote view: invalid metadata update") | |
| 533 | 562 | } | |
| 534 | - | return nil | |
| 535 | 563 | case ports.MsgPing: | |
| 536 | 564 | if _, err := ports.UnmarshalPing(frame.Payload); err != nil { | |
| 537 | 565 | return fmt.Errorf("remote view: decode ping: %w", err) | |
| @@ -557,6 +585,19 @@ func (d *Daemon) handleRemoteLinkFrame(link *remoteLink, frame ports.Frame) erro | |||
| 557 | 585 | } | |
| 558 | 586 | } | |
| 559 | 587 | ||
| 588 | + | // repaintRemoteViewAttachments starts one local-chrome repaint for every | |
| 589 | + | // attachment captured by an accepted remote update. The snapshot is taken | |
| 590 | + | // under view.mu, but all token validation, rendering callbacks, and transport | |
| 591 | + | // I/O happen after that lock has been released. | |
| 592 | + | func (d *Daemon) repaintRemoteViewAttachments(view *remoteView, attachments []*attachedClient) { | |
| 593 | + | for _, attachment := range attachments { | |
| 594 | + | token := attachmentOwnerToken(view, attachment, attachment.transport()) | |
| 595 | + | if token.ac != nil { | |
| 596 | + | go d.paintRemoteView(view, attachment, false, token) | |
| 597 | + | } | |
| 598 | + | } | |
| 599 | + | } | |
| 600 | + | ||
| 560 | 601 | // send serializes the complete outbound remote stream. It captures the exact | |
| 561 | 602 | // current transport under view.mu, releases every architecture lock, then lets | |
| 562 | 603 | // Close interrupt a blocked Send concurrently. | |
| @@ -397,27 +397,54 @@ func cloneRemoteSessionMeta(metadata ports.SessionMeta) ports.SessionMeta { | |||||
| 397 | return metadata | 397 | return metadata | ||
| 398 | } | 398 | } | ||
| 399 | 399 | ||||
| 400 | - | // applyRemoteMetadata commits one validated update only for the exact current | 400 | + | type remoteMetadataResult uint8 |
| 401 | - | // link. The view lock protects content state only; no renderer or callback runs | 401 | + | |
| 402 | - | // before it is released. | 402 | + | const ( |
| 403 | + | remoteMetadataInvalid remoteMetadataResult = iota | |||
| 404 | + | remoteMetadataStale | |||
| 405 | + | remoteMetadataAccepted | |||
| 406 | + | ) | |||
| 407 | + | ||||
| 408 | + | // applyRemoteMetadata retains the boolean helper used by initial handshake | |||
| 409 | + | // callers. The detailed result is used by the live-link frame handler so a | |||
| 410 | + | // valid stale revision can be ignored without stopping the link. | |||
| 403 | func (v *remoteView) applyRemoteMetadata(link *remoteLink, metadata ports.SessionMeta, initial bool) bool { | 411 | func (v *remoteView) applyRemoteMetadata(link *remoteLink, metadata ports.SessionMeta, initial bool) bool { | ||
| 404 | - | if v == nil || link == nil { | 412 | + | result, _ := v.applyRemoteMetadataWithAttachments(link, metadata, initial) |
| 405 | - | return false | 413 | + | return result == remoteMetadataAccepted |
| 414 | + | } | |||
| 415 | + | ||||
| 416 | + | // applyRemoteMetadataWithAttachments commits one validated update only for the | |||
| 417 | + | // exact current link. The view lock protects content state only; no renderer or | |||
| 418 | + | // callback runs before it is released. Accepted updates return an attachment | |||
| 419 | + | // snapshot for repainting after the lock is released. | |||
| 420 | + | func (v *remoteView) applyRemoteMetadataWithAttachments(link *remoteLink, metadata ports.SessionMeta, initial bool) (remoteMetadataResult, []*attachedClient) { | |||
| 421 | + | if v == nil || link == nil || ports.ValidateSessionMeta(metadata) != nil { | |||
| 422 | + | return remoteMetadataInvalid, nil | |||
| 423 | + | } | |||
| 424 | + | if metadata.LifecycleID != link.target.LifecycleID || metadata.SessionName != link.target.SessionName { | |||
| 425 | + | return remoteMetadataInvalid, nil | |||
| 406 | } | 426 | } | ||
| 407 | v.mu.Lock() | 427 | v.mu.Lock() | ||
| 408 | defer v.mu.Unlock() | 428 | defer v.mu.Unlock() | ||
| 409 | if v.closed || v.link != link || v.linkGeneration != link.generation { | 429 | if v.closed || v.link != link || v.linkGeneration != link.generation { | ||
| 410 | - | return false | 430 | + | return remoteMetadataInvalid, nil |
| 411 | } | 431 | } | ||
| 412 | if initial { | 432 | if initial { | ||
| 413 | - | if v.metadata.Revision != 0 { | 433 | + | if v.metadata.Revision != 0 || metadata.Revision != 1 { |
| 414 | - | return false | 434 | + | return remoteMetadataInvalid, nil |
| 415 | } | 435 | } | ||
| 416 | } else if metadata.Revision <= v.metadata.Revision { | 436 | } else if metadata.Revision <= v.metadata.Revision { | ||
| 417 | - | return false | 437 | + | return remoteMetadataStale, nil |
| 418 | } | 438 | } | ||
| 419 | v.metadata = cloneRemoteSessionMeta(metadata) | 439 | v.metadata = cloneRemoteSessionMeta(metadata) | ||
| 420 | - | return true | 440 | + | if initial { |
| 441 | + | return remoteMetadataAccepted, nil | |||
| 442 | + | } | |||
| 443 | + | attachments := make([]*attachedClient, 0, len(v.attachments)) | |||
| 444 | + | for attachment := range v.attachments { | |||
| 445 | + | attachments = append(attachments, attachment) | |||
| 446 | + | } | |||
| 447 | + | return remoteMetadataAccepted, attachments | |||
| 421 | } | 448 | } | ||
| 422 | 449 | ||||
| 423 | // applyRemoteOutput validates and applies state-bearing ANSI to the persistent | 450 | // applyRemoteOutput validates and applies state-bearing ANSI to the persistent | ||
| @@ -516,22 +543,23 @@ func (d *Daemon) handleRemoteLinkFrame(link *remoteLink, frame ports.Frame) erro | |||||
| 516 | return fmt.Errorf("remote view: acknowledge output: %w", err) | 543 | return fmt.Errorf("remote view: acknowledge output: %w", err) | ||
| 517 | } | 544 | } | ||
| 518 | } | 545 | } | ||
| 519 | - | for _, attachment := range attachments { | 546 | + | d.repaintRemoteViewAttachments(link.view, attachments) |
| 520 | - | token := attachmentOwnerToken(link.view, attachment, attachment.transport()) | |||
| 521 | - | if token.ac != nil { | |||
| 522 | - | go d.paintRemoteView(link.view, attachment, false, token) | |||
| 523 | - | } | |||
| 524 | - | } | |||
| 525 | return nil | 547 | return nil | ||
| 526 | case ports.MsgSessionMeta: | 548 | case ports.MsgSessionMeta: | ||
| 527 | metadata, err := ports.UnmarshalSessionMeta(frame.Payload) | 549 | metadata, err := ports.UnmarshalSessionMeta(frame.Payload) | ||
| 528 | if err != nil { | 550 | if err != nil { | ||
| 529 | return fmt.Errorf("remote view: decode metadata: %w", err) | 551 | return fmt.Errorf("remote view: decode metadata: %w", err) | ||
| 530 | } | 552 | } | ||
| 531 | - | if metadata.LifecycleID != link.target.LifecycleID || metadata.SessionName != link.target.SessionName || !link.view.applyRemoteMetadata(link, metadata, false) { | 553 | + | result, attachments := link.view.applyRemoteMetadataWithAttachments(link, metadata, false) |
| 554 | + | switch result { | |||
| 555 | + | case remoteMetadataAccepted: | |||
| 556 | + | d.repaintRemoteViewAttachments(link.view, attachments) | |||
| 557 | + | return nil | |||
| 558 | + | case remoteMetadataStale: | |||
| 559 | + | return nil | |||
| 560 | + | default: | |||
| 532 | return errors.New("remote view: invalid metadata update") | 561 | return errors.New("remote view: invalid metadata update") | ||
| 533 | } | 562 | } | ||
| 534 | - | return nil | |||
| 535 | case ports.MsgPing: | 563 | case ports.MsgPing: | ||
| 536 | if _, err := ports.UnmarshalPing(frame.Payload); err != nil { | 564 | if _, err := ports.UnmarshalPing(frame.Payload); err != nil { | ||
| 537 | return fmt.Errorf("remote view: decode ping: %w", err) | 565 | return fmt.Errorf("remote view: decode ping: %w", err) | ||
| @@ -557,6 +585,19 @@ func (d *Daemon) handleRemoteLinkFrame(link *remoteLink, frame ports.Frame) erro | |||||
| 557 | } | 585 | } | ||
| 558 | } | 586 | } | ||
| 559 | 587 | ||||
| 588 | + | // repaintRemoteViewAttachments starts one local-chrome repaint for every | |||
| 589 | + | // attachment captured by an accepted remote update. The snapshot is taken | |||
| 590 | + | // under view.mu, but all token validation, rendering callbacks, and transport | |||
| 591 | + | // I/O happen after that lock has been released. | |||
| 592 | + | func (d *Daemon) repaintRemoteViewAttachments(view *remoteView, attachments []*attachedClient) { | |||
| 593 | + | for _, attachment := range attachments { | |||
| 594 | + | token := attachmentOwnerToken(view, attachment, attachment.transport()) | |||
| 595 | + | if token.ac != nil { | |||
| 596 | + | go d.paintRemoteView(view, attachment, false, token) | |||
| 597 | + | } | |||
| 598 | + | } | |||
| 599 | + | } | |||
| 600 | + | ||||
| 560 | // send serializes the complete outbound remote stream. It captures the exact | 601 | // send serializes the complete outbound remote stream. It captures the exact | ||
| 561 | // current transport under view.mu, releases every architecture lock, then lets | 602 | // current transport under view.mu, releases every architecture lock, then lets | ||
| 562 | // Close interrupt a blocked Send concurrently. | 603 | // Close interrupt a blocked Send concurrently. | ||
M internal/usecase/daemon/remote_link_test.go +177 -0
| @@ -10,6 +10,7 @@ import ( | |||
| 10 | 10 | ||
| 11 | 11 | "github.com/bnema/vev/internal/domain" | |
| 12 | 12 | "github.com/bnema/vev/internal/ports" | |
| 13 | + | "github.com/bnema/vev/pkg/vt" | |
| 13 | 14 | "github.com/stretchr/testify/require" | |
| 14 | 15 | ) | |
| 15 | 16 | ||
| @@ -120,6 +121,182 @@ func enqueueRemoteLinkHandshake(t *testing.T, transport *remoteLinkTestTransport | |||
| 120 | 121 | transport.recv <- remoteLinkTestReceive{frame: ports.Frame{Type: ports.MsgOutput, Payload: outputPayload}} | |
| 121 | 122 | } | |
| 122 | 123 | ||
| 124 | + | func newRemoteMetadataLinkFixture(t *testing.T) (*Daemon, *remoteView, *remoteLink, *remoteLinkTestTransport) { | |
| 125 | + | t.Helper() | |
| 126 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 127 | + | transport := newRemoteLinkTestTransport() | |
| 128 | + | target := remoteLinkTestTarget() | |
| 129 | + | view := &remoteView{ | |
| 130 | + | key: remoteViewKey{ | |
| 131 | + | endpoint: target.Endpoint, | |
| 132 | + | lifecycleID: target.LifecycleID, | |
| 133 | + | sessionName: target.SessionName, | |
| 134 | + | }, | |
| 135 | + | screen: vt.NewScreen(80, 22), | |
| 136 | + | linkGeneration: 1, | |
| 137 | + | metadata: ports.SessionMeta{ | |
| 138 | + | LifecycleID: target.LifecycleID, | |
| 139 | + | Revision: 1, | |
| 140 | + | SessionName: target.SessionName, | |
| 141 | + | ActiveTabID: target.LiveTabID, | |
| 142 | + | Tabs: []ports.SessionTabMeta{{ID: target.LiveTabID, Name: "main"}}, | |
| 143 | + | }, | |
| 144 | + | } | |
| 145 | + | link := &remoteLink{ | |
| 146 | + | view: view, | |
| 147 | + | generation: 1, | |
| 148 | + | target: target, | |
| 149 | + | transport: transport, | |
| 150 | + | cancel: func() {}, | |
| 151 | + | active: true, | |
| 152 | + | } | |
| 153 | + | view.link = link | |
| 154 | + | d.mu.Lock() | |
| 155 | + | require.NoError(t, d.registerRemoteViewLocked(view)) | |
| 156 | + | d.mu.Unlock() | |
| 157 | + | return d, view, link, transport | |
| 158 | + | } | |
| 159 | + | ||
| 160 | + | func attachRemoteMetadataClient(t *testing.T, view *remoteView) (*attachedClient, chan ports.Frame) { | |
| 161 | + | t.Helper() | |
| 162 | + | transport, sends := newCapturingTransport(t) | |
| 163 | + | ac := &attachedClient{tr: transport, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |
| 164 | + | ac.initOverlays() | |
| 165 | + | ac.setAttachmentOwner(view) | |
| 166 | + | view.mu.Lock() | |
| 167 | + | require.True(t, view.registerAttachmentLocked(ac)) | |
| 168 | + | view.mu.Unlock() | |
| 169 | + | return ac, sends | |
| 170 | + | } | |
| 171 | + | ||
| 172 | + | func remoteMetadataFrame(t *testing.T, metadata ports.SessionMeta) ports.Frame { | |
| 173 | + | t.Helper() | |
| 174 | + | payload, err := ports.MarshalSessionMeta(metadata) | |
| 175 | + | require.NoError(t, err) | |
| 176 | + | return ports.Frame{Type: ports.MsgSessionMeta, Payload: payload} | |
| 177 | + | } | |
| 178 | + | ||
| 179 | + | func TestRemoteLinkAcceptedMetadataRepaintsEveryAttachedClientUnderLocalChrome(t *testing.T) { | |
| 180 | + | d, view, link, _ := newRemoteMetadataLinkFixture(t) | |
| 181 | + | first, firstSends := attachRemoteMetadataClient(t, view) | |
| 182 | + | second, secondSends := attachRemoteMetadataClient(t, view) | |
| 183 | + | callbacks := make(chan struct{}, 2) | |
| 184 | + | for _, ac := range []*attachedClient{first, second} { | |
| 185 | + | ac.beforeAttachmentTokenValidation = func() { | |
| 186 | + | view.mu.Lock() | |
| 187 | + | view.mu.Unlock() | |
| 188 | + | callbacks <- struct{}{} | |
| 189 | + | } | |
| 190 | + | } | |
| 191 | + | ||
| 192 | + | metadata := ports.SessionMeta{ | |
| 193 | + | LifecycleID: view.key.lifecycleID, | |
| 194 | + | Revision: 2, | |
| 195 | + | SessionName: view.key.sessionName, | |
| 196 | + | ActiveTabID: "tab-1", | |
| 197 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "renamed"}}, | |
| 198 | + | } | |
| 199 | + | require.NoError(t, d.handleRemoteLinkFrame(link, remoteMetadataFrame(t, metadata))) | |
| 200 | + | for range 2 { | |
| 201 | + | awaitTestValue(t, callbacks, "metadata repaint callback did not run outside remoteView.mu") | |
| 202 | + | } | |
| 203 | + | ||
| 204 | + | for _, sends := range []chan ports.Frame{firstSends, secondSends} { | |
| 205 | + | frame := awaitFrame(t, sends, ports.MsgOutput) | |
| 206 | + | output, err := ports.UnmarshalOutput(frame.Payload) | |
| 207 | + | require.NoError(t, err) | |
| 208 | + | require.Contains(t, string(output.Data), "renamed", "accepted metadata must repaint local chrome") | |
| 209 | + | } | |
| 210 | + | view.mu.Lock() | |
| 211 | + | require.Equal(t, uint64(2), view.metadata.Revision) | |
| 212 | + | require.Equal(t, "renamed", view.metadata.Tabs[0].Name) | |
| 213 | + | view.mu.Unlock() | |
| 214 | + | } | |
| 215 | + | ||
| 216 | + | func TestRemoteLinkIgnoresDuplicateAndOlderMetadataWithoutStopping(t *testing.T) { | |
| 217 | + | d, view, link, transport := newRemoteMetadataLinkFixture(t) | |
| 218 | + | metadata := ports.SessionMeta{ | |
| 219 | + | LifecycleID: view.key.lifecycleID, | |
| 220 | + | Revision: 2, | |
| 221 | + | SessionName: view.key.sessionName, | |
| 222 | + | ActiveTabID: "tab-1", | |
| 223 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "new"}}, | |
| 224 | + | } | |
| 225 | + | frame := remoteMetadataFrame(t, metadata) | |
| 226 | + | require.NoError(t, d.handleRemoteLinkFrame(link, frame)) | |
| 227 | + | require.NoError(t, d.handleRemoteLinkFrame(link, frame), "duplicate metadata must be ignored") | |
| 228 | + | require.NoError(t, d.handleRemoteLinkFrame(link, remoteMetadataFrame(t, ports.SessionMeta{ | |
| 229 | + | LifecycleID: view.key.lifecycleID, | |
| 230 | + | Revision: 1, | |
| 231 | + | SessionName: view.key.sessionName, | |
| 232 | + | ActiveTabID: "tab-1", | |
| 233 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "old"}}, | |
| 234 | + | })), "older metadata must be ignored") | |
| 235 | + | ||
| 236 | + | view.mu.Lock() | |
| 237 | + | require.Same(t, link, view.link) | |
| 238 | + | require.True(t, link.active) | |
| 239 | + | require.Equal(t, uint64(2), view.metadata.Revision) | |
| 240 | + | require.Equal(t, "new", view.metadata.Tabs[0].Name) | |
| 241 | + | view.mu.Unlock() | |
| 242 | + | ||
| 243 | + | require.NoError(t, d.handleRemoteLinkFrame(link, ports.Frame{Type: ports.MsgPing, Payload: ports.MarshalPing(ports.Ping{})})) | |
| 244 | + | awaitFrame(t, transport.sent, ports.MsgPong) | |
| 245 | + | } | |
| 246 | + | ||
| 247 | + | func TestRemoteLinkRejectsInvalidMetadataIdentityAndOrdering(t *testing.T) { | |
| 248 | + | tests := []struct { | |
| 249 | + | name string | |
| 250 | + | mutate func(*testing.T, []byte) | |
| 251 | + | }{ | |
| 252 | + | { | |
| 253 | + | name: "lifecycle identity", | |
| 254 | + | mutate: func(_ *testing.T, payload []byte) { | |
| 255 | + | payload[0] = 2 | |
| 256 | + | }, | |
| 257 | + | }, | |
| 258 | + | { | |
| 259 | + | name: "session identity", | |
| 260 | + | mutate: func(t *testing.T, payload []byte) { | |
| 261 | + | metadata := ports.SessionMeta{ | |
| 262 | + | LifecycleID: domain.SessionLifecycleID{1}, Revision: 2, SessionName: "remote!", ActiveTabID: "tab-1", | |
| 263 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "new"}}, | |
| 264 | + | } | |
| 265 | + | encoded, err := ports.MarshalSessionMeta(metadata) | |
| 266 | + | require.NoError(t, err) | |
| 267 | + | copy(payload, encoded) | |
| 268 | + | }, | |
| 269 | + | }, | |
| 270 | + | { | |
| 271 | + | name: "zero revision", | |
| 272 | + | mutate: func(_ *testing.T, payload []byte) { | |
| 273 | + | clear(payload[16:24]) | |
| 274 | + | }, | |
| 275 | + | }, | |
| 276 | + | } | |
| 277 | + | for _, test := range tests { | |
| 278 | + | t.Run(test.name, func(t *testing.T) { | |
| 279 | + | d, view, link, _ := newRemoteMetadataLinkFixture(t) | |
| 280 | + | metadata := ports.SessionMeta{ | |
| 281 | + | LifecycleID: view.key.lifecycleID, | |
| 282 | + | Revision: 2, | |
| 283 | + | SessionName: view.key.sessionName, | |
| 284 | + | ActiveTabID: "tab-1", | |
| 285 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "new"}}, | |
| 286 | + | } | |
| 287 | + | payload, err := ports.MarshalSessionMeta(metadata) | |
| 288 | + | require.NoError(t, err) | |
| 289 | + | test.mutate(t, payload) | |
| 290 | + | err = d.handleRemoteLinkFrame(link, ports.Frame{Type: ports.MsgSessionMeta, Payload: payload}) | |
| 291 | + | require.Error(t, err) | |
| 292 | + | view.mu.Lock() | |
| 293 | + | require.Same(t, link, view.link) | |
| 294 | + | require.Equal(t, uint64(1), view.metadata.Revision) | |
| 295 | + | view.mu.Unlock() | |
| 296 | + | }) | |
| 297 | + | } | |
| 298 | + | } | |
| 299 | + | ||
| 123 | 300 | func TestOpenRemoteViewPublishesHandshakeReadyCandidate(t *testing.T) { | |
| 124 | 301 | d := newTestDaemon(t, nil, stubClock{}) | |
| 125 | 302 | target := remoteLinkTestTarget() | |
| @@ -10,6 +10,7 @@ import ( | |||||
| 10 | 10 | ||||
| 11 | "github.com/bnema/vev/internal/domain" | 11 | "github.com/bnema/vev/internal/domain" | ||
| 12 | "github.com/bnema/vev/internal/ports" | 12 | "github.com/bnema/vev/internal/ports" | ||
| 13 | + | "github.com/bnema/vev/pkg/vt" | |||
| 13 | "github.com/stretchr/testify/require" | 14 | "github.com/stretchr/testify/require" | ||
| 14 | ) | 15 | ) | ||
| 15 | 16 | ||||
| @@ -120,6 +121,182 @@ func enqueueRemoteLinkHandshake(t *testing.T, transport *remoteLinkTestTransport | |||||
| 120 | transport.recv <- remoteLinkTestReceive{frame: ports.Frame{Type: ports.MsgOutput, Payload: outputPayload}} | 121 | transport.recv <- remoteLinkTestReceive{frame: ports.Frame{Type: ports.MsgOutput, Payload: outputPayload}} | ||
| 121 | } | 122 | } | ||
| 122 | 123 | ||||
| 124 | + | func newRemoteMetadataLinkFixture(t *testing.T) (*Daemon, *remoteView, *remoteLink, *remoteLinkTestTransport) { | |||
| 125 | + | t.Helper() | |||
| 126 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 127 | + | transport := newRemoteLinkTestTransport() | |||
| 128 | + | target := remoteLinkTestTarget() | |||
| 129 | + | view := &remoteView{ | |||
| 130 | + | key: remoteViewKey{ | |||
| 131 | + | endpoint: target.Endpoint, | |||
| 132 | + | lifecycleID: target.LifecycleID, | |||
| 133 | + | sessionName: target.SessionName, | |||
| 134 | + | }, | |||
| 135 | + | screen: vt.NewScreen(80, 22), | |||
| 136 | + | linkGeneration: 1, | |||
| 137 | + | metadata: ports.SessionMeta{ | |||
| 138 | + | LifecycleID: target.LifecycleID, | |||
| 139 | + | Revision: 1, | |||
| 140 | + | SessionName: target.SessionName, | |||
| 141 | + | ActiveTabID: target.LiveTabID, | |||
| 142 | + | Tabs: []ports.SessionTabMeta{{ID: target.LiveTabID, Name: "main"}}, | |||
| 143 | + | }, | |||
| 144 | + | } | |||
| 145 | + | link := &remoteLink{ | |||
| 146 | + | view: view, | |||
| 147 | + | generation: 1, | |||
| 148 | + | target: target, | |||
| 149 | + | transport: transport, | |||
| 150 | + | cancel: func() {}, | |||
| 151 | + | active: true, | |||
| 152 | + | } | |||
| 153 | + | view.link = link | |||
| 154 | + | d.mu.Lock() | |||
| 155 | + | require.NoError(t, d.registerRemoteViewLocked(view)) | |||
| 156 | + | d.mu.Unlock() | |||
| 157 | + | return d, view, link, transport | |||
| 158 | + | } | |||
| 159 | + | ||||
| 160 | + | func attachRemoteMetadataClient(t *testing.T, view *remoteView) (*attachedClient, chan ports.Frame) { | |||
| 161 | + | t.Helper() | |||
| 162 | + | transport, sends := newCapturingTransport(t) | |||
| 163 | + | ac := &attachedClient{tr: transport, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |||
| 164 | + | ac.initOverlays() | |||
| 165 | + | ac.setAttachmentOwner(view) | |||
| 166 | + | view.mu.Lock() | |||
| 167 | + | require.True(t, view.registerAttachmentLocked(ac)) | |||
| 168 | + | view.mu.Unlock() | |||
| 169 | + | return ac, sends | |||
| 170 | + | } | |||
| 171 | + | ||||
| 172 | + | func remoteMetadataFrame(t *testing.T, metadata ports.SessionMeta) ports.Frame { | |||
| 173 | + | t.Helper() | |||
| 174 | + | payload, err := ports.MarshalSessionMeta(metadata) | |||
| 175 | + | require.NoError(t, err) | |||
| 176 | + | return ports.Frame{Type: ports.MsgSessionMeta, Payload: payload} | |||
| 177 | + | } | |||
| 178 | + | ||||
| 179 | + | func TestRemoteLinkAcceptedMetadataRepaintsEveryAttachedClientUnderLocalChrome(t *testing.T) { | |||
| 180 | + | d, view, link, _ := newRemoteMetadataLinkFixture(t) | |||
| 181 | + | first, firstSends := attachRemoteMetadataClient(t, view) | |||
| 182 | + | second, secondSends := attachRemoteMetadataClient(t, view) | |||
| 183 | + | callbacks := make(chan struct{}, 2) | |||
| 184 | + | for _, ac := range []*attachedClient{first, second} { | |||
| 185 | + | ac.beforeAttachmentTokenValidation = func() { | |||
| 186 | + | view.mu.Lock() | |||
| 187 | + | view.mu.Unlock() | |||
| 188 | + | callbacks <- struct{}{} | |||
| 189 | + | } | |||
| 190 | + | } | |||
| 191 | + | ||||
| 192 | + | metadata := ports.SessionMeta{ | |||
| 193 | + | LifecycleID: view.key.lifecycleID, | |||
| 194 | + | Revision: 2, | |||
| 195 | + | SessionName: view.key.sessionName, | |||
| 196 | + | ActiveTabID: "tab-1", | |||
| 197 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "renamed"}}, | |||
| 198 | + | } | |||
| 199 | + | require.NoError(t, d.handleRemoteLinkFrame(link, remoteMetadataFrame(t, metadata))) | |||
| 200 | + | for range 2 { | |||
| 201 | + | awaitTestValue(t, callbacks, "metadata repaint callback did not run outside remoteView.mu") | |||
| 202 | + | } | |||
| 203 | + | ||||
| 204 | + | for _, sends := range []chan ports.Frame{firstSends, secondSends} { | |||
| 205 | + | frame := awaitFrame(t, sends, ports.MsgOutput) | |||
| 206 | + | output, err := ports.UnmarshalOutput(frame.Payload) | |||
| 207 | + | require.NoError(t, err) | |||
| 208 | + | require.Contains(t, string(output.Data), "renamed", "accepted metadata must repaint local chrome") | |||
| 209 | + | } | |||
| 210 | + | view.mu.Lock() | |||
| 211 | + | require.Equal(t, uint64(2), view.metadata.Revision) | |||
| 212 | + | require.Equal(t, "renamed", view.metadata.Tabs[0].Name) | |||
| 213 | + | view.mu.Unlock() | |||
| 214 | + | } | |||
| 215 | + | ||||
| 216 | + | func TestRemoteLinkIgnoresDuplicateAndOlderMetadataWithoutStopping(t *testing.T) { | |||
| 217 | + | d, view, link, transport := newRemoteMetadataLinkFixture(t) | |||
| 218 | + | metadata := ports.SessionMeta{ | |||
| 219 | + | LifecycleID: view.key.lifecycleID, | |||
| 220 | + | Revision: 2, | |||
| 221 | + | SessionName: view.key.sessionName, | |||
| 222 | + | ActiveTabID: "tab-1", | |||
| 223 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "new"}}, | |||
| 224 | + | } | |||
| 225 | + | frame := remoteMetadataFrame(t, metadata) | |||
| 226 | + | require.NoError(t, d.handleRemoteLinkFrame(link, frame)) | |||
| 227 | + | require.NoError(t, d.handleRemoteLinkFrame(link, frame), "duplicate metadata must be ignored") | |||
| 228 | + | require.NoError(t, d.handleRemoteLinkFrame(link, remoteMetadataFrame(t, ports.SessionMeta{ | |||
| 229 | + | LifecycleID: view.key.lifecycleID, | |||
| 230 | + | Revision: 1, | |||
| 231 | + | SessionName: view.key.sessionName, | |||
| 232 | + | ActiveTabID: "tab-1", | |||
| 233 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "old"}}, | |||
| 234 | + | })), "older metadata must be ignored") | |||
| 235 | + | ||||
| 236 | + | view.mu.Lock() | |||
| 237 | + | require.Same(t, link, view.link) | |||
| 238 | + | require.True(t, link.active) | |||
| 239 | + | require.Equal(t, uint64(2), view.metadata.Revision) | |||
| 240 | + | require.Equal(t, "new", view.metadata.Tabs[0].Name) | |||
| 241 | + | view.mu.Unlock() | |||
| 242 | + | ||||
| 243 | + | require.NoError(t, d.handleRemoteLinkFrame(link, ports.Frame{Type: ports.MsgPing, Payload: ports.MarshalPing(ports.Ping{})})) | |||
| 244 | + | awaitFrame(t, transport.sent, ports.MsgPong) | |||
| 245 | + | } | |||
| 246 | + | ||||
| 247 | + | func TestRemoteLinkRejectsInvalidMetadataIdentityAndOrdering(t *testing.T) { | |||
| 248 | + | tests := []struct { | |||
| 249 | + | name string | |||
| 250 | + | mutate func(*testing.T, []byte) | |||
| 251 | + | }{ | |||
| 252 | + | { | |||
| 253 | + | name: "lifecycle identity", | |||
| 254 | + | mutate: func(_ *testing.T, payload []byte) { | |||
| 255 | + | payload[0] = 2 | |||
| 256 | + | }, | |||
| 257 | + | }, | |||
| 258 | + | { | |||
| 259 | + | name: "session identity", | |||
| 260 | + | mutate: func(t *testing.T, payload []byte) { | |||
| 261 | + | metadata := ports.SessionMeta{ | |||
| 262 | + | LifecycleID: domain.SessionLifecycleID{1}, Revision: 2, SessionName: "remote!", ActiveTabID: "tab-1", | |||
| 263 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "new"}}, | |||
| 264 | + | } | |||
| 265 | + | encoded, err := ports.MarshalSessionMeta(metadata) | |||
| 266 | + | require.NoError(t, err) | |||
| 267 | + | copy(payload, encoded) | |||
| 268 | + | }, | |||
| 269 | + | }, | |||
| 270 | + | { | |||
| 271 | + | name: "zero revision", | |||
| 272 | + | mutate: func(_ *testing.T, payload []byte) { | |||
| 273 | + | clear(payload[16:24]) | |||
| 274 | + | }, | |||
| 275 | + | }, | |||
| 276 | + | } | |||
| 277 | + | for _, test := range tests { | |||
| 278 | + | t.Run(test.name, func(t *testing.T) { | |||
| 279 | + | d, view, link, _ := newRemoteMetadataLinkFixture(t) | |||
| 280 | + | metadata := ports.SessionMeta{ | |||
| 281 | + | LifecycleID: view.key.lifecycleID, | |||
| 282 | + | Revision: 2, | |||
| 283 | + | SessionName: view.key.sessionName, | |||
| 284 | + | ActiveTabID: "tab-1", | |||
| 285 | + | Tabs: []ports.SessionTabMeta{{ID: "tab-1", Name: "new"}}, | |||
| 286 | + | } | |||
| 287 | + | payload, err := ports.MarshalSessionMeta(metadata) | |||
| 288 | + | require.NoError(t, err) | |||
| 289 | + | test.mutate(t, payload) | |||
| 290 | + | err = d.handleRemoteLinkFrame(link, ports.Frame{Type: ports.MsgSessionMeta, Payload: payload}) | |||
| 291 | + | require.Error(t, err) | |||
| 292 | + | view.mu.Lock() | |||
| 293 | + | require.Same(t, link, view.link) | |||
| 294 | + | require.Equal(t, uint64(1), view.metadata.Revision) | |||
| 295 | + | view.mu.Unlock() | |||
| 296 | + | }) | |||
| 297 | + | } | |||
| 298 | + | } | |||
| 299 | + | ||||
| 123 | func TestOpenRemoteViewPublishesHandshakeReadyCandidate(t *testing.T) { | 300 | func TestOpenRemoteViewPublishesHandshakeReadyCandidate(t *testing.T) { | ||
| 124 | d := newTestDaemon(t, nil, stubClock{}) | 301 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 125 | target := remoteLinkTestTarget() | 302 | target := remoteLinkTestTarget() | ||