fix(remote): harden remote view lifecycle
- commit
- 1626715d95e3b94e39a76ca403b2da5835fceb13
- author
- brice · · 2026-08-07 10:45 UTC
- parent
- de8bb89
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
12 files · +113 -54
M internal/usecase/client/client.go +1 -0
| @@ -471,6 +471,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 471 | 471 | dialer = nextDialer | |
| 472 | 472 | attemptRequest = nextRequest | |
| 473 | 473 | remote = attemptRequest.Remote || r.remote | |
| 474 | + | reconnect.remote = remote | |
| 474 | 475 | resumeToken = 0 | |
| 475 | 476 | backoff = defaultReconnectBackoff.initial | |
| 476 | 477 | continue | |
| @@ -471,6 +471,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 471 | dialer = nextDialer | 471 | dialer = nextDialer | ||
| 472 | attemptRequest = nextRequest | 472 | attemptRequest = nextRequest | ||
| 473 | remote = attemptRequest.Remote || r.remote | 473 | remote = attemptRequest.Remote || r.remote | ||
| 474 | + | reconnect.remote = remote | |||
| 474 | resumeToken = 0 | 475 | resumeToken = 0 | ||
| 475 | backoff = defaultReconnectBackoff.initial | 476 | backoff = defaultReconnectBackoff.initial | ||
| 476 | continue | 477 | continue | ||
M internal/usecase/daemon/coderabbit_regression_test.go +14 -0
| @@ -243,6 +243,20 @@ func TestCreateSessionRechecksShutdownAfterCatalogueRead(t *testing.T) { | |||
| 243 | 243 | require.Empty(t, d.sessions) | |
| 244 | 244 | } | |
| 245 | 245 | ||
| 246 | + | func TestShutdownDeadlineDoesNotWaitForUncooperativeRemoteConstruction(t *testing.T) { | |
| 247 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 248 | + | construction := &remoteViewConstruction{done: make(chan struct{})} | |
| 249 | + | d.mu.Lock() | |
| 250 | + | d.remoteViewConstructions[remoteViewKey{}] = construction | |
| 251 | + | d.mu.Unlock() | |
| 252 | + | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | |
| 253 | + | close(deadline.done) | |
| 254 | + | ||
| 255 | + | result := make(chan bool, 1) | |
| 256 | + | go func() { result <- d.shutdownAllWithSnapshotDeadline(ports.ReasonServerShutdown, deadline) }() | |
| 257 | + | require.True(t, awaitTestValue(t, result, "shutdown blocked on canceled remote construction")) | |
| 258 | + | } | |
| 259 | + | ||
| 246 | 260 | func TestSnapshotStopContextCancelIsIdempotent(t *testing.T) { | |
| 247 | 261 | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | |
| 248 | 262 | _, cancel := snapshotStopContext(deadline) | |
| @@ -243,6 +243,20 @@ func TestCreateSessionRechecksShutdownAfterCatalogueRead(t *testing.T) { | |||||
| 243 | require.Empty(t, d.sessions) | 243 | require.Empty(t, d.sessions) | ||
| 244 | } | 244 | } | ||
| 245 | 245 | ||||
| 246 | + | func TestShutdownDeadlineDoesNotWaitForUncooperativeRemoteConstruction(t *testing.T) { | |||
| 247 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 248 | + | construction := &remoteViewConstruction{done: make(chan struct{})} | |||
| 249 | + | d.mu.Lock() | |||
| 250 | + | d.remoteViewConstructions[remoteViewKey{}] = construction | |||
| 251 | + | d.mu.Unlock() | |||
| 252 | + | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | |||
| 253 | + | close(deadline.done) | |||
| 254 | + | ||||
| 255 | + | result := make(chan bool, 1) | |||
| 256 | + | go func() { result <- d.shutdownAllWithSnapshotDeadline(ports.ReasonServerShutdown, deadline) }() | |||
| 257 | + | require.True(t, awaitTestValue(t, result, "shutdown blocked on canceled remote construction")) | |||
| 258 | + | } | |||
| 259 | + | ||||
| 246 | func TestSnapshotStopContextCancelIsIdempotent(t *testing.T) { | 260 | func TestSnapshotStopContextCancelIsIdempotent(t *testing.T) { | ||
| 247 | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | 261 | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | ||
| 248 | _, cancel := snapshotStopContext(deadline) | 262 | _, cancel := snapshotStopContext(deadline) | ||
M internal/usecase/daemon/daemon.go +13 -4
| @@ -833,11 +833,20 @@ func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapsho | |||
| 833 | 833 | } | |
| 834 | 834 | } | |
| 835 | 835 | // Candidate construction owns dial and handshake I/O outside daemon locks. | |
| 836 | - | // Shutdown waits for each canceled constructor to close its completion gate | |
| 837 | - | // so no unpublished transport or callback can outlive daemon teardown. | |
| 836 | + | // A shutdown deadline bounds a non-cooperative dialer; a late constructor | |
| 837 | + | // observes d.closing and retires its unpublished candidate independently. | |
| 838 | 838 | for _, construction := range constructions { | |
| 839 | - | if construction != nil && construction.done != nil { | |
| 839 | + | if construction == nil || construction.done == nil { | |
| 840 | + | continue | |
| 841 | + | } | |
| 842 | + | if deadline == nil { | |
| 840 | 843 | <-construction.done | |
| 844 | + | continue | |
| 845 | + | } | |
| 846 | + | select { | |
| 847 | + | case <-construction.done: | |
| 848 | + | case <-deadline.Done(): | |
| 849 | + | checkpointIncomplete = true | |
| 841 | 850 | } | |
| 842 | 851 | } | |
| 843 | 852 | remoteWarms := make([]*remoteViewWarm, 0, len(remoteViews)) | |
| @@ -863,7 +872,7 @@ func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapsho | |||
| 863 | 872 | d.log.Info("graceful shutdown begin", "reason", reason, "sessions", len(snapshot)) | |
| 864 | 873 | if empty { | |
| 865 | 874 | d.doneOnce.Do(func() { close(d.done) }) | |
| 866 | - | return false | |
| 875 | + | return checkpointIncomplete | |
| 867 | 876 | } | |
| 868 | 877 | for _, s := range snapshot { | |
| 869 | 878 | // Cancellation and PTY closure must not wait behind a teardown owner that | |
| @@ -833,11 +833,20 @@ func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapsho | |||||
| 833 | } | 833 | } | ||
| 834 | } | 834 | } | ||
| 835 | // Candidate construction owns dial and handshake I/O outside daemon locks. | 835 | // Candidate construction owns dial and handshake I/O outside daemon locks. | ||
| 836 | - | // Shutdown waits for each canceled constructor to close its completion gate | 836 | + | // A shutdown deadline bounds a non-cooperative dialer; a late constructor |
| 837 | - | // so no unpublished transport or callback can outlive daemon teardown. | 837 | + | // observes d.closing and retires its unpublished candidate independently. |
| 838 | for _, construction := range constructions { | 838 | for _, construction := range constructions { | ||
| 839 | - | if construction != nil && construction.done != nil { | 839 | + | if construction == nil || construction.done == nil { |
| 840 | + | continue | |||
| 841 | + | } | |||
| 842 | + | if deadline == nil { | |||
| 840 | <-construction.done | 843 | <-construction.done | ||
| 844 | + | continue | |||
| 845 | + | } | |||
| 846 | + | select { | |||
| 847 | + | case <-construction.done: | |||
| 848 | + | case <-deadline.Done(): | |||
| 849 | + | checkpointIncomplete = true | |||
| 841 | } | 850 | } | ||
| 842 | } | 851 | } | ||
| 843 | remoteWarms := make([]*remoteViewWarm, 0, len(remoteViews)) | 852 | remoteWarms := make([]*remoteViewWarm, 0, len(remoteViews)) | ||
| @@ -863,7 +872,7 @@ func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapsho | |||||
| 863 | d.log.Info("graceful shutdown begin", "reason", reason, "sessions", len(snapshot)) | 872 | d.log.Info("graceful shutdown begin", "reason", reason, "sessions", len(snapshot)) | ||
| 864 | if empty { | 873 | if empty { | ||
| 865 | d.doneOnce.Do(func() { close(d.done) }) | 874 | d.doneOnce.Do(func() { close(d.done) }) | ||
| 866 | - | return false | 875 | + | return checkpointIncomplete |
| 867 | } | 876 | } | ||
| 868 | for _, s := range snapshot { | 877 | for _, s := range snapshot { | ||
| 869 | // Cancellation and PTY closure must not wait behind a teardown owner that | 878 | // Cancellation and PTY closure must not wait behind a teardown owner that | ||
M internal/usecase/daemon/picker.go +20 -10
| @@ -349,12 +349,12 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||
| 349 | 349 | if result.action == 's' { | |
| 350 | 350 | d.togglePickerSort() | |
| 351 | 351 | d.refreshPickerOpts(ac, pickerRefreshOptions{preserveSelection: true, nearestRow: -1}) | |
| 352 | - | d.invalidateRender(sess, ac, true, "picker.go") | |
| 352 | + | d.invalidateAttachedOwner(ac, true, "picker.go") | |
| 353 | 353 | return | |
| 354 | 354 | } | |
| 355 | 355 | if (result.action == '\r' || result.action == '\n') && !ok && cursorOK && (cursor.RemoteTarget != nil || cursor.RemoteHost != "") { | |
| 356 | 356 | d.notifyRemotePickerUnavailable(sess, cursor) | |
| 357 | - | d.invalidateRender(sess, ac, true, "picker.go") | |
| 357 | + | d.invalidateAttachedOwner(ac, true, "picker.go") | |
| 358 | 358 | return | |
| 359 | 359 | } | |
| 360 | 360 | if result.action == 'x' { | |
| @@ -382,7 +382,7 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||
| 382 | 382 | defer fresh.End() | |
| 383 | 383 | } | |
| 384 | 384 | d.refreshPickerOpts(ac, pickerRefreshOptions{nearestRow: prevIdx}) | |
| 385 | - | d.invalidateRender(sess, ac, true, "picker.go") | |
| 385 | + | d.invalidateAttachedOwner(ac, true, "picker.go") | |
| 386 | 386 | return | |
| 387 | 387 | } | |
| 388 | 388 | if result.changed { | |
| @@ -396,7 +396,7 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||
| 396 | 396 | if intent == pickerMovePane || intent == pickerMoveTab { | |
| 397 | 397 | if err := d.movePickerSourceError(source); err != nil { | |
| 398 | 398 | d.reportAttachmentError(sess, err) | |
| 399 | - | d.invalidateRender(sess, ac, true, "picker.go") | |
| 399 | + | d.invalidateAttachedOwner(ac, true, "picker.go") | |
| 400 | 400 | return | |
| 401 | 401 | } | |
| 402 | 402 | if len(effects) != 0 && effects[0] != nil { | |
| @@ -406,10 +406,10 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||
| 406 | 406 | err := d.commitMovePickerSelection(intent, source, target) | |
| 407 | 407 | if err != nil { | |
| 408 | 408 | d.reportAttachmentError(sess, movePickerUserError(err)) | |
| 409 | - | d.invalidateRender(sess, ac, true, "picker.go") | |
| 409 | + | d.invalidateAttachedOwner(ac, true, "picker.go") | |
| 410 | 410 | return | |
| 411 | 411 | } | |
| 412 | - | d.invalidateRender(sess, ac, true, "picker.go") | |
| 412 | + | d.invalidateAttachedOwner(ac, true, "picker.go") | |
| 413 | 413 | return | |
| 414 | 414 | } | |
| 415 | 415 | var err error | |
| @@ -928,13 +928,23 @@ func (d *Daemon) switchToTargetForAttachment(token attachmentConnectionToken, ta | |||
| 928 | 928 | if target.RemoteKey != nil { | |
| 929 | 929 | return errAttachmentTransition | |
| 930 | 930 | } | |
| 931 | - | if !retainedLocalTargetMatches(target, token.ac.previousOwner.Get()) { | |
| 931 | + | previous := token.ac.previousOwner.Get() | |
| 932 | + | if !retainedLocalTargetMatches(target, previous) { | |
| 932 | 933 | return errAttachmentTransition | |
| 933 | 934 | } | |
| 934 | - | if guard.closePicker { | |
| 935 | - | d.closePicker(token.ac) | |
| 935 | + | transitioned := false | |
| 936 | + | if err := d.backSessionForAttachment(token, func() { | |
| 937 | + | transitioned = true | |
| 938 | + | if guard.closePicker { | |
| 939 | + | d.closePicker(token.ac) | |
| 940 | + | } | |
| 941 | + | }); err != nil { | |
| 942 | + | return err | |
| 936 | 943 | } | |
| 937 | - | return d.backSessionForAttachment(token) | |
| 944 | + | if !transitioned { | |
| 945 | + | return errAttachmentTransition | |
| 946 | + | } | |
| 947 | + | return nil | |
| 938 | 948 | } | |
| 939 | 949 | if token.localSession() == nil || token.effect == nil { | |
| 940 | 950 | return nil | |
| @@ -349,12 +349,12 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||||
| 349 | if result.action == 's' { | 349 | if result.action == 's' { | ||
| 350 | d.togglePickerSort() | 350 | d.togglePickerSort() | ||
| 351 | d.refreshPickerOpts(ac, pickerRefreshOptions{preserveSelection: true, nearestRow: -1}) | 351 | d.refreshPickerOpts(ac, pickerRefreshOptions{preserveSelection: true, nearestRow: -1}) | ||
| 352 | - | d.invalidateRender(sess, ac, true, "picker.go") | 352 | + | d.invalidateAttachedOwner(ac, true, "picker.go") |
| 353 | return | 353 | return | ||
| 354 | } | 354 | } | ||
| 355 | if (result.action == '\r' || result.action == '\n') && !ok && cursorOK && (cursor.RemoteTarget != nil || cursor.RemoteHost != "") { | 355 | if (result.action == '\r' || result.action == '\n') && !ok && cursorOK && (cursor.RemoteTarget != nil || cursor.RemoteHost != "") { | ||
| 356 | d.notifyRemotePickerUnavailable(sess, cursor) | 356 | d.notifyRemotePickerUnavailable(sess, cursor) | ||
| 357 | - | d.invalidateRender(sess, ac, true, "picker.go") | 357 | + | d.invalidateAttachedOwner(ac, true, "picker.go") |
| 358 | return | 358 | return | ||
| 359 | } | 359 | } | ||
| 360 | if result.action == 'x' { | 360 | if result.action == 'x' { | ||
| @@ -382,7 +382,7 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||||
| 382 | defer fresh.End() | 382 | defer fresh.End() | ||
| 383 | } | 383 | } | ||
| 384 | d.refreshPickerOpts(ac, pickerRefreshOptions{nearestRow: prevIdx}) | 384 | d.refreshPickerOpts(ac, pickerRefreshOptions{nearestRow: prevIdx}) | ||
| 385 | - | d.invalidateRender(sess, ac, true, "picker.go") | 385 | + | d.invalidateAttachedOwner(ac, true, "picker.go") |
| 386 | return | 386 | return | ||
| 387 | } | 387 | } | ||
| 388 | if result.changed { | 388 | if result.changed { | ||
| @@ -396,7 +396,7 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||||
| 396 | if intent == pickerMovePane || intent == pickerMoveTab { | 396 | if intent == pickerMovePane || intent == pickerMoveTab { | ||
| 397 | if err := d.movePickerSourceError(source); err != nil { | 397 | if err := d.movePickerSourceError(source); err != nil { | ||
| 398 | d.reportAttachmentError(sess, err) | 398 | d.reportAttachmentError(sess, err) | ||
| 399 | - | d.invalidateRender(sess, ac, true, "picker.go") | 399 | + | d.invalidateAttachedOwner(ac, true, "picker.go") |
| 400 | return | 400 | return | ||
| 401 | } | 401 | } | ||
| 402 | if len(effects) != 0 && effects[0] != nil { | 402 | if len(effects) != 0 && effects[0] != nil { | ||
| @@ -406,10 +406,10 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||||
| 406 | err := d.commitMovePickerSelection(intent, source, target) | 406 | err := d.commitMovePickerSelection(intent, source, target) | ||
| 407 | if err != nil { | 407 | if err != nil { | ||
| 408 | d.reportAttachmentError(sess, movePickerUserError(err)) | 408 | d.reportAttachmentError(sess, movePickerUserError(err)) | ||
| 409 | - | d.invalidateRender(sess, ac, true, "picker.go") | 409 | + | d.invalidateAttachedOwner(ac, true, "picker.go") |
| 410 | return | 410 | return | ||
| 411 | } | 411 | } | ||
| 412 | - | d.invalidateRender(sess, ac, true, "picker.go") | 412 | + | d.invalidateAttachedOwner(ac, true, "picker.go") |
| 413 | return | 413 | return | ||
| 414 | } | 414 | } | ||
| 415 | var err error | 415 | var err error | ||
| @@ -928,13 +928,23 @@ func (d *Daemon) switchToTargetForAttachment(token attachmentConnectionToken, ta | |||||
| 928 | if target.RemoteKey != nil { | 928 | if target.RemoteKey != nil { | ||
| 929 | return errAttachmentTransition | 929 | return errAttachmentTransition | ||
| 930 | } | 930 | } | ||
| 931 | - | if !retainedLocalTargetMatches(target, token.ac.previousOwner.Get()) { | 931 | + | previous := token.ac.previousOwner.Get() |
| 932 | + | if !retainedLocalTargetMatches(target, previous) { | |||
| 932 | return errAttachmentTransition | 933 | return errAttachmentTransition | ||
| 933 | } | 934 | } | ||
| 934 | - | if guard.closePicker { | 935 | + | transitioned := false |
| 935 | - | d.closePicker(token.ac) | 936 | + | if err := d.backSessionForAttachment(token, func() { |
| 937 | + | transitioned = true | |||
| 938 | + | if guard.closePicker { | |||
| 939 | + | d.closePicker(token.ac) | |||
| 940 | + | } | |||
| 941 | + | }); err != nil { | |||
| 942 | + | return err | |||
| 936 | } | 943 | } | ||
| 937 | - | return d.backSessionForAttachment(token) | 944 | + | if !transitioned { |
| 945 | + | return errAttachmentTransition | |||
| 946 | + | } | |||
| 947 | + | return nil | |||
| 938 | } | 948 | } | ||
| 939 | if token.localSession() == nil || token.effect == nil { | 949 | if token.localSession() == nil || token.effect == nil { | ||
| 940 | return nil | 950 | return nil | ||
M internal/usecase/daemon/remote_link.go +26 -27
| @@ -18,6 +18,8 @@ var ( | |||
| 18 | 18 | errRemoteViewStale = errors.New("remote view: link is stale") | |
| 19 | 19 | ) | |
| 20 | 20 | ||
| 21 | + | const maxRemoteViewOpenAttempts = 3 | |
| 22 | + | ||
| 21 | 23 | // remoteViewConstruction reserves one remote-view key while its candidate | |
| 22 | 24 | // dials and handshakes. It is guarded by Daemon.mu. cancel is invoked only | |
| 23 | 25 | // after the daemon lock is released, because it may interrupt adapter I/O. | |
| @@ -125,7 +127,7 @@ func (d *Daemon) openRemoteView(ctx context.Context, target domain.RemoteSession | |||
| 125 | 127 | return nil, fmt.Errorf("remote view: key: %w", err) | |
| 126 | 128 | } | |
| 127 | 129 | ||
| 128 | - | for { | |
| 130 | + | for attempts := 0; ; { | |
| 129 | 131 | if err := ctx.Err(); err != nil { | |
| 130 | 132 | return nil, err | |
| 131 | 133 | } | |
| @@ -257,6 +259,10 @@ func (d *Daemon) openRemoteView(ctx context.Context, target domain.RemoteSession | |||
| 257 | 259 | // A winner can be invalidated by its receiver immediately after | |
| 258 | 260 | // publication. Do not spin constructors in a tight loop while the | |
| 259 | 261 | // remote endpoint is unavailable. | |
| 262 | + | attempts++ | |
| 263 | + | if attempts >= maxRemoteViewOpenAttempts { | |
| 264 | + | return nil, errRemoteViewUnavailable | |
| 265 | + | } | |
| 260 | 266 | if !sleepRemoteViewRetry(ctx, d.clock) { | |
| 261 | 267 | return nil, ctx.Err() | |
| 262 | 268 | } | |
| @@ -796,6 +802,23 @@ func (d *Daemon) repaintRemoteViewAttachments(view *remoteView, attachments []*a | |||
| 796 | 802 | } | |
| 797 | 803 | } | |
| 798 | 804 | ||
| 805 | + | // failSend invalidates the exact link after an outbound transport failure. | |
| 806 | + | func (link *remoteLink) failSend(view *remoteView, transport ports.Transport, err error) { | |
| 807 | + | view.mu.Lock() | |
| 808 | + | if view.link == link && view.linkGeneration == link.generation { | |
| 809 | + | link.active = false | |
| 810 | + | signalRemoteViewMetadataChangedLocked(view) | |
| 811 | + | } | |
| 812 | + | view.mu.Unlock() | |
| 813 | + | if link.commands != nil { | |
| 814 | + | link.commands.FailGeneration(link.generation, err) | |
| 815 | + | } | |
| 816 | + | if link.cancel != nil { | |
| 817 | + | link.cancel() | |
| 818 | + | } | |
| 819 | + | _ = transport.Close() | |
| 820 | + | } | |
| 821 | + | ||
| 799 | 822 | // send serializes the complete outbound remote stream. It captures the exact | |
| 800 | 823 | // current transport under view.mu, releases every architecture lock, then lets | |
| 801 | 824 | // Close interrupt a blocked Send concurrently. | |
| @@ -817,19 +840,7 @@ func (link *remoteLink) send(frame ports.Frame) error { | |||
| 817 | 840 | err := transport.Send(frame) | |
| 818 | 841 | link.sendMu.Unlock() | |
| 819 | 842 | if err != nil { | |
| 820 | - | view.mu.Lock() | |
| 821 | - | if view.link == link && view.linkGeneration == link.generation { | |
| 822 | - | link.active = false | |
| 823 | - | signalRemoteViewMetadataChangedLocked(view) | |
| 824 | - | } | |
| 825 | - | view.mu.Unlock() | |
| 826 | - | if link.commands != nil { | |
| 827 | - | link.commands.FailGeneration(link.generation, err) | |
| 828 | - | } | |
| 829 | - | if link.cancel != nil { | |
| 830 | - | link.cancel() | |
| 831 | - | } | |
| 832 | - | _ = transport.Close() | |
| 843 | + | link.failSend(view, transport, err) | |
| 833 | 844 | } | |
| 834 | 845 | return err | |
| 835 | 846 | } | |
| @@ -845,19 +856,7 @@ func (link *remoteLink) send(frame ports.Frame) error { | |||
| 845 | 856 | case err := <-result: | |
| 846 | 857 | timer.Stop() | |
| 847 | 858 | if err != nil { | |
| 848 | - | view.mu.Lock() | |
| 849 | - | if view.link == link && view.linkGeneration == link.generation { | |
| 850 | - | link.active = false | |
| 851 | - | signalRemoteViewMetadataChangedLocked(view) | |
| 852 | - | } | |
| 853 | - | view.mu.Unlock() | |
| 854 | - | if link.commands != nil { | |
| 855 | - | link.commands.FailGeneration(link.generation, err) | |
| 856 | - | } | |
| 857 | - | if link.cancel != nil { | |
| 858 | - | link.cancel() | |
| 859 | - | } | |
| 860 | - | _ = transport.Close() | |
| 859 | + | link.failSend(view, transport, err) | |
| 861 | 860 | } | |
| 862 | 861 | return err | |
| 863 | 862 | case <-timer.C(): | |
| @@ -18,6 +18,8 @@ var ( | |||||
| 18 | errRemoteViewStale = errors.New("remote view: link is stale") | 18 | errRemoteViewStale = errors.New("remote view: link is stale") | ||
| 19 | ) | 19 | ) | ||
| 20 | 20 | ||||
| 21 | + | const maxRemoteViewOpenAttempts = 3 | |||
| 22 | + | ||||
| 21 | // remoteViewConstruction reserves one remote-view key while its candidate | 23 | // remoteViewConstruction reserves one remote-view key while its candidate | ||
| 22 | // dials and handshakes. It is guarded by Daemon.mu. cancel is invoked only | 24 | // dials and handshakes. It is guarded by Daemon.mu. cancel is invoked only | ||
| 23 | // after the daemon lock is released, because it may interrupt adapter I/O. | 25 | // after the daemon lock is released, because it may interrupt adapter I/O. | ||
| @@ -125,7 +127,7 @@ func (d *Daemon) openRemoteView(ctx context.Context, target domain.RemoteSession | |||||
| 125 | return nil, fmt.Errorf("remote view: key: %w", err) | 127 | return nil, fmt.Errorf("remote view: key: %w", err) | ||
| 126 | } | 128 | } | ||
| 127 | 129 | ||||
| 128 | - | for { | 130 | + | for attempts := 0; ; { |
| 129 | if err := ctx.Err(); err != nil { | 131 | if err := ctx.Err(); err != nil { | ||
| 130 | return nil, err | 132 | return nil, err | ||
| 131 | } | 133 | } | ||
| @@ -257,6 +259,10 @@ func (d *Daemon) openRemoteView(ctx context.Context, target domain.RemoteSession | |||||
| 257 | // A winner can be invalidated by its receiver immediately after | 259 | // A winner can be invalidated by its receiver immediately after | ||
| 258 | // publication. Do not spin constructors in a tight loop while the | 260 | // publication. Do not spin constructors in a tight loop while the | ||
| 259 | // remote endpoint is unavailable. | 261 | // remote endpoint is unavailable. | ||
| 262 | + | attempts++ | |||
| 263 | + | if attempts >= maxRemoteViewOpenAttempts { | |||
| 264 | + | return nil, errRemoteViewUnavailable | |||
| 265 | + | } | |||
| 260 | if !sleepRemoteViewRetry(ctx, d.clock) { | 266 | if !sleepRemoteViewRetry(ctx, d.clock) { | ||
| 261 | return nil, ctx.Err() | 267 | return nil, ctx.Err() | ||
| 262 | } | 268 | } | ||
| @@ -796,6 +802,23 @@ func (d *Daemon) repaintRemoteViewAttachments(view *remoteView, attachments []*a | |||||
| 796 | } | 802 | } | ||
| 797 | } | 803 | } | ||
| 798 | 804 | ||||
| 805 | + | // failSend invalidates the exact link after an outbound transport failure. | |||
| 806 | + | func (link *remoteLink) failSend(view *remoteView, transport ports.Transport, err error) { | |||
| 807 | + | view.mu.Lock() | |||
| 808 | + | if view.link == link && view.linkGeneration == link.generation { | |||
| 809 | + | link.active = false | |||
| 810 | + | signalRemoteViewMetadataChangedLocked(view) | |||
| 811 | + | } | |||
| 812 | + | view.mu.Unlock() | |||
| 813 | + | if link.commands != nil { | |||
| 814 | + | link.commands.FailGeneration(link.generation, err) | |||
| 815 | + | } | |||
| 816 | + | if link.cancel != nil { | |||
| 817 | + | link.cancel() | |||
| 818 | + | } | |||
| 819 | + | _ = transport.Close() | |||
| 820 | + | } | |||
| 821 | + | ||||
| 799 | // send serializes the complete outbound remote stream. It captures the exact | 822 | // send serializes the complete outbound remote stream. It captures the exact | ||
| 800 | // current transport under view.mu, releases every architecture lock, then lets | 823 | // current transport under view.mu, releases every architecture lock, then lets | ||
| 801 | // Close interrupt a blocked Send concurrently. | 824 | // Close interrupt a blocked Send concurrently. | ||
| @@ -817,19 +840,7 @@ func (link *remoteLink) send(frame ports.Frame) error { | |||||
| 817 | err := transport.Send(frame) | 840 | err := transport.Send(frame) | ||
| 818 | link.sendMu.Unlock() | 841 | link.sendMu.Unlock() | ||
| 819 | if err != nil { | 842 | if err != nil { | ||
| 820 | - | view.mu.Lock() | 843 | + | link.failSend(view, transport, err) |
| 821 | - | if view.link == link && view.linkGeneration == link.generation { | |||
| 822 | - | link.active = false | |||
| 823 | - | signalRemoteViewMetadataChangedLocked(view) | |||
| 824 | - | } | |||
| 825 | - | view.mu.Unlock() | |||
| 826 | - | if link.commands != nil { | |||
| 827 | - | link.commands.FailGeneration(link.generation, err) | |||
| 828 | - | } | |||
| 829 | - | if link.cancel != nil { | |||
| 830 | - | link.cancel() | |||
| 831 | - | } | |||
| 832 | - | _ = transport.Close() | |||
| 833 | } | 844 | } | ||
| 834 | return err | 845 | return err | ||
| 835 | } | 846 | } | ||
| @@ -845,19 +856,7 @@ func (link *remoteLink) send(frame ports.Frame) error { | |||||
| 845 | case err := <-result: | 856 | case err := <-result: | ||
| 846 | timer.Stop() | 857 | timer.Stop() | ||
| 847 | if err != nil { | 858 | if err != nil { | ||
| 848 | - | view.mu.Lock() | 859 | + | link.failSend(view, transport, err) |
| 849 | - | if view.link == link && view.linkGeneration == link.generation { | |||
| 850 | - | link.active = false | |||
| 851 | - | signalRemoteViewMetadataChangedLocked(view) | |||
| 852 | - | } | |||
| 853 | - | view.mu.Unlock() | |||
| 854 | - | if link.commands != nil { | |||
| 855 | - | link.commands.FailGeneration(link.generation, err) | |||
| 856 | - | } | |||
| 857 | - | if link.cancel != nil { | |||
| 858 | - | link.cancel() | |||
| 859 | - | } | |||
| 860 | - | _ = transport.Close() | |||
| 861 | } | 860 | } | ||
| 862 | return err | 861 | return err | ||
| 863 | case <-timer.C(): | 862 | case <-timer.C(): | ||
M internal/usecase/daemon/remote_reconnect.go +1 -1
| @@ -15,7 +15,7 @@ func sleepRemoteViewRetry(ctx context.Context, clock ports.Clock) bool { | |||
| 15 | 15 | ctx = context.Background() | |
| 16 | 16 | } | |
| 17 | 17 | if clock == nil { | |
| 18 | - | return true | |
| 18 | + | clock = systemClock{} | |
| 19 | 19 | } | |
| 20 | 20 | timer := clock.NewTimer(remoteViewRetryDelay) | |
| 21 | 21 | defer timer.Stop() | |
| @@ -15,7 +15,7 @@ func sleepRemoteViewRetry(ctx context.Context, clock ports.Clock) bool { | |||||
| 15 | ctx = context.Background() | 15 | ctx = context.Background() | ||
| 16 | } | 16 | } | ||
| 17 | if clock == nil { | 17 | if clock == nil { | ||
| 18 | - | return true | 18 | + | clock = systemClock{} |
| 19 | } | 19 | } | ||
| 20 | timer := clock.NewTimer(remoteViewRetryDelay) | 20 | timer := clock.NewTimer(remoteViewRetryDelay) | ||
| 21 | defer timer.Stop() | 21 | defer timer.Stop() | ||
M internal/usecase/daemon/remote_render.go +6 -6
| @@ -23,11 +23,11 @@ type remoteViewRenderSnapshot struct { | |||
| 23 | 23 | // resizeScreen updates only the private remote VT surface. The attachment | |
| 24 | 24 | // owns the outer window and callers must not route this through local session | |
| 25 | 25 | // geometry or PTY resize machinery. | |
| 26 | - | func (v *remoteView) resizeScreen(size domain.Size) bool { | |
| 27 | - | if v == nil || !size.Valid() { | |
| 26 | + | func (v *remoteView) resizeScreen(window domain.Size) bool { | |
| 27 | + | if v == nil || !window.Valid() { | |
| 28 | 28 | return false | |
| 29 | 29 | } | |
| 30 | - | content := contentSize(size) | |
| 30 | + | content := contentSize(window) | |
| 31 | 31 | v.mu.Lock() | |
| 32 | 32 | defer v.mu.Unlock() | |
| 33 | 33 | if v.closed { | |
| @@ -44,8 +44,8 @@ func (v *remoteView) resizeScreen(size domain.Size) bool { | |||
| 44 | 44 | // renderSnapshot is the remote-content ownership boundary. It copies every | |
| 45 | 45 | // mutable VT and metadata value while view.mu is held, then releases that lock | |
| 46 | 46 | // before any attachment, overlay, renderer, or transport work begins. | |
| 47 | - | func (v *remoteView) renderSnapshot(size domain.Size) (remoteViewRenderSnapshot, bool) { | |
| 48 | - | if v == nil || !size.Valid() { | |
| 47 | + | func (v *remoteView) renderSnapshot(content domain.Size) (remoteViewRenderSnapshot, bool) { | |
| 48 | + | if v == nil || !content.Valid() { | |
| 49 | 49 | return remoteViewRenderSnapshot{}, false | |
| 50 | 50 | } | |
| 51 | 51 | v.mu.Lock() | |
| @@ -54,7 +54,7 @@ func (v *remoteView) renderSnapshot(size domain.Size) (remoteViewRenderSnapshot, | |||
| 54 | 54 | return remoteViewRenderSnapshot{}, false | |
| 55 | 55 | } | |
| 56 | 56 | if v.screen == nil { | |
| 57 | - | v.screen = vt.NewScreen(size.Cols, size.Rows) | |
| 57 | + | v.screen = vt.NewScreen(content.Cols, content.Rows) | |
| 58 | 58 | } | |
| 59 | 59 | // The shared remote screen follows the latest valid content-size claim via | |
| 60 | 60 | // resizeScreen. Rendering another local attachment must only capture it; | |
| @@ -23,11 +23,11 @@ type remoteViewRenderSnapshot struct { | |||||
| 23 | // resizeScreen updates only the private remote VT surface. The attachment | 23 | // resizeScreen updates only the private remote VT surface. The attachment | ||
| 24 | // owns the outer window and callers must not route this through local session | 24 | // owns the outer window and callers must not route this through local session | ||
| 25 | // geometry or PTY resize machinery. | 25 | // geometry or PTY resize machinery. | ||
| 26 | - | func (v *remoteView) resizeScreen(size domain.Size) bool { | 26 | + | func (v *remoteView) resizeScreen(window domain.Size) bool { |
| 27 | - | if v == nil || !size.Valid() { | 27 | + | if v == nil || !window.Valid() { |
| 28 | return false | 28 | return false | ||
| 29 | } | 29 | } | ||
| 30 | - | content := contentSize(size) | 30 | + | content := contentSize(window) |
| 31 | v.mu.Lock() | 31 | v.mu.Lock() | ||
| 32 | defer v.mu.Unlock() | 32 | defer v.mu.Unlock() | ||
| 33 | if v.closed { | 33 | if v.closed { | ||
| @@ -44,8 +44,8 @@ func (v *remoteView) resizeScreen(size domain.Size) bool { | |||||
| 44 | // renderSnapshot is the remote-content ownership boundary. It copies every | 44 | // renderSnapshot is the remote-content ownership boundary. It copies every | ||
| 45 | // mutable VT and metadata value while view.mu is held, then releases that lock | 45 | // mutable VT and metadata value while view.mu is held, then releases that lock | ||
| 46 | // before any attachment, overlay, renderer, or transport work begins. | 46 | // before any attachment, overlay, renderer, or transport work begins. | ||
| 47 | - | func (v *remoteView) renderSnapshot(size domain.Size) (remoteViewRenderSnapshot, bool) { | 47 | + | func (v *remoteView) renderSnapshot(content domain.Size) (remoteViewRenderSnapshot, bool) { |
| 48 | - | if v == nil || !size.Valid() { | 48 | + | if v == nil || !content.Valid() { |
| 49 | return remoteViewRenderSnapshot{}, false | 49 | return remoteViewRenderSnapshot{}, false | ||
| 50 | } | 50 | } | ||
| 51 | v.mu.Lock() | 51 | v.mu.Lock() | ||
| @@ -54,7 +54,7 @@ func (v *remoteView) renderSnapshot(size domain.Size) (remoteViewRenderSnapshot, | |||||
| 54 | return remoteViewRenderSnapshot{}, false | 54 | return remoteViewRenderSnapshot{}, false | ||
| 55 | } | 55 | } | ||
| 56 | if v.screen == nil { | 56 | if v.screen == nil { | ||
| 57 | - | v.screen = vt.NewScreen(size.Cols, size.Rows) | 57 | + | v.screen = vt.NewScreen(content.Cols, content.Rows) |
| 58 | } | 58 | } | ||
| 59 | // The shared remote screen follows the latest valid content-size claim via | 59 | // The shared remote screen follows the latest valid content-size claim via | ||
| 60 | // resizeScreen. Rendering another local attachment must only capture it; | 60 | // resizeScreen. Rendering another local attachment must only capture it; | ||
M internal/usecase/daemon/session_back.go +4 -1
| @@ -9,7 +9,7 @@ import ( | |||
| 9 | 9 | // the immediately preceding successfully activated local session. A remote | |
| 10 | 10 | // owner is retained in the owner history but is not interpreted as a local | |
| 11 | 11 | // session; remote back navigation is published with remote transitions. | |
| 12 | - | func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error { | |
| 12 | + | func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken, afterRemoteTransition ...func()) error { | |
| 13 | 13 | if d == nil || token.ac == nil { | |
| 14 | 14 | return nil | |
| 15 | 15 | } | |
| @@ -32,6 +32,9 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error | |||
| 32 | 32 | } | |
| 33 | 33 | return err | |
| 34 | 34 | } | |
| 35 | + | if len(afterRemoteTransition) != 0 && afterRemoteTransition[0] != nil { | |
| 36 | + | afterRemoteTransition[0]() | |
| 37 | + | } | |
| 35 | 38 | d.touchMRU(target) | |
| 36 | 39 | d.firstPaintForTransition(published) | |
| 37 | 40 | return nil | |
| @@ -9,7 +9,7 @@ import ( | |||||
| 9 | // the immediately preceding successfully activated local session. A remote | 9 | // the immediately preceding successfully activated local session. A remote | ||
| 10 | // owner is retained in the owner history but is not interpreted as a local | 10 | // owner is retained in the owner history but is not interpreted as a local | ||
| 11 | // session; remote back navigation is published with remote transitions. | 11 | // session; remote back navigation is published with remote transitions. | ||
| 12 | - | func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error { | 12 | + | func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken, afterRemoteTransition ...func()) error { |
| 13 | if d == nil || token.ac == nil { | 13 | if d == nil || token.ac == nil { | ||
| 14 | return nil | 14 | return nil | ||
| 15 | } | 15 | } | ||
| @@ -32,6 +32,9 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error | |||||
| 32 | } | 32 | } | ||
| 33 | return err | 33 | return err | ||
| 34 | } | 34 | } | ||
| 35 | + | if len(afterRemoteTransition) != 0 && afterRemoteTransition[0] != nil { | |||
| 36 | + | afterRemoteTransition[0]() | |||
| 37 | + | } | |||
| 35 | d.touchMRU(target) | 38 | d.touchMRU(target) | ||
| 36 | d.firstPaintForTransition(published) | 39 | d.firstPaintForTransition(published) | ||
| 37 | return nil | 40 | return nil | ||
M scripts/remote-picker-harness/main.go +3 -1
| @@ -293,7 +293,9 @@ func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports | |||
| 293 | 293 | if err := assertNoRemoteChrome(local.probe, remoteSession); err != nil { | |
| 294 | 294 | return fmt.Errorf("remote content chrome: %w", err) | |
| 295 | 295 | } | |
| 296 | - | if err := sendInputAndAwait(ctx, local, "printf '"+inputMarker+"'", inputMarker); err != nil { | |
| 296 | + | // Split the marker so the terminal's echoed command line cannot satisfy the | |
| 297 | + | // awaited-output assertion before the shell executes it. | |
| 298 | + | if err := sendInputAndAwait(ctx, local, "printf '%s%s' 'VEV_REMOTE_PICKER' '_INPUT_OK'", inputMarker); err != nil { | |
| 297 | 299 | return fmt.Errorf("continued input on local transport: %w", err) | |
| 298 | 300 | } | |
| 299 | 301 | if err := assertNoDirectHandoff(local); err != nil { | |
| @@ -293,7 +293,9 @@ func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports | |||||
| 293 | if err := assertNoRemoteChrome(local.probe, remoteSession); err != nil { | 293 | if err := assertNoRemoteChrome(local.probe, remoteSession); err != nil { | ||
| 294 | return fmt.Errorf("remote content chrome: %w", err) | 294 | return fmt.Errorf("remote content chrome: %w", err) | ||
| 295 | } | 295 | } | ||
| 296 | - | if err := sendInputAndAwait(ctx, local, "printf '"+inputMarker+"'", inputMarker); err != nil { | 296 | + | // Split the marker so the terminal's echoed command line cannot satisfy the |
| 297 | + | // awaited-output assertion before the shell executes it. | |||
| 298 | + | if err := sendInputAndAwait(ctx, local, "printf '%s%s' 'VEV_REMOTE_PICKER' '_INPUT_OK'", inputMarker); err != nil { | |||
| 297 | return fmt.Errorf("continued input on local transport: %w", err) | 299 | return fmt.Errorf("continued input on local transport: %w", err) | ||
| 298 | } | 300 | } | ||
| 299 | if err := assertNoDirectHandoff(local); err != nil { | 301 | if err := assertNoDirectHandoff(local); err != nil { | ||
M scripts/remote-picker-harness/run.sh +4 -1
| @@ -119,7 +119,10 @@ harness_status=0 | |||
| 119 | 119 | if [ -n "$artifact_dir" ]; then | |
| 120 | 120 | docker exec --user test -e HOME=/home/test -e VEV_HARNESS_ARTIFACT_DIR=/tmp/vev-harness-artifact "$local_container" /usr/local/bin/remote-picker-harness || harness_status=$? | |
| 121 | 121 | if ! docker cp "$local_container:/tmp/vev-harness-artifact/remote-picker-harness.json" "$artifact_dir/remote-picker-harness.json"; then | |
| 122 | - | echo 'remote picker harness: failed to collect probe artifact' >&2 | |
| 122 | + | echo "remote picker harness: failed to collect probe artifact (acceptance status $harness_status)" >&2 | |
| 123 | + | if [ "$harness_status" -ne 0 ]; then | |
| 124 | + | exit "$harness_status" | |
| 125 | + | fi | |
| 123 | 126 | exit 1 | |
| 124 | 127 | fi | |
| 125 | 128 | chmod 0600 "$artifact_dir/remote-picker-harness.json" | |
| @@ -119,7 +119,10 @@ harness_status=0 | |||||
| 119 | if [ -n "$artifact_dir" ]; then | 119 | if [ -n "$artifact_dir" ]; then | ||
| 120 | docker exec --user test -e HOME=/home/test -e VEV_HARNESS_ARTIFACT_DIR=/tmp/vev-harness-artifact "$local_container" /usr/local/bin/remote-picker-harness || harness_status=$? | 120 | docker exec --user test -e HOME=/home/test -e VEV_HARNESS_ARTIFACT_DIR=/tmp/vev-harness-artifact "$local_container" /usr/local/bin/remote-picker-harness || harness_status=$? | ||
| 121 | if ! docker cp "$local_container:/tmp/vev-harness-artifact/remote-picker-harness.json" "$artifact_dir/remote-picker-harness.json"; then | 121 | if ! docker cp "$local_container:/tmp/vev-harness-artifact/remote-picker-harness.json" "$artifact_dir/remote-picker-harness.json"; then | ||
| 122 | - | echo 'remote picker harness: failed to collect probe artifact' >&2 | 122 | + | echo "remote picker harness: failed to collect probe artifact (acceptance status $harness_status)" >&2 |
| 123 | + | if [ "$harness_status" -ne 0 ]; then | |||
| 124 | + | exit "$harness_status" | |||
| 125 | + | fi | |||
| 123 | exit 1 | 126 | exit 1 | ||
| 124 | fi | 127 | fi | ||
| 125 | chmod 0600 "$artifact_dir/remote-picker-harness.json" | 128 | chmod 0600 "$artifact_dir/remote-picker-harness.json" | ||
M scripts/remote-picker-harness/visual_probe.go +4 -1
| @@ -208,6 +208,9 @@ func (p *visualProbe) apply(output ports.Output) visualOutputResult { | |||
| 208 | 208 | ||
| 209 | 209 | // The client writes side effects as well as state-bearing frames, but only | |
| 210 | 210 | // state-bearing frames advance the dependency chain and receive an ACK. | |
| 211 | + | if p.screen.Frame.Width != output.Size.Cols || p.screen.Frame.Height != output.Size.Rows { | |
| 212 | + | p.screen.Resize(output.Size.Cols, output.Size.Rows) | |
| 213 | + | } | |
| 211 | 214 | p.screen.Write(output.Data) | |
| 212 | 215 | p.state = next | |
| 213 | 216 | checkpoint := visualCheckpoint{event: event, state: p.state, snapshot: p.screen.Snapshot()} | |
| @@ -320,7 +323,7 @@ func assertNoRemoteChrome(probe *visualProbe, remoteSession string) error { | |||
| 320 | 323 | // The first and last rows belong to the local attachment's bars. Remote | |
| 321 | 324 | // daemon chrome must not be replayed into the content rows. | |
| 322 | 325 | for _, line := range lines[1 : len(lines)-1] { | |
| 323 | - | if strings.Contains(line, remoteSession+" at remote") || strings.Contains(line, " Sessions · ") { | |
| 326 | + | if strings.Contains(line, remoteSession+"@remote") || strings.Contains(line, " Sessions · ") { | |
| 324 | 327 | return fmt.Errorf("remote chrome leaked into content row %q", strings.TrimSpace(line)) | |
| 325 | 328 | } | |
| 326 | 329 | } | |
| @@ -208,6 +208,9 @@ func (p *visualProbe) apply(output ports.Output) visualOutputResult { | |||||
| 208 | 208 | ||||
| 209 | // The client writes side effects as well as state-bearing frames, but only | 209 | // The client writes side effects as well as state-bearing frames, but only | ||
| 210 | // state-bearing frames advance the dependency chain and receive an ACK. | 210 | // state-bearing frames advance the dependency chain and receive an ACK. | ||
| 211 | + | if p.screen.Frame.Width != output.Size.Cols || p.screen.Frame.Height != output.Size.Rows { | |||
| 212 | + | p.screen.Resize(output.Size.Cols, output.Size.Rows) | |||
| 213 | + | } | |||
| 211 | p.screen.Write(output.Data) | 214 | p.screen.Write(output.Data) | ||
| 212 | p.state = next | 215 | p.state = next | ||
| 213 | checkpoint := visualCheckpoint{event: event, state: p.state, snapshot: p.screen.Snapshot()} | 216 | checkpoint := visualCheckpoint{event: event, state: p.state, snapshot: p.screen.Snapshot()} | ||
| @@ -320,7 +323,7 @@ func assertNoRemoteChrome(probe *visualProbe, remoteSession string) error { | |||||
| 320 | // The first and last rows belong to the local attachment's bars. Remote | 323 | // The first and last rows belong to the local attachment's bars. Remote | ||
| 321 | // daemon chrome must not be replayed into the content rows. | 324 | // daemon chrome must not be replayed into the content rows. | ||
| 322 | for _, line := range lines[1 : len(lines)-1] { | 325 | for _, line := range lines[1 : len(lines)-1] { | ||
| 323 | - | if strings.Contains(line, remoteSession+" at remote") || strings.Contains(line, " Sessions · ") { | 326 | + | if strings.Contains(line, remoteSession+"@remote") || strings.Contains(line, " Sessions · ") { |
| 324 | return fmt.Errorf("remote chrome leaked into content row %q", strings.TrimSpace(line)) | 327 | return fmt.Errorf("remote chrome leaked into content row %q", strings.TrimSpace(line)) | ||
| 325 | } | 328 | } | ||
| 326 | } | 329 | } | ||
M scripts/remote-picker-harness/visual_probe_test.go +17 -2
| @@ -84,6 +84,21 @@ func TestVisualProbePersistsFullAndIncrementalOutput(t *testing.T) { | |||
| 84 | 84 | } | |
| 85 | 85 | } | |
| 86 | 86 | ||
| 87 | + | func TestVisualProbeResizesToAcceptedOutput(t *testing.T) { | |
| 88 | + | probe := newVisualProbe(domain.Size{Cols: probeTestCols, Rows: probeTestRows}) | |
| 89 | + | outputSize := domain.Size{Cols: 48, Rows: 3} | |
| 90 | + | result := probe.apply(ports.Output{ | |
| 91 | + | Epoch: 1, New: 1, Full: true, ViewRevision: 1, Size: outputSize, Data: []byte("resized"), | |
| 92 | + | }) | |
| 93 | + | ||
| 94 | + | require.True(t, result.Accepted) | |
| 95 | + | require.Equal(t, outputSize.Cols, probe.screen.Frame.Width) | |
| 96 | + | require.Equal(t, outputSize.Rows, probe.screen.Frame.Height) | |
| 97 | + | require.Len(t, probe.checkpoints, 1) | |
| 98 | + | require.Equal(t, outputSize.Cols, probe.checkpoints[0].snapshot.Columns()) | |
| 99 | + | require.Equal(t, outputSize.Rows, probe.checkpoints[0].snapshot.Rows()) | |
| 100 | + | } | |
| 101 | + | ||
| 87 | 102 | func TestVisualProbeRejectsOutputWithoutMutationOrAck(t *testing.T) { | |
| 88 | 103 | size := domain.Size{Cols: probeTestCols, Rows: probeTestRows} | |
| 89 | 104 | for _, tt := range []struct { | |
| @@ -269,8 +284,8 @@ func TestAssertNoRemoteChromeIgnoresLocalBarsButRejectsContentLeak(t *testing.T) | |||
| 269 | 284 | content string | |
| 270 | 285 | wantErr bool | |
| 271 | 286 | }{ | |
| 272 | - | {name: "local bars", content: "picker at remote\nremote content\nlocal bottom", wantErr: false}, | |
| 273 | - | {name: "remote status in content", content: "local top\npicker at remote\nlocal bottom", wantErr: true}, | |
| 287 | + | {name: "local bars", content: "picker@remote\r\nremote content\r\nlocal bottom", wantErr: false}, | |
| 288 | + | {name: "remote status in content", content: "local top\r\npicker@remote\r\nlocal bottom", wantErr: true}, | |
| 274 | 289 | } { | |
| 275 | 290 | t.Run(test.name, func(t *testing.T) { | |
| 276 | 291 | probe := newVisualProbe(domain.Size{Cols: 48, Rows: 3}) | |
| @@ -84,6 +84,21 @@ func TestVisualProbePersistsFullAndIncrementalOutput(t *testing.T) { | |||||
| 84 | } | 84 | } | ||
| 85 | } | 85 | } | ||
| 86 | 86 | ||||
| 87 | + | func TestVisualProbeResizesToAcceptedOutput(t *testing.T) { | |||
| 88 | + | probe := newVisualProbe(domain.Size{Cols: probeTestCols, Rows: probeTestRows}) | |||
| 89 | + | outputSize := domain.Size{Cols: 48, Rows: 3} | |||
| 90 | + | result := probe.apply(ports.Output{ | |||
| 91 | + | Epoch: 1, New: 1, Full: true, ViewRevision: 1, Size: outputSize, Data: []byte("resized"), | |||
| 92 | + | }) | |||
| 93 | + | ||||
| 94 | + | require.True(t, result.Accepted) | |||
| 95 | + | require.Equal(t, outputSize.Cols, probe.screen.Frame.Width) | |||
| 96 | + | require.Equal(t, outputSize.Rows, probe.screen.Frame.Height) | |||
| 97 | + | require.Len(t, probe.checkpoints, 1) | |||
| 98 | + | require.Equal(t, outputSize.Cols, probe.checkpoints[0].snapshot.Columns()) | |||
| 99 | + | require.Equal(t, outputSize.Rows, probe.checkpoints[0].snapshot.Rows()) | |||
| 100 | + | } | |||
| 101 | + | ||||
| 87 | func TestVisualProbeRejectsOutputWithoutMutationOrAck(t *testing.T) { | 102 | func TestVisualProbeRejectsOutputWithoutMutationOrAck(t *testing.T) { | ||
| 88 | size := domain.Size{Cols: probeTestCols, Rows: probeTestRows} | 103 | size := domain.Size{Cols: probeTestCols, Rows: probeTestRows} | ||
| 89 | for _, tt := range []struct { | 104 | for _, tt := range []struct { | ||
| @@ -269,8 +284,8 @@ func TestAssertNoRemoteChromeIgnoresLocalBarsButRejectsContentLeak(t *testing.T) | |||||
| 269 | content string | 284 | content string | ||
| 270 | wantErr bool | 285 | wantErr bool | ||
| 271 | }{ | 286 | }{ | ||
| 272 | - | {name: "local bars", content: "picker at remote\nremote content\nlocal bottom", wantErr: false}, | 287 | + | {name: "local bars", content: "picker@remote\r\nremote content\r\nlocal bottom", wantErr: false}, |
| 273 | - | {name: "remote status in content", content: "local top\npicker at remote\nlocal bottom", wantErr: true}, | 288 | + | {name: "remote status in content", content: "local top\r\npicker@remote\r\nlocal bottom", wantErr: true}, |
| 274 | } { | 289 | } { | ||
| 275 | t.Run(test.name, func(t *testing.T) { | 290 | t.Run(test.name, func(t *testing.T) { | ||
| 276 | probe := newVisualProbe(domain.Size{Cols: 48, Rows: 3}) | 291 | probe := newVisualProbe(domain.Size{Cols: 48, Rows: 3}) | ||