fix(navigation): fence target lifecycle at publication
- commit
- 736629f3cf9637fa520c1bd6765a2ece7b9b184a
- author
- brice · · 2026-08-18 21:01 UTC
- committer
- brice · · 2026-08-19 04:15 UTC
- parent
- 5639083
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +78 -25
M internal/usecase/daemon/attachment_effect_gate_test.go +5 -4
| @@ -701,10 +701,11 @@ func TestAttachedNavigationCommandSendsResultAfterLocalTransition(t *testing.T) | |||
| 701 | 701 | d.sessions[target.id] = target | |
| 702 | 702 | d.mu.Unlock() | |
| 703 | 703 | ac.setRouteSnapshot(ports.RecentRouteSnapshot{ | |
| 704 | - | Generation: 2, | |
| 705 | - | Active: ports.RouteRef{Key: 2, Generation: 2}, | |
| 706 | - | Previous: ports.RouteRef{Key: 1, Generation: 1}, | |
| 707 | - | Entries: []ports.RecentRouteEntry{testRouteEntry(1, 1, "target", 1, ports.RouteKindLocal)}, | |
| 704 | + | Generation: 2, | |
| 705 | + | Active: ports.RouteRef{Key: 2, Generation: 2}, | |
| 706 | + | ActiveEntry: testRouteEntry(2, 2, source.name, 2, ports.RouteKindLocal), | |
| 707 | + | Previous: ports.RouteRef{Key: 1, Generation: 1}, | |
| 708 | + | Entries: []ports.RecentRouteEntry{testRouteEntry(1, 1, "target", 1, ports.RouteKindLocal)}, | |
| 708 | 709 | }) | |
| 709 | 710 | ||
| 710 | 711 | transport := &closeTrackingTransport{} | |
| @@ -701,10 +701,11 @@ func TestAttachedNavigationCommandSendsResultAfterLocalTransition(t *testing.T) | |||||
| 701 | d.sessions[target.id] = target | 701 | d.sessions[target.id] = target | ||
| 702 | d.mu.Unlock() | 702 | d.mu.Unlock() | ||
| 703 | ac.setRouteSnapshot(ports.RecentRouteSnapshot{ | 703 | ac.setRouteSnapshot(ports.RecentRouteSnapshot{ | ||
| 704 | - | Generation: 2, | 704 | + | Generation: 2, |
| 705 | - | Active: ports.RouteRef{Key: 2, Generation: 2}, | 705 | + | Active: ports.RouteRef{Key: 2, Generation: 2}, |
| 706 | - | Previous: ports.RouteRef{Key: 1, Generation: 1}, | 706 | + | ActiveEntry: testRouteEntry(2, 2, source.name, 2, ports.RouteKindLocal), |
| 707 | - | Entries: []ports.RecentRouteEntry{testRouteEntry(1, 1, "target", 1, ports.RouteKindLocal)}, | 707 | + | Previous: ports.RouteRef{Key: 1, Generation: 1}, |
| 708 | + | Entries: []ports.RecentRouteEntry{testRouteEntry(1, 1, "target", 1, ports.RouteKindLocal)}, | |||
| 708 | }) | 709 | }) | ||
| 709 | 710 | ||||
| 710 | transport := &closeTrackingTransport{} | 711 | transport := &closeTrackingTransport{} | ||
M internal/usecase/daemon/navigation_attachment_incarnation_test.go +40 -0
| @@ -8,6 +8,46 @@ import ( | |||
| 8 | 8 | "github.com/stretchr/testify/require" | |
| 9 | 9 | ) | |
| 10 | 10 | ||
| 11 | + | func TestNavigationHandoffRejectsReplacedTargetLifecycleAtPublication(t *testing.T) { | |
| 12 | + | d, source, ac, _ := newManualSessionWithPTYs(t, nil) | |
| 13 | + | target := &session{sessionCore: sessionCore{id: "target", name: "target", incarnation: domain.SessionLifecycleID{1}}, ctx: source.ctx, cancel: func() {}, tabs: []*tab{ | |
| 14 | + | newTab(nil, domain.Size{Cols: 80, Rows: 23}), | |
| 15 | + | newTab(nil, domain.Size{Cols: 80, Rows: 23}), | |
| 16 | + | }} | |
| 17 | + | d.mu.Lock() | |
| 18 | + | d.sessions[target.id] = target | |
| 19 | + | d.mu.Unlock() | |
| 20 | + | ||
| 21 | + | rc := d.attachCoordinator(source, nil, ac, true) | |
| 22 | + | token := source.attachmentToken(ac, ac.transport()) | |
| 23 | + | token.lease = rc.attachmentLease(ac) | |
| 24 | + | ac.publishAttachmentCapability(token) | |
| 25 | + | effect, admitted := ac.beginAttachmentEffect(token) | |
| 26 | + | require.True(t, admitted) | |
| 27 | + | ||
| 28 | + | d.afterAttachmentTransitionCoordinatorsLocked = func() { | |
| 29 | + | // This hook runs with the target session locked, immediately before the | |
| 30 | + | // transition's lifecycle fence is validated. | |
| 31 | + | target.incarnation = domain.SessionLifecycleID{2} | |
| 32 | + | } | |
| 33 | + | err := d.switchToTargetForAttachment(effect.connectionToken(), picker.Target{ | |
| 34 | + | Session: target.id, | |
| 35 | + | Incarnation: domain.SessionLifecycleID{1}, | |
| 36 | + | TabID: domain.TabStableID(target.tabs[1].stableID), | |
| 37 | + | TabIndex: 1, | |
| 38 | + | }, sessionHandoffGuard{}, "test-handoff") | |
| 39 | + | ||
| 40 | + | require.Error(t, err) | |
| 41 | + | var userErr *domain.UserError | |
| 42 | + | require.ErrorAs(t, err, &userErr) | |
| 43 | + | require.Equal(t, domain.NoticeSessionUnavailable, userErr.Code) | |
| 44 | + | require.Same(t, source, ac.currentSession()) | |
| 45 | + | target.mu.Lock() | |
| 46 | + | require.Empty(t, target.snapshotAttachmentsLocked()) | |
| 47 | + | require.Zero(t, testAttachmentTabIndexLocked(target)) | |
| 48 | + | target.mu.Unlock() | |
| 49 | + | } | |
| 50 | + | ||
| 11 | 51 | func TestNavigationHandoffDoesNotMutateAfterInitiatorIncarnationChanges(t *testing.T) { | |
| 12 | 52 | tests := []struct { | |
| 13 | 53 | name string | |
| @@ -8,6 +8,46 @@ import ( | |||||
| 8 | "github.com/stretchr/testify/require" | 8 | "github.com/stretchr/testify/require" | ||
| 9 | ) | 9 | ) | ||
| 10 | 10 | ||||
| 11 | + | func TestNavigationHandoffRejectsReplacedTargetLifecycleAtPublication(t *testing.T) { | |||
| 12 | + | d, source, ac, _ := newManualSessionWithPTYs(t, nil) | |||
| 13 | + | target := &session{sessionCore: sessionCore{id: "target", name: "target", incarnation: domain.SessionLifecycleID{1}}, ctx: source.ctx, cancel: func() {}, tabs: []*tab{ | |||
| 14 | + | newTab(nil, domain.Size{Cols: 80, Rows: 23}), | |||
| 15 | + | newTab(nil, domain.Size{Cols: 80, Rows: 23}), | |||
| 16 | + | }} | |||
| 17 | + | d.mu.Lock() | |||
| 18 | + | d.sessions[target.id] = target | |||
| 19 | + | d.mu.Unlock() | |||
| 20 | + | ||||
| 21 | + | rc := d.attachCoordinator(source, nil, ac, true) | |||
| 22 | + | token := source.attachmentToken(ac, ac.transport()) | |||
| 23 | + | token.lease = rc.attachmentLease(ac) | |||
| 24 | + | ac.publishAttachmentCapability(token) | |||
| 25 | + | effect, admitted := ac.beginAttachmentEffect(token) | |||
| 26 | + | require.True(t, admitted) | |||
| 27 | + | ||||
| 28 | + | d.afterAttachmentTransitionCoordinatorsLocked = func() { | |||
| 29 | + | // This hook runs with the target session locked, immediately before the | |||
| 30 | + | // transition's lifecycle fence is validated. | |||
| 31 | + | target.incarnation = domain.SessionLifecycleID{2} | |||
| 32 | + | } | |||
| 33 | + | err := d.switchToTargetForAttachment(effect.connectionToken(), picker.Target{ | |||
| 34 | + | Session: target.id, | |||
| 35 | + | Incarnation: domain.SessionLifecycleID{1}, | |||
| 36 | + | TabID: domain.TabStableID(target.tabs[1].stableID), | |||
| 37 | + | TabIndex: 1, | |||
| 38 | + | }, sessionHandoffGuard{}, "test-handoff") | |||
| 39 | + | ||||
| 40 | + | require.Error(t, err) | |||
| 41 | + | var userErr *domain.UserError | |||
| 42 | + | require.ErrorAs(t, err, &userErr) | |||
| 43 | + | require.Equal(t, domain.NoticeSessionUnavailable, userErr.Code) | |||
| 44 | + | require.Same(t, source, ac.currentSession()) | |||
| 45 | + | target.mu.Lock() | |||
| 46 | + | require.Empty(t, target.snapshotAttachmentsLocked()) | |||
| 47 | + | require.Zero(t, testAttachmentTabIndexLocked(target)) | |||
| 48 | + | target.mu.Unlock() | |||
| 49 | + | } | |||
| 50 | + | ||||
| 11 | func TestNavigationHandoffDoesNotMutateAfterInitiatorIncarnationChanges(t *testing.T) { | 51 | func TestNavigationHandoffDoesNotMutateAfterInitiatorIncarnationChanges(t *testing.T) { | ||
| 12 | tests := []struct { | 52 | tests := []struct { | ||
| 13 | name string | 53 | name string | ||
M internal/usecase/daemon/palette_test.go +17 -11
| @@ -21,8 +21,9 @@ import ( | |||
| 21 | 21 | ||
| 22 | 22 | func testRecentRouteSnapshot() ports.RecentRouteSnapshot { | |
| 23 | 23 | return ports.RecentRouteSnapshot{ | |
| 24 | - | Generation: 1, | |
| 25 | - | Active: ports.RouteRef{Key: 1, Generation: 1}, | |
| 24 | + | Generation: 1, | |
| 25 | + | Active: ports.RouteRef{Key: 1, Generation: 1}, | |
| 26 | + | ActiveEntry: testRouteEntry(1, 1, "current", 1, ports.RouteKindLocal), | |
| 26 | 27 | Entries: []ports.RecentRouteEntry{ | |
| 27 | 28 | testRouteEntry(2, 1, "recent", 2, ports.RouteKindLocal), | |
| 28 | 29 | testRouteEntry(3, 1, "older", 3, ports.RouteKindLocal), | |
| @@ -476,10 +477,13 @@ func TestPaletteLifecycleTargetRejectsSameNameReplacement(t *testing.T) { | |||
| 476 | 477 | ||
| 477 | 478 | require.ErrorIs(t, err, errAttachmentTransition) | |
| 478 | 479 | require.Same(t, current, ac.currentSession()) | |
| 479 | - | select { | |
| 480 | - | case frame := <-sends: | |
| 481 | - | require.NotEqual(t, ports.MsgAttachTarget, frame.Type) | |
| 482 | - | default: | |
| 480 | + | for { | |
| 481 | + | select { | |
| 482 | + | case frame := <-sends: | |
| 483 | + | require.NotEqual(t, ports.MsgAttachTarget, frame.Type) | |
| 484 | + | default: | |
| 485 | + | return | |
| 486 | + | } | |
| 483 | 487 | } | |
| 484 | 488 | } | |
| 485 | 489 | ||
| @@ -487,8 +491,9 @@ func TestPaletteFuzzyRemoteRecentRouteSendsExactNavigationAction(t *testing.T) { | |||
| 487 | 491 | d, current, ac, sends, releases := newRecentNavigationTestSessions(t) | |
| 488 | 492 | defer releaseAll(releases) | |
| 489 | 493 | ac.setRouteSnapshot(ports.RecentRouteSnapshot{ | |
| 490 | - | Generation: 9, | |
| 491 | - | Active: ports.RouteRef{Key: 1, Generation: 1}, | |
| 494 | + | Generation: 9, | |
| 495 | + | Active: ports.RouteRef{Key: 1, Generation: 1}, | |
| 496 | + | ActiveEntry: testRouteEntry(1, 1, current.name, 1, ports.RouteKindLocal), | |
| 492 | 497 | Entries: []ports.RecentRouteEntry{{ | |
| 493 | 498 | Key: 8, Generation: 4, Target: testRouteTarget("logs", 8), Name: "logs", HostLabel: "edge", Kind: ports.RouteKindRemote, | |
| 494 | 499 | }}, | |
| @@ -566,9 +571,10 @@ func TestPaletteJRSDoesNotRevalidateTargetInDaemon(t *testing.T) { | |||
| 566 | 571 | defer release() | |
| 567 | 572 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |
| 568 | 573 | ac.setRouteSnapshot(ports.RecentRouteSnapshot{ | |
| 569 | - | Generation: 7, | |
| 570 | - | Active: ports.RouteRef{Key: 1, Generation: 1}, | |
| 571 | - | Entries: []ports.RecentRouteEntry{testRouteEntry(9, 4, "captured", 9, ports.RouteKindLocal)}, | |
| 574 | + | Generation: 7, | |
| 575 | + | Active: ports.RouteRef{Key: 1, Generation: 1}, | |
| 576 | + | ActiveEntry: testRouteEntry(1, 1, sess.name, 1, ports.RouteKindLocal), | |
| 577 | + | Entries: []ports.RecentRouteEntry{testRouteEntry(9, 4, "captured", 9, ports.RouteKindLocal)}, | |
| 572 | 578 | }) | |
| 573 | 579 | ||
| 574 | 580 | validated := make(chan struct{}) | |
| @@ -21,8 +21,9 @@ import ( | |||||
| 21 | 21 | ||||
| 22 | func testRecentRouteSnapshot() ports.RecentRouteSnapshot { | 22 | func testRecentRouteSnapshot() ports.RecentRouteSnapshot { | ||
| 23 | return ports.RecentRouteSnapshot{ | 23 | return ports.RecentRouteSnapshot{ | ||
| 24 | - | Generation: 1, | 24 | + | Generation: 1, |
| 25 | - | Active: ports.RouteRef{Key: 1, Generation: 1}, | 25 | + | Active: ports.RouteRef{Key: 1, Generation: 1}, |
| 26 | + | ActiveEntry: testRouteEntry(1, 1, "current", 1, ports.RouteKindLocal), | |||
| 26 | Entries: []ports.RecentRouteEntry{ | 27 | Entries: []ports.RecentRouteEntry{ | ||
| 27 | testRouteEntry(2, 1, "recent", 2, ports.RouteKindLocal), | 28 | testRouteEntry(2, 1, "recent", 2, ports.RouteKindLocal), | ||
| 28 | testRouteEntry(3, 1, "older", 3, ports.RouteKindLocal), | 29 | testRouteEntry(3, 1, "older", 3, ports.RouteKindLocal), | ||
| @@ -476,10 +477,13 @@ func TestPaletteLifecycleTargetRejectsSameNameReplacement(t *testing.T) { | |||||
| 476 | 477 | ||||
| 477 | require.ErrorIs(t, err, errAttachmentTransition) | 478 | require.ErrorIs(t, err, errAttachmentTransition) | ||
| 478 | require.Same(t, current, ac.currentSession()) | 479 | require.Same(t, current, ac.currentSession()) | ||
| 479 | - | select { | 480 | + | for { |
| 480 | - | case frame := <-sends: | 481 | + | select { |
| 481 | - | require.NotEqual(t, ports.MsgAttachTarget, frame.Type) | 482 | + | case frame := <-sends: |
| 482 | - | default: | 483 | + | require.NotEqual(t, ports.MsgAttachTarget, frame.Type) |
| 484 | + | default: | |||
| 485 | + | return | |||
| 486 | + | } | |||
| 483 | } | 487 | } | ||
| 484 | } | 488 | } | ||
| 485 | 489 | ||||
| @@ -487,8 +491,9 @@ func TestPaletteFuzzyRemoteRecentRouteSendsExactNavigationAction(t *testing.T) { | |||||
| 487 | d, current, ac, sends, releases := newRecentNavigationTestSessions(t) | 491 | d, current, ac, sends, releases := newRecentNavigationTestSessions(t) | ||
| 488 | defer releaseAll(releases) | 492 | defer releaseAll(releases) | ||
| 489 | ac.setRouteSnapshot(ports.RecentRouteSnapshot{ | 493 | ac.setRouteSnapshot(ports.RecentRouteSnapshot{ | ||
| 490 | - | Generation: 9, | 494 | + | Generation: 9, |
| 491 | - | Active: ports.RouteRef{Key: 1, Generation: 1}, | 495 | + | Active: ports.RouteRef{Key: 1, Generation: 1}, |
| 496 | + | ActiveEntry: testRouteEntry(1, 1, current.name, 1, ports.RouteKindLocal), | |||
| 492 | Entries: []ports.RecentRouteEntry{{ | 497 | Entries: []ports.RecentRouteEntry{{ | ||
| 493 | Key: 8, Generation: 4, Target: testRouteTarget("logs", 8), Name: "logs", HostLabel: "edge", Kind: ports.RouteKindRemote, | 498 | Key: 8, Generation: 4, Target: testRouteTarget("logs", 8), Name: "logs", HostLabel: "edge", Kind: ports.RouteKindRemote, | ||
| 494 | }}, | 499 | }}, | ||
| @@ -566,9 +571,10 @@ func TestPaletteJRSDoesNotRevalidateTargetInDaemon(t *testing.T) { | |||||
| 566 | defer release() | 571 | defer release() | ||
| 567 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | 572 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | ||
| 568 | ac.setRouteSnapshot(ports.RecentRouteSnapshot{ | 573 | ac.setRouteSnapshot(ports.RecentRouteSnapshot{ | ||
| 569 | - | Generation: 7, | 574 | + | Generation: 7, |
| 570 | - | Active: ports.RouteRef{Key: 1, Generation: 1}, | 575 | + | Active: ports.RouteRef{Key: 1, Generation: 1}, |
| 571 | - | Entries: []ports.RecentRouteEntry{testRouteEntry(9, 4, "captured", 9, ports.RouteKindLocal)}, | 576 | + | ActiveEntry: testRouteEntry(1, 1, sess.name, 1, ports.RouteKindLocal), |
| 577 | + | Entries: []ports.RecentRouteEntry{testRouteEntry(9, 4, "captured", 9, ports.RouteKindLocal)}, | |||
| 572 | }) | 578 | }) | ||
| 573 | 579 | ||||
| 574 | validated := make(chan struct{}) | 580 | validated := make(chan struct{}) | ||
M internal/usecase/daemon/picker.go +16 -10
| @@ -1203,8 +1203,13 @@ func (d *Daemon) switchToActiveTargetLocked(from *session, ac *attachedClient, t | |||
| 1203 | 1203 | transition, err := d.transitionAttachment(attachmentTransitionRequest{ | |
| 1204 | 1204 | source: from, target: targetSess, next: ac, | |
| 1205 | 1205 | ||
| 1206 | - | expectedTransport: sourceToken.transport, sourceToken: sourceToken, action: action, | |
| 1207 | - | activateTargetTab: true, targetTabIndex: target.TabIndex, preserveAttachment: true, | |
| 1206 | + | expectedTransport: sourceToken.transport, | |
| 1207 | + | sourceToken: sourceToken, | |
| 1208 | + | action: action, | |
| 1209 | + | expectedTargetLifecycle: pickerTargetLifecycleFence(target), | |
| 1210 | + | activateTargetTab: true, | |
| 1211 | + | targetTabIndex: target.TabIndex, | |
| 1212 | + | preserveAttachment: true, | |
| 1208 | 1213 | }) | |
| 1209 | 1214 | d.mu.Lock() | |
| 1210 | 1215 | return targetSess, transition, err == nil | |
| @@ -1244,14 +1249,15 @@ func (d *Daemon) switchToActiveTargetLocked(from *session, ac *attachedClient, t | |||
| 1244 | 1249 | target: targetSess, | |
| 1245 | 1250 | next: ac, | |
| 1246 | 1251 | ||
| 1247 | - | expectedTransport: expectedTransport, | |
| 1248 | - | sourceToken: sourceToken, | |
| 1249 | - | action: action, | |
| 1250 | - | expectedSourceTab: guard.expectedSource, | |
| 1251 | - | activateTargetTab: target.TabIndex >= 0, | |
| 1252 | - | targetTabIndex: target.TabIndex, | |
| 1253 | - | copySourceEnvironment: true, | |
| 1254 | - | ready: true, | |
| 1252 | + | expectedTransport: expectedTransport, | |
| 1253 | + | sourceToken: sourceToken, | |
| 1254 | + | action: action, | |
| 1255 | + | expectedTargetLifecycle: pickerTargetLifecycleFence(target), | |
| 1256 | + | expectedSourceTab: guard.expectedSource, | |
| 1257 | + | activateTargetTab: target.TabIndex >= 0, | |
| 1258 | + | targetTabIndex: target.TabIndex, | |
| 1259 | + | copySourceEnvironment: true, | |
| 1260 | + | ready: true, | |
| 1255 | 1261 | }) | |
| 1256 | 1262 | d.mu.Lock() | |
| 1257 | 1263 | if err != nil { | |
| @@ -1203,8 +1203,13 @@ func (d *Daemon) switchToActiveTargetLocked(from *session, ac *attachedClient, t | |||||
| 1203 | transition, err := d.transitionAttachment(attachmentTransitionRequest{ | 1203 | transition, err := d.transitionAttachment(attachmentTransitionRequest{ | ||
| 1204 | source: from, target: targetSess, next: ac, | 1204 | source: from, target: targetSess, next: ac, | ||
| 1205 | 1205 | ||||
| 1206 | - | expectedTransport: sourceToken.transport, sourceToken: sourceToken, action: action, | 1206 | + | expectedTransport: sourceToken.transport, |
| 1207 | - | activateTargetTab: true, targetTabIndex: target.TabIndex, preserveAttachment: true, | 1207 | + | sourceToken: sourceToken, |
| 1208 | + | action: action, | |||
| 1209 | + | expectedTargetLifecycle: pickerTargetLifecycleFence(target), | |||
| 1210 | + | activateTargetTab: true, | |||
| 1211 | + | targetTabIndex: target.TabIndex, | |||
| 1212 | + | preserveAttachment: true, | |||
| 1208 | }) | 1213 | }) | ||
| 1209 | d.mu.Lock() | 1214 | d.mu.Lock() | ||
| 1210 | return targetSess, transition, err == nil | 1215 | return targetSess, transition, err == nil | ||
| @@ -1244,14 +1249,15 @@ func (d *Daemon) switchToActiveTargetLocked(from *session, ac *attachedClient, t | |||||
| 1244 | target: targetSess, | 1249 | target: targetSess, | ||
| 1245 | next: ac, | 1250 | next: ac, | ||
| 1246 | 1251 | ||||
| 1247 | - | expectedTransport: expectedTransport, | 1252 | + | expectedTransport: expectedTransport, |
| 1248 | - | sourceToken: sourceToken, | 1253 | + | sourceToken: sourceToken, |
| 1249 | - | action: action, | 1254 | + | action: action, |
| 1250 | - | expectedSourceTab: guard.expectedSource, | 1255 | + | expectedTargetLifecycle: pickerTargetLifecycleFence(target), |
| 1251 | - | activateTargetTab: target.TabIndex >= 0, | 1256 | + | expectedSourceTab: guard.expectedSource, |
| 1252 | - | targetTabIndex: target.TabIndex, | 1257 | + | activateTargetTab: target.TabIndex >= 0, |
| 1253 | - | copySourceEnvironment: true, | 1258 | + | targetTabIndex: target.TabIndex, |
| 1254 | - | ready: true, | 1259 | + | copySourceEnvironment: true, |
| 1260 | + | ready: true, | |||
| 1255 | }) | 1261 | }) | ||
| 1256 | d.mu.Lock() | 1262 | d.mu.Lock() | ||
| 1257 | if err != nil { | 1263 | if err != nil { | ||