fix(remote): hide unavailable resume targets
- commit
- 5664af3a81e8e4821d0b5ab148cbec068aaffb28
- author
- brice · · 2026-08-24 17:05 UTC
- parent
- f923d9b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +39 -17
M internal/usecase/daemon/coderabbit_regression_test.go +2 -2
| @@ -352,7 +352,7 @@ func TestRemoteInactiveRouteRejectsReplacementDuringCatalogueRead(t *testing.T) | |||
| 352 | 352 | ||
| 353 | 353 | var protocol *protoErr | |
| 354 | 354 | require.ErrorAs(t, <-result, &protocol) | |
| 355 | - | require.Equal(t, uint16(ports.ErrNoSuchTarget), protocol.code) | |
| 355 | + | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) | |
| 356 | 356 | require.Empty(t, catalogue.MetadataUpdates()) | |
| 357 | 357 | d.mu.Lock() | |
| 358 | 358 | defer d.mu.Unlock() | |
| @@ -445,7 +445,7 @@ func TestRemoteInactiveRouteRejectsTabAuthorityChangeDuringCatalogueRead(t *test | |||
| 445 | 445 | ||
| 446 | 446 | var protocol *protoErr | |
| 447 | 447 | require.ErrorAs(t, <-result, &protocol) | |
| 448 | - | require.Equal(t, uint16(ports.ErrNoSuchTarget), protocol.code) | |
| 448 | + | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) | |
| 449 | 449 | require.Empty(t, catalogue.MetadataUpdates()) | |
| 450 | 450 | d.mu.Lock() | |
| 451 | 451 | defer d.mu.Unlock() | |
| @@ -352,7 +352,7 @@ func TestRemoteInactiveRouteRejectsReplacementDuringCatalogueRead(t *testing.T) | |||||
| 352 | 352 | ||||
| 353 | var protocol *protoErr | 353 | var protocol *protoErr | ||
| 354 | require.ErrorAs(t, <-result, &protocol) | 354 | require.ErrorAs(t, <-result, &protocol) | ||
| 355 | - | require.Equal(t, uint16(ports.ErrNoSuchTarget), protocol.code) | 355 | + | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) |
| 356 | require.Empty(t, catalogue.MetadataUpdates()) | 356 | require.Empty(t, catalogue.MetadataUpdates()) | ||
| 357 | d.mu.Lock() | 357 | d.mu.Lock() | ||
| 358 | defer d.mu.Unlock() | 358 | defer d.mu.Unlock() | ||
| @@ -445,7 +445,7 @@ func TestRemoteInactiveRouteRejectsTabAuthorityChangeDuringCatalogueRead(t *test | |||||
| 445 | 445 | ||||
| 446 | var protocol *protoErr | 446 | var protocol *protoErr | ||
| 447 | require.ErrorAs(t, <-result, &protocol) | 447 | require.ErrorAs(t, <-result, &protocol) | ||
| 448 | - | require.Equal(t, uint16(ports.ErrNoSuchTarget), protocol.code) | 448 | + | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) |
| 449 | require.Empty(t, catalogue.MetadataUpdates()) | 449 | require.Empty(t, catalogue.MetadataUpdates()) | ||
| 450 | d.mu.Lock() | 450 | d.mu.Lock() | ||
| 451 | defer d.mu.Unlock() | 451 | defer d.mu.Unlock() | ||
M internal/usecase/daemon/control_test.go +4 -2
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "context" | |
| 6 | 6 | "encoding/json" | |
| 7 | 7 | "errors" | |
| 8 | + | "fmt" | |
| 8 | 9 | "io" | |
| 9 | 10 | "log/slog" | |
| 10 | 11 | "strings" | |
| @@ -821,10 +822,11 @@ func TestRemoteCatalogRejectsTooManyTabs(t *testing.T) { | |||
| 821 | 822 | d := newTestDaemon(t, nil, stubClock{}) | |
| 822 | 823 | sess := addControlSession(d, "work", "t_work", "p_work") | |
| 823 | 824 | sess.mu.Lock() | |
| 824 | - | base := sess.tabs[0] | |
| 825 | 825 | sess.tabs = make([]*tab, ports.RemoteCatalogMaxTabsPerSess+1) | |
| 826 | 826 | for i := range sess.tabs { | |
| 827 | - | sess.tabs[i] = base | |
| 827 | + | sess.tabs[i] = newTabWithStableID( | |
| 828 | + | fmt.Sprintf("tab-%d", i), fmt.Sprintf("pane-%d", i), newQuietPTY(), domain.Size{Cols: 80, Rows: 22}, | |
| 829 | + | ) | |
| 828 | 830 | } | |
| 829 | 831 | sess.mu.Unlock() | |
| 830 | 832 | ||
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "context" | 5 | "context" | ||
| 6 | "encoding/json" | 6 | "encoding/json" | ||
| 7 | "errors" | 7 | "errors" | ||
| 8 | + | "fmt" | |||
| 8 | "io" | 9 | "io" | ||
| 9 | "log/slog" | 10 | "log/slog" | ||
| 10 | "strings" | 11 | "strings" | ||
| @@ -821,10 +822,11 @@ func TestRemoteCatalogRejectsTooManyTabs(t *testing.T) { | |||||
| 821 | d := newTestDaemon(t, nil, stubClock{}) | 822 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 822 | sess := addControlSession(d, "work", "t_work", "p_work") | 823 | sess := addControlSession(d, "work", "t_work", "p_work") | ||
| 823 | sess.mu.Lock() | 824 | sess.mu.Lock() | ||
| 824 | - | base := sess.tabs[0] | |||
| 825 | sess.tabs = make([]*tab, ports.RemoteCatalogMaxTabsPerSess+1) | 825 | sess.tabs = make([]*tab, ports.RemoteCatalogMaxTabsPerSess+1) | ||
| 826 | for i := range sess.tabs { | 826 | for i := range sess.tabs { | ||
| 827 | - | sess.tabs[i] = base | 827 | + | sess.tabs[i] = newTabWithStableID( |
| 828 | + | fmt.Sprintf("tab-%d", i), fmt.Sprintf("pane-%d", i), newQuietPTY(), domain.Size{Cols: 80, Rows: 22}, | |||
| 829 | + | ) | |||
| 828 | } | 830 | } | ||
| 829 | sess.mu.Unlock() | 831 | sess.mu.Unlock() | ||
| 830 | 832 | ||||
M internal/usecase/daemon/palette.go +3 -2
| @@ -61,8 +61,10 @@ func (d *Daemon) paletteResults(current *session, commands []command.Command, ro | |||
| 61 | 61 | d.mu.Lock() | |
| 62 | 62 | sessions := d.sessionsSnapshotLocked() | |
| 63 | 63 | stopped := make([]inactiveSession, 0, len(d.inactive)) | |
| 64 | + | localLifecycles := make(map[domain.SessionLifecycleID]struct{}, len(sessions)+len(d.inactive)) | |
| 64 | 65 | for _, entry := range d.inactive { | |
| 65 | - | if entry.visible() { | |
| 66 | + | localLifecycles[entry.incarnation] = struct{}{} | |
| 67 | + | if entry.canResume() { | |
| 66 | 68 | stopped = append(stopped, entry) | |
| 67 | 69 | } | |
| 68 | 70 | } | |
| @@ -73,7 +75,6 @@ func (d *Daemon) paletteResults(current *session, commands []command.Command, ro | |||
| 73 | 75 | results = append(results, palette.NewCommandResult(cmd)) | |
| 74 | 76 | } | |
| 75 | 77 | active := make([]palette.Result, 0, len(sessions)) | |
| 76 | - | localLifecycles := make(map[domain.SessionLifecycleID]struct{}, len(sessions)+len(stopped)) | |
| 77 | 78 | for _, candidate := range sessions { | |
| 78 | 79 | snap := candidate.snapshotView(viewOptions{}) | |
| 79 | 80 | if snap.name == "" || snap.ephemeral { | |
| @@ -61,8 +61,10 @@ func (d *Daemon) paletteResults(current *session, commands []command.Command, ro | |||||
| 61 | d.mu.Lock() | 61 | d.mu.Lock() | ||
| 62 | sessions := d.sessionsSnapshotLocked() | 62 | sessions := d.sessionsSnapshotLocked() | ||
| 63 | stopped := make([]inactiveSession, 0, len(d.inactive)) | 63 | stopped := make([]inactiveSession, 0, len(d.inactive)) | ||
| 64 | + | localLifecycles := make(map[domain.SessionLifecycleID]struct{}, len(sessions)+len(d.inactive)) | |||
| 64 | for _, entry := range d.inactive { | 65 | for _, entry := range d.inactive { | ||
| 65 | - | if entry.visible() { | 66 | + | localLifecycles[entry.incarnation] = struct{}{} |
| 67 | + | if entry.canResume() { | |||
| 66 | stopped = append(stopped, entry) | 68 | stopped = append(stopped, entry) | ||
| 67 | } | 69 | } | ||
| 68 | } | 70 | } | ||
| @@ -73,7 +75,6 @@ func (d *Daemon) paletteResults(current *session, commands []command.Command, ro | |||||
| 73 | results = append(results, palette.NewCommandResult(cmd)) | 75 | results = append(results, palette.NewCommandResult(cmd)) | ||
| 74 | } | 76 | } | ||
| 75 | active := make([]palette.Result, 0, len(sessions)) | 77 | active := make([]palette.Result, 0, len(sessions)) | ||
| 76 | - | localLifecycles := make(map[domain.SessionLifecycleID]struct{}, len(sessions)+len(stopped)) | |||
| 77 | for _, candidate := range sessions { | 78 | for _, candidate := range sessions { | ||
| 78 | snap := candidate.snapshotView(viewOptions{}) | 79 | snap := candidate.snapshotView(viewOptions{}) | ||
| 79 | if snap.name == "" || snap.ephemeral { | 80 | if snap.name == "" || snap.ephemeral { | ||
M internal/usecase/daemon/palette_test.go +9 -2
| @@ -228,8 +228,15 @@ func TestPaletteEntryPublishesEligibleNamedSessionResults(t *testing.T) { | |||
| 228 | 228 | d, current, ac, _ := newManualSessionWithPTYs(t, p) | |
| 229 | 229 | d.sessions["active"] = &session{sessionCore: sessionCore{id: "active", name: "active", createdAt: 10}} | |
| 230 | 230 | d.sessions["ephemeral"] = &session{sessionCore: sessionCore{id: "ephemeral", name: "ephemeral", ephemeral: true, createdAt: 11}} | |
| 231 | - | d.inactive["stopped"] = inactiveSession{name: "stopped", createdAt: 12} | |
| 232 | - | d.inactive["purging"] = inactiveSession{name: "purging", createdAt: 13, purging: true} | |
| 231 | + | d.inactive["stopped"] = inactiveSession{name: "stopped", createdAt: 12, incarnation: domain.IncarnationID{3}, state: ports.SessionDown} | |
| 232 | + | d.inactive["purging"] = inactiveSession{name: "purging", createdAt: 13, incarnation: domain.IncarnationID{4}, state: ports.SessionDown, purging: true} | |
| 233 | + | d.inactive["broken"] = inactiveSession{name: "broken", createdAt: 14, incarnation: domain.IncarnationID{5}, state: ports.SessionBroken} | |
| 234 | + | d.inactive["degraded"] = inactiveSession{name: "degraded", createdAt: 15, incarnation: domain.IncarnationID{6}, state: ports.SessionDown, record: domain.CatalogueRecord{DegradedReason: "checkpoint unavailable"}} | |
| 235 | + | ac.setRouteSnapshot(ports.RecentRouteSnapshot{Generation: 1, Entries: []ports.RecentRouteEntry{ | |
| 236 | + | testRouteEntry(1, 1, "purging", 4, ports.RouteKindLocal), | |
| 237 | + | testRouteEntry(2, 1, "broken", 5, ports.RouteKindLocal), | |
| 238 | + | testRouteEntry(3, 1, "degraded", 6, ports.RouteKindLocal), | |
| 239 | + | }}) | |
| 233 | 240 | ||
| 234 | 241 | d.enterPalette(current, ac) | |
| 235 | 242 | ac.overlays.paletteMu.Lock() | |
| @@ -228,8 +228,15 @@ func TestPaletteEntryPublishesEligibleNamedSessionResults(t *testing.T) { | |||||
| 228 | d, current, ac, _ := newManualSessionWithPTYs(t, p) | 228 | d, current, ac, _ := newManualSessionWithPTYs(t, p) | ||
| 229 | d.sessions["active"] = &session{sessionCore: sessionCore{id: "active", name: "active", createdAt: 10}} | 229 | d.sessions["active"] = &session{sessionCore: sessionCore{id: "active", name: "active", createdAt: 10}} | ||
| 230 | d.sessions["ephemeral"] = &session{sessionCore: sessionCore{id: "ephemeral", name: "ephemeral", ephemeral: true, createdAt: 11}} | 230 | d.sessions["ephemeral"] = &session{sessionCore: sessionCore{id: "ephemeral", name: "ephemeral", ephemeral: true, createdAt: 11}} | ||
| 231 | - | d.inactive["stopped"] = inactiveSession{name: "stopped", createdAt: 12} | 231 | + | d.inactive["stopped"] = inactiveSession{name: "stopped", createdAt: 12, incarnation: domain.IncarnationID{3}, state: ports.SessionDown} |
| 232 | - | d.inactive["purging"] = inactiveSession{name: "purging", createdAt: 13, purging: true} | 232 | + | d.inactive["purging"] = inactiveSession{name: "purging", createdAt: 13, incarnation: domain.IncarnationID{4}, state: ports.SessionDown, purging: true} |
| 233 | + | d.inactive["broken"] = inactiveSession{name: "broken", createdAt: 14, incarnation: domain.IncarnationID{5}, state: ports.SessionBroken} | |||
| 234 | + | d.inactive["degraded"] = inactiveSession{name: "degraded", createdAt: 15, incarnation: domain.IncarnationID{6}, state: ports.SessionDown, record: domain.CatalogueRecord{DegradedReason: "checkpoint unavailable"}} | |||
| 235 | + | ac.setRouteSnapshot(ports.RecentRouteSnapshot{Generation: 1, Entries: []ports.RecentRouteEntry{ | |||
| 236 | + | testRouteEntry(1, 1, "purging", 4, ports.RouteKindLocal), | |||
| 237 | + | testRouteEntry(2, 1, "broken", 5, ports.RouteKindLocal), | |||
| 238 | + | testRouteEntry(3, 1, "degraded", 6, ports.RouteKindLocal), | |||
| 239 | + | }}) | |||
| 233 | 240 | ||||
| 234 | d.enterPalette(current, ac) | 241 | d.enterPalette(current, ac) | ||
| 235 | ac.overlays.paletteMu.Lock() | 242 | ac.overlays.paletteMu.Lock() | ||
M internal/usecase/picker/model.go +1 -1
| @@ -364,7 +364,7 @@ func rowsForSession(session SessionView, config SelectionConfig) []row { | |||
| 364 | 364 | } | |
| 365 | 365 | ||
| 366 | 366 | func remoteStoppedOrdinalSelector(index int, rawName string, tabCount int) (domain.TabSelector, bool) { | |
| 367 | - | if tabCount > math.MaxUint16 { | |
| 367 | + | if tabCount > math.MaxUint16 || index < 0 || index >= tabCount { | |
| 368 | 368 | return domain.TabSelector{}, false | |
| 369 | 369 | } | |
| 370 | 370 | return domain.NewOrdinalTabSelector(uint16(index), rawName, uint16(tabCount)), true | |
| @@ -364,7 +364,7 @@ func rowsForSession(session SessionView, config SelectionConfig) []row { | |||||
| 364 | } | 364 | } | ||
| 365 | 365 | ||||
| 366 | func remoteStoppedOrdinalSelector(index int, rawName string, tabCount int) (domain.TabSelector, bool) { | 366 | func remoteStoppedOrdinalSelector(index int, rawName string, tabCount int) (domain.TabSelector, bool) { | ||
| 367 | - | if tabCount > math.MaxUint16 { | 367 | + | if tabCount > math.MaxUint16 || index < 0 || index >= tabCount { |
| 368 | return domain.TabSelector{}, false | 368 | return domain.TabSelector{}, false | ||
| 369 | } | 369 | } | ||
| 370 | return domain.NewOrdinalTabSelector(uint16(index), rawName, uint16(tabCount)), true | 370 | return domain.NewOrdinalTabSelector(uint16(index), rawName, uint16(tabCount)), true | ||
M internal/usecase/picker/model_test.go +20 -8
| @@ -777,14 +777,26 @@ func TestStructuredRemoteTargetOwnsStoppedState(t *testing.T) { | |||
| 777 | 777 | } | |
| 778 | 778 | } | |
| 779 | 779 | ||
| 780 | - | func TestRemoteStoppedOrdinalSelectorRejectsUnrepresentableTabCount(t *testing.T) { | |
| 781 | - | selector, ok := remoteStoppedOrdinalSelector(math.MaxUint16-1, "last", math.MaxUint16) | |
| 782 | - | require.True(t, ok) | |
| 783 | - | require.Equal(t, domain.NewOrdinalTabSelector(math.MaxUint16-1, "last", math.MaxUint16), selector) | |
| 784 | - | ||
| 785 | - | selector, ok = remoteStoppedOrdinalSelector(math.MaxUint16, "wrapped", math.MaxUint16+1) | |
| 786 | - | require.False(t, ok) | |
| 787 | - | require.Equal(t, domain.TabSelector{}, selector) | |
| 780 | + | func TestRemoteStoppedOrdinalSelectorBounds(t *testing.T) { | |
| 781 | + | for _, test := range []struct { | |
| 782 | + | name string | |
| 783 | + | index int | |
| 784 | + | tabCount int | |
| 785 | + | want domain.TabSelector | |
| 786 | + | ok bool | |
| 787 | + | }{ | |
| 788 | + | {name: "largest representable", index: math.MaxUint16 - 1, tabCount: math.MaxUint16, want: domain.NewOrdinalTabSelector(math.MaxUint16-1, "tab", math.MaxUint16), ok: true}, | |
| 789 | + | {name: "unrepresentable count", index: math.MaxUint16, tabCount: math.MaxUint16 + 1}, | |
| 790 | + | {name: "negative index", index: -1, tabCount: 1}, | |
| 791 | + | {name: "index equals count", index: 1, tabCount: 1}, | |
| 792 | + | {name: "empty count", index: 0, tabCount: 0}, | |
| 793 | + | } { | |
| 794 | + | t.Run(test.name, func(t *testing.T) { | |
| 795 | + | selector, ok := remoteStoppedOrdinalSelector(test.index, "tab", test.tabCount) | |
| 796 | + | require.Equal(t, test.ok, ok) | |
| 797 | + | require.Equal(t, test.want, selector) | |
| 798 | + | }) | |
| 799 | + | } | |
| 788 | 800 | } | |
| 789 | 801 | ||
| 790 | 802 | func TestRemoteRowsCannotAcceptMovesInEitherMoveMode(t *testing.T) { | |
| @@ -777,14 +777,26 @@ func TestStructuredRemoteTargetOwnsStoppedState(t *testing.T) { | |||||
| 777 | } | 777 | } | ||
| 778 | } | 778 | } | ||
| 779 | 779 | ||||
| 780 | - | func TestRemoteStoppedOrdinalSelectorRejectsUnrepresentableTabCount(t *testing.T) { | 780 | + | func TestRemoteStoppedOrdinalSelectorBounds(t *testing.T) { |
| 781 | - | selector, ok := remoteStoppedOrdinalSelector(math.MaxUint16-1, "last", math.MaxUint16) | 781 | + | for _, test := range []struct { |
| 782 | - | require.True(t, ok) | 782 | + | name string |
| 783 | - | require.Equal(t, domain.NewOrdinalTabSelector(math.MaxUint16-1, "last", math.MaxUint16), selector) | 783 | + | index int |
| 784 | - | 784 | + | tabCount int | |
| 785 | - | selector, ok = remoteStoppedOrdinalSelector(math.MaxUint16, "wrapped", math.MaxUint16+1) | 785 | + | want domain.TabSelector |
| 786 | - | require.False(t, ok) | 786 | + | ok bool |
| 787 | - | require.Equal(t, domain.TabSelector{}, selector) | 787 | + | }{ |
| 788 | + | {name: "largest representable", index: math.MaxUint16 - 1, tabCount: math.MaxUint16, want: domain.NewOrdinalTabSelector(math.MaxUint16-1, "tab", math.MaxUint16), ok: true}, | |||
| 789 | + | {name: "unrepresentable count", index: math.MaxUint16, tabCount: math.MaxUint16 + 1}, | |||
| 790 | + | {name: "negative index", index: -1, tabCount: 1}, | |||
| 791 | + | {name: "index equals count", index: 1, tabCount: 1}, | |||
| 792 | + | {name: "empty count", index: 0, tabCount: 0}, | |||
| 793 | + | } { | |||
| 794 | + | t.Run(test.name, func(t *testing.T) { | |||
| 795 | + | selector, ok := remoteStoppedOrdinalSelector(test.index, "tab", test.tabCount) | |||
| 796 | + | require.Equal(t, test.ok, ok) | |||
| 797 | + | require.Equal(t, test.want, selector) | |||
| 798 | + | }) | |||
| 799 | + | } | |||
| 788 | } | 800 | } | ||
| 789 | 801 | ||||
| 790 | func TestRemoteRowsCannotAcceptMovesInEitherMoveMode(t *testing.T) { | 802 | func TestRemoteRowsCannotAcceptMovesInEitherMoveMode(t *testing.T) { | ||