fix(remote): preserve picker tab identity
- commit
- d2f99579e7455bf069d172206ce8b3615045456d
- author
- brice · · 2026-07-31 21:07 UTC
- parent
- 67576a2
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +44 -2
M internal/usecase/daemon/attachment_transition.go +3 -0
| @@ -14,6 +14,9 @@ type attachmentLifecycleFence struct { | |||
| 14 | 14 | checkCreatedAt bool | |
| 15 | 15 | incarnation domain.IncarnationID | |
| 16 | 16 | checkIncarnation bool | |
| 17 | + | tabID domain.TabStableID | |
| 18 | + | tabIndex int | |
| 19 | + | checkTab bool | |
| 17 | 20 | } | |
| 18 | 21 | ||
| 19 | 22 | type attachmentTransitionRequest struct { | |
| @@ -14,6 +14,9 @@ type attachmentLifecycleFence struct { | |||||
| 14 | checkCreatedAt bool | 14 | checkCreatedAt bool | ||
| 15 | incarnation domain.IncarnationID | 15 | incarnation domain.IncarnationID | ||
| 16 | checkIncarnation bool | 16 | checkIncarnation bool | ||
| 17 | + | tabID domain.TabStableID | |||
| 18 | + | tabIndex int | |||
| 19 | + | checkTab bool | |||
| 17 | } | 20 | } | ||
| 18 | 21 | ||||
| 19 | type attachmentTransitionRequest struct { | 22 | type attachmentTransitionRequest struct { | ||
M internal/usecase/daemon/attachment_transition_publish.go +7 -1
| @@ -1,5 +1,7 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | + | import "github.com/bnema/vev/internal/domain" | |
| 4 | + | ||
| 3 | 5 | // transitionSourcePreflightLocked validates an initiating role while its gate | |
| 4 | 6 | // is frozen and d.mu is held. No caller may create or delete a target before | |
| 5 | 7 | // this succeeds. | |
| @@ -105,7 +107,11 @@ func attachmentLifecycleCurrentLocked(entry attachmentSession, fence *attachment | |||
| 105 | 107 | if fence.checkCreatedAt && (target.name != fence.name || target.createdAt != fence.createdAt) { | |
| 106 | 108 | return false | |
| 107 | 109 | } | |
| 108 | - | return !fence.checkIncarnation || target.incarnation == fence.incarnation | |
| 110 | + | if fence.checkIncarnation && target.incarnation != fence.incarnation { | |
| 111 | + | return false | |
| 112 | + | } | |
| 113 | + | return !fence.checkTab || fence.tabIndex >= 0 && fence.tabIndex < len(target.tabs) && | |
| 114 | + | domain.TabStableID(target.tabs[fence.tabIndex].stableID) == fence.tabID | |
| 109 | 115 | } | |
| 110 | 116 | ||
| 111 | 117 | // validateAttachmentTransitionPrelocked performs every fallible transition | |
| @@ -1,5 +1,7 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | + | import "github.com/bnema/vev/internal/domain" | |||
| 4 | + | ||||
| 3 | // transitionSourcePreflightLocked validates an initiating role while its gate | 5 | // transitionSourcePreflightLocked validates an initiating role while its gate | ||
| 4 | // is frozen and d.mu is held. No caller may create or delete a target before | 6 | // is frozen and d.mu is held. No caller may create or delete a target before | ||
| 5 | // this succeeds. | 7 | // this succeeds. | ||
| @@ -105,7 +107,11 @@ func attachmentLifecycleCurrentLocked(entry attachmentSession, fence *attachment | |||||
| 105 | if fence.checkCreatedAt && (target.name != fence.name || target.createdAt != fence.createdAt) { | 107 | if fence.checkCreatedAt && (target.name != fence.name || target.createdAt != fence.createdAt) { | ||
| 106 | return false | 108 | return false | ||
| 107 | } | 109 | } | ||
| 108 | - | return !fence.checkIncarnation || target.incarnation == fence.incarnation | 110 | + | if fence.checkIncarnation && target.incarnation != fence.incarnation { |
| 111 | + | return false | |||
| 112 | + | } | |||
| 113 | + | return !fence.checkTab || fence.tabIndex >= 0 && fence.tabIndex < len(target.tabs) && | |||
| 114 | + | domain.TabStableID(target.tabs[fence.tabIndex].stableID) == fence.tabID | |||
| 109 | } | 115 | } | ||
| 110 | 116 | ||||
| 111 | // validateAttachmentTransitionPrelocked performs every fallible transition | 117 | // validateAttachmentTransitionPrelocked performs every fallible transition | ||
M internal/usecase/daemon/picker.go +6 -1
| @@ -592,7 +592,12 @@ func pickerTargetLifecycleFence(target picker.Target) *attachmentLifecycleFence | |||
| 592 | 592 | fence.incarnation = target.Incarnation | |
| 593 | 593 | fence.checkIncarnation = true | |
| 594 | 594 | } | |
| 595 | - | if !fence.checkCreatedAt && !fence.checkIncarnation { | |
| 595 | + | if target.TabID != "" { | |
| 596 | + | fence.tabID = target.TabID | |
| 597 | + | fence.tabIndex = target.TabIndex | |
| 598 | + | fence.checkTab = true | |
| 599 | + | } | |
| 600 | + | if !fence.checkCreatedAt && !fence.checkIncarnation && !fence.checkTab { | |
| 596 | 601 | return nil | |
| 597 | 602 | } | |
| 598 | 603 | return fence | |
| @@ -592,7 +592,12 @@ func pickerTargetLifecycleFence(target picker.Target) *attachmentLifecycleFence | |||||
| 592 | fence.incarnation = target.Incarnation | 592 | fence.incarnation = target.Incarnation | ||
| 593 | fence.checkIncarnation = true | 593 | fence.checkIncarnation = true | ||
| 594 | } | 594 | } | ||
| 595 | - | if !fence.checkCreatedAt && !fence.checkIncarnation { | 595 | + | if target.TabID != "" { |
| 596 | + | fence.tabID = target.TabID | |||
| 597 | + | fence.tabIndex = target.TabIndex | |||
| 598 | + | fence.checkTab = true | |||
| 599 | + | } | |||
| 600 | + | if !fence.checkCreatedAt && !fence.checkIncarnation && !fence.checkTab { | |||
| 596 | return nil | 601 | return nil | ||
| 597 | } | 602 | } | ||
| 598 | return fence | 603 | return fence | ||
M internal/usecase/daemon/proxy_input_test.go +28 -0
| @@ -734,6 +734,34 @@ func TestRemotePickerRejectsReplacedLocalLifecycle(t *testing.T) { | |||
| 734 | 734 | require.Same(t, proxy, ac.currentAttachmentSession(), "stale picker lifecycle must not redirect the attachment") | |
| 735 | 735 | } | |
| 736 | 736 | ||
| 737 | + | func TestRemotePickerRejectsReplacedLocalTab(t *testing.T) { | |
| 738 | + | d, proxy, ac, _, handler := newProxyInputHarness(t) | |
| 739 | + | first := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |
| 740 | + | first.stableID = "first" | |
| 741 | + | second := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |
| 742 | + | second.stableID = "second" | |
| 743 | + | local := &session{sessionCore: sessionCore{id: "local", name: "local"}, tabs: []*tab{first, second}} | |
| 744 | + | publishTiledPaneOwners(local, first) | |
| 745 | + | publishTiledPaneOwners(local, second) | |
| 746 | + | d.mu.Lock() | |
| 747 | + | require.True(t, d.registerSessionLocked(local)) | |
| 748 | + | d.mu.Unlock() | |
| 749 | + | ||
| 750 | + | model := d.newPickerModel(proxy, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{Session: local.id, TabID: domain.TabStableID(second.stableID)}) | |
| 751 | + | target, selectable := model.Selected() | |
| 752 | + | require.True(t, selectable) | |
| 753 | + | require.Equal(t, domain.TabStableID(second.stableID), target.TabID) | |
| 754 | + | d.afterRoleEffectsFrozen = func() { | |
| 755 | + | d.afterRoleEffectsFrozen = nil | |
| 756 | + | local.mu.Lock() | |
| 757 | + | local.tabs[0], local.tabs[1] = local.tabs[1], local.tabs[0] | |
| 758 | + | local.mu.Unlock() | |
| 759 | + | } | |
| 760 | + | ||
| 761 | + | require.Error(t, d.switchToTargetForRole(handler.roleToken, target, sessionHandoffGuard{}, "picker-select")) | |
| 762 | + | require.Same(t, proxy, ac.currentAttachmentSession(), "stale picker tab must not redirect the attachment") | |
| 763 | + | } | |
| 764 | + | ||
| 737 | 765 | func TestRemotePickerCanReselectWarmProxyAfterReturningLocal(t *testing.T) { | |
| 738 | 766 | d, local, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | |
| 739 | 767 | d.attachCoordinator(local, nil, ac, true) | |
| @@ -734,6 +734,34 @@ func TestRemotePickerRejectsReplacedLocalLifecycle(t *testing.T) { | |||||
| 734 | require.Same(t, proxy, ac.currentAttachmentSession(), "stale picker lifecycle must not redirect the attachment") | 734 | require.Same(t, proxy, ac.currentAttachmentSession(), "stale picker lifecycle must not redirect the attachment") | ||
| 735 | } | 735 | } | ||
| 736 | 736 | ||||
| 737 | + | func TestRemotePickerRejectsReplacedLocalTab(t *testing.T) { | |||
| 738 | + | d, proxy, ac, _, handler := newProxyInputHarness(t) | |||
| 739 | + | first := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |||
| 740 | + | first.stableID = "first" | |||
| 741 | + | second := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |||
| 742 | + | second.stableID = "second" | |||
| 743 | + | local := &session{sessionCore: sessionCore{id: "local", name: "local"}, tabs: []*tab{first, second}} | |||
| 744 | + | publishTiledPaneOwners(local, first) | |||
| 745 | + | publishTiledPaneOwners(local, second) | |||
| 746 | + | d.mu.Lock() | |||
| 747 | + | require.True(t, d.registerSessionLocked(local)) | |||
| 748 | + | d.mu.Unlock() | |||
| 749 | + | ||||
| 750 | + | model := d.newPickerModel(proxy, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{Session: local.id, TabID: domain.TabStableID(second.stableID)}) | |||
| 751 | + | target, selectable := model.Selected() | |||
| 752 | + | require.True(t, selectable) | |||
| 753 | + | require.Equal(t, domain.TabStableID(second.stableID), target.TabID) | |||
| 754 | + | d.afterRoleEffectsFrozen = func() { | |||
| 755 | + | d.afterRoleEffectsFrozen = nil | |||
| 756 | + | local.mu.Lock() | |||
| 757 | + | local.tabs[0], local.tabs[1] = local.tabs[1], local.tabs[0] | |||
| 758 | + | local.mu.Unlock() | |||
| 759 | + | } | |||
| 760 | + | ||||
| 761 | + | require.Error(t, d.switchToTargetForRole(handler.roleToken, target, sessionHandoffGuard{}, "picker-select")) | |||
| 762 | + | require.Same(t, proxy, ac.currentAttachmentSession(), "stale picker tab must not redirect the attachment") | |||
| 763 | + | } | |||
| 764 | + | ||||
| 737 | func TestRemotePickerCanReselectWarmProxyAfterReturningLocal(t *testing.T) { | 765 | func TestRemotePickerCanReselectWarmProxyAfterReturningLocal(t *testing.T) { | ||
| 738 | d, local, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | 766 | d, local, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | ||
| 739 | d.attachCoordinator(local, nil, ac, true) | 767 | d.attachCoordinator(local, nil, ac, true) | ||