fix(remote): harden picker acceptance
- commit
- 8b760082cece7da76c240ecbf4ce3e201b6bfb40
- author
- brice · · 2026-08-06 10:31 UTC
- parent
- 8ecb88d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +279 -111
M internal/usecase/daemon/remote_picker.go +1 -1
| @@ -139,7 +139,7 @@ func remotePickerView(key domain.RemoteSessionKey, session ports.RemoteCatalogSe | |||
| 139 | 139 | viewTabs := make([]picker.TabEntry, 0, len(tabs)) | |
| 140 | 140 | active := 0 | |
| 141 | 141 | for i, tab := range tabs { | |
| 142 | - | if tab.ID == session.ActiveTabID { | |
| 142 | + | if session.ActiveTabID != "" && tab.ID == session.ActiveTabID { | |
| 143 | 143 | active = i | |
| 144 | 144 | } | |
| 145 | 145 | name := tab.Name | |
| @@ -139,7 +139,7 @@ func remotePickerView(key domain.RemoteSessionKey, session ports.RemoteCatalogSe | |||||
| 139 | viewTabs := make([]picker.TabEntry, 0, len(tabs)) | 139 | viewTabs := make([]picker.TabEntry, 0, len(tabs)) | ||
| 140 | active := 0 | 140 | active := 0 | ||
| 141 | for i, tab := range tabs { | 141 | for i, tab := range tabs { | ||
| 142 | - | if tab.ID == session.ActiveTabID { | 142 | + | if session.ActiveTabID != "" && tab.ID == session.ActiveTabID { |
| 143 | active = i | 143 | active = i | ||
| 144 | } | 144 | } | ||
| 145 | name := tab.Name | 145 | name := tab.Name | ||
M internal/usecase/daemon/remote_picker_test.go +63 -24
| @@ -77,35 +77,74 @@ func TestRemotePickerCatalogExpiresAtAttachTTL(t *testing.T) { | |||
| 77 | 77 | } | |
| 78 | 78 | ||
| 79 | 79 | func TestRemotePickerSelectsCatalogActiveTab(t *testing.T) { | |
| 80 | - | d := newRemotePickerDaemon(nil) | |
| 81 | - | lifecycle := remoteLifecycleForTest() | |
| 82 | - | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |
| 83 | - | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | |
| 84 | - | LifecycleID: lifecycle, | |
| 85 | - | Name: "vive", | |
| 86 | - | State: "running", | |
| 87 | - | Tabs: []ports.RemoteCatalogTab{ | |
| 80 | + | tests := []struct { | |
| 81 | + | name string | |
| 82 | + | tabs []ports.RemoteCatalogTab | |
| 83 | + | activeTabID string | |
| 84 | + | wantActive int | |
| 85 | + | wantTabID domain.TabStableID | |
| 86 | + | }{ | |
| 87 | + | { | |
| 88 | + | name: "active first", | |
| 89 | + | tabs: []ports.RemoteCatalogTab{ | |
| 90 | + | {ID: "tab-active", Index: 0, Name: "active"}, | |
| 91 | + | {ID: "tab-last", Index: 1, Name: "last"}, | |
| 92 | + | }, | |
| 93 | + | activeTabID: "tab-active", | |
| 94 | + | wantActive: 0, | |
| 95 | + | wantTabID: "tab-active", | |
| 96 | + | }, | |
| 97 | + | { | |
| 98 | + | name: "active last", | |
| 99 | + | tabs: []ports.RemoteCatalogTab{ | |
| 88 | 100 | {ID: "tab-first", Index: 0, Name: "first"}, | |
| 89 | 101 | {ID: "tab-active", Index: 1, Name: "active"}, | |
| 90 | 102 | }, | |
| 91 | - | ActiveTabID: "tab-active", | |
| 92 | - | }}, | |
| 93 | - | }}) | |
| 94 | - | d.remoteCatalog.mu.Lock() | |
| 95 | - | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 96 | - | d.remoteCatalog.mu.Unlock() | |
| 103 | + | activeTabID: "tab-active", | |
| 104 | + | wantActive: 1, | |
| 105 | + | wantTabID: "tab-active", | |
| 106 | + | }, | |
| 107 | + | { | |
| 108 | + | name: "empty active tab ID falls back to first stable tab", | |
| 109 | + | tabs: []ports.RemoteCatalogTab{ | |
| 110 | + | {ID: "tab-first", Index: 0, Name: "first"}, | |
| 111 | + | {ID: "", Index: 1, Name: "malformed"}, | |
| 112 | + | }, | |
| 113 | + | wantActive: 0, | |
| 114 | + | wantTabID: "tab-first", | |
| 115 | + | }, | |
| 116 | + | } | |
| 97 | 117 | ||
| 98 | - | views, _ := d.pickerViews(nil, nil) | |
| 99 | - | require.Len(t, views, 1) | |
| 100 | - | require.Equal(t, 1, views[0].Active) | |
| 101 | - | require.NotNil(t, views[0].RemoteTarget) | |
| 102 | - | require.Equal(t, domain.TabStableID("tab-active"), views[0].RemoteTarget.LiveTabID) | |
| 118 | + | for _, test := range tests { | |
| 119 | + | t.Run(test.name, func(t *testing.T) { | |
| 120 | + | d := newRemotePickerDaemon(nil) | |
| 121 | + | lifecycle := remoteLifecycleForTest() | |
| 122 | + | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |
| 123 | + | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | |
| 124 | + | LifecycleID: lifecycle, | |
| 125 | + | Name: "vive", | |
| 126 | + | State: "running", | |
| 127 | + | Tabs: test.tabs, | |
| 128 | + | ActiveTabID: test.activeTabID, | |
| 129 | + | }}, | |
| 130 | + | }}) | |
| 131 | + | d.remoteCatalog.mu.Lock() | |
| 132 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 133 | + | d.remoteCatalog.mu.Unlock() | |
| 103 | 134 | ||
| 104 | - | model := d.newPickerModel(nil, nil, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{}) | |
| 105 | - | target, ok := model.Selected() | |
| 106 | - | require.True(t, ok) | |
| 107 | - | require.NotNil(t, target.RemoteTarget) | |
| 108 | - | require.Equal(t, domain.TabStableID("tab-active"), target.RemoteTarget.LiveTabID) | |
| 135 | + | views, _ := d.pickerViews(nil, nil) | |
| 136 | + | require.Len(t, views, 1) | |
| 137 | + | require.Equal(t, test.wantActive, views[0].Active) | |
| 138 | + | require.NotNil(t, views[0].RemoteTarget) | |
| 139 | + | require.Equal(t, test.wantTabID, views[0].RemoteTarget.LiveTabID) | |
| 140 | + | ||
| 141 | + | model := d.newPickerModel(nil, nil, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{}) | |
| 142 | + | target, ok := model.Selected() | |
| 143 | + | require.True(t, ok) | |
| 144 | + | require.NotNil(t, target.RemoteTarget) | |
| 145 | + | require.Equal(t, test.wantTabID, target.RemoteTarget.LiveTabID) | |
| 146 | + | }) | |
| 147 | + | } | |
| 109 | 148 | } | |
| 110 | 149 | ||
| 111 | 150 | func TestRemotePickerAvailabilityUsesCachedRowsForFailures(t *testing.T) { | |
| @@ -77,35 +77,74 @@ func TestRemotePickerCatalogExpiresAtAttachTTL(t *testing.T) { | |||||
| 77 | } | 77 | } | ||
| 78 | 78 | ||||
| 79 | func TestRemotePickerSelectsCatalogActiveTab(t *testing.T) { | 79 | func TestRemotePickerSelectsCatalogActiveTab(t *testing.T) { | ||
| 80 | - | d := newRemotePickerDaemon(nil) | 80 | + | tests := []struct { |
| 81 | - | lifecycle := remoteLifecycleForTest() | 81 | + | name string |
| 82 | - | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | 82 | + | tabs []ports.RemoteCatalogTab |
| 83 | - | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | 83 | + | activeTabID string |
| 84 | - | LifecycleID: lifecycle, | 84 | + | wantActive int |
| 85 | - | Name: "vive", | 85 | + | wantTabID domain.TabStableID |
| 86 | - | State: "running", | 86 | + | }{ |
| 87 | - | Tabs: []ports.RemoteCatalogTab{ | 87 | + | { |
| 88 | + | name: "active first", | |||
| 89 | + | tabs: []ports.RemoteCatalogTab{ | |||
| 90 | + | {ID: "tab-active", Index: 0, Name: "active"}, | |||
| 91 | + | {ID: "tab-last", Index: 1, Name: "last"}, | |||
| 92 | + | }, | |||
| 93 | + | activeTabID: "tab-active", | |||
| 94 | + | wantActive: 0, | |||
| 95 | + | wantTabID: "tab-active", | |||
| 96 | + | }, | |||
| 97 | + | { | |||
| 98 | + | name: "active last", | |||
| 99 | + | tabs: []ports.RemoteCatalogTab{ | |||
| 88 | {ID: "tab-first", Index: 0, Name: "first"}, | 100 | {ID: "tab-first", Index: 0, Name: "first"}, | ||
| 89 | {ID: "tab-active", Index: 1, Name: "active"}, | 101 | {ID: "tab-active", Index: 1, Name: "active"}, | ||
| 90 | }, | 102 | }, | ||
| 91 | - | ActiveTabID: "tab-active", | 103 | + | activeTabID: "tab-active", |
| 92 | - | }}, | 104 | + | wantActive: 1, |
| 93 | - | }}) | 105 | + | wantTabID: "tab-active", |
| 94 | - | d.remoteCatalog.mu.Lock() | 106 | + | }, |
| 95 | - | d.remoteCatalog.status["arch"] = remoteHostFresh | 107 | + | { |
| 96 | - | d.remoteCatalog.mu.Unlock() | 108 | + | name: "empty active tab ID falls back to first stable tab", |
| 109 | + | tabs: []ports.RemoteCatalogTab{ | |||
| 110 | + | {ID: "tab-first", Index: 0, Name: "first"}, | |||
| 111 | + | {ID: "", Index: 1, Name: "malformed"}, | |||
| 112 | + | }, | |||
| 113 | + | wantActive: 0, | |||
| 114 | + | wantTabID: "tab-first", | |||
| 115 | + | }, | |||
| 116 | + | } | |||
| 97 | 117 | ||||
| 98 | - | views, _ := d.pickerViews(nil, nil) | 118 | + | for _, test := range tests { |
| 99 | - | require.Len(t, views, 1) | 119 | + | t.Run(test.name, func(t *testing.T) { |
| 100 | - | require.Equal(t, 1, views[0].Active) | 120 | + | d := newRemotePickerDaemon(nil) |
| 101 | - | require.NotNil(t, views[0].RemoteTarget) | 121 | + | lifecycle := remoteLifecycleForTest() |
| 102 | - | require.Equal(t, domain.TabStableID("tab-active"), views[0].RemoteTarget.LiveTabID) | 122 | + | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ |
| 123 | + | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | |||
| 124 | + | LifecycleID: lifecycle, | |||
| 125 | + | Name: "vive", | |||
| 126 | + | State: "running", | |||
| 127 | + | Tabs: test.tabs, | |||
| 128 | + | ActiveTabID: test.activeTabID, | |||
| 129 | + | }}, | |||
| 130 | + | }}) | |||
| 131 | + | d.remoteCatalog.mu.Lock() | |||
| 132 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |||
| 133 | + | d.remoteCatalog.mu.Unlock() | |||
| 103 | 134 | ||||
| 104 | - | model := d.newPickerModel(nil, nil, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{}) | 135 | + | views, _ := d.pickerViews(nil, nil) |
| 105 | - | target, ok := model.Selected() | 136 | + | require.Len(t, views, 1) |
| 106 | - | require.True(t, ok) | 137 | + | require.Equal(t, test.wantActive, views[0].Active) |
| 107 | - | require.NotNil(t, target.RemoteTarget) | 138 | + | require.NotNil(t, views[0].RemoteTarget) |
| 108 | - | require.Equal(t, domain.TabStableID("tab-active"), target.RemoteTarget.LiveTabID) | 139 | + | require.Equal(t, test.wantTabID, views[0].RemoteTarget.LiveTabID) |
| 140 | + | ||||
| 141 | + | model := d.newPickerModel(nil, nil, pickerNavigate, moveSourceLocator{}, picker.SourceFilter{}) | |||
| 142 | + | target, ok := model.Selected() | |||
| 143 | + | require.True(t, ok) | |||
| 144 | + | require.NotNil(t, target.RemoteTarget) | |||
| 145 | + | require.Equal(t, test.wantTabID, target.RemoteTarget.LiveTabID) | |||
| 146 | + | }) | |||
| 147 | + | } | |||
| 109 | } | 148 | } | ||
| 110 | 149 | ||||
| 111 | func TestRemotePickerAvailabilityUsesCachedRowsForFailures(t *testing.T) { | 150 | func TestRemotePickerAvailabilityUsesCachedRowsForFailures(t *testing.T) { | ||
M internal/usecase/daemon/remote_preview_capture_test.go +85 -69
| @@ -35,80 +35,96 @@ func TestCaptureRemotePreviewUsesBottomRowsForShorterPreview(t *testing.T) { | |||
| 35 | 35 | require.Equal(t, []rune{'b', 'c'}, []rune{preview.Cells[0].Rune, preview.Cells[1].Rune}) | |
| 36 | 36 | } | |
| 37 | 37 | ||
| 38 | - | func TestCaptureRemotePreviewCompactsBlankTailToKeepShortScreenContentVisible(t *testing.T) { | |
| 39 | - | d := newTestDaemon(t, nil, stubClock{}) | |
| 40 | - | sess := addControlSession(d, "work", "tab-1", "pane-1") | |
| 41 | - | sess.ephemeral = false | |
| 42 | - | sess.incarnation = remoteLifecycleForTest() | |
| 43 | - | pane := sess.tabs[0].focusedPane() | |
| 44 | - | pane.mu.Lock() | |
| 45 | - | pane.screen = vt.NewScreen(2, 4) | |
| 46 | - | pane.screen.Frame.Set(0, 0, renderer.Cell{Rune: 'v'}) | |
| 47 | - | pane.mu.Unlock() | |
| 48 | - | ||
| 49 | - | target := domain.RemoteSessionTarget{ | |
| 50 | - | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, | |
| 51 | - | SessionName: "work", LiveTabID: "tab-1", | |
| 38 | + | func TestCaptureRemotePreviewCompactsBlankTail(t *testing.T) { | |
| 39 | + | type screenCell struct { | |
| 40 | + | x, y int | |
| 41 | + | cell renderer.Cell | |
| 52 | 42 | } | |
| 53 | - | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | |
| 54 | - | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: 2, Height: 2, | |
| 55 | - | }) | |
| 56 | - | require.NoError(t, err) | |
| 57 | - | require.Equal(t, uint16(1), preview.Height) | |
| 58 | - | require.Equal(t, []rune{'v', ' '}, []rune{preview.Cells[0].Rune, preview.Cells[1].Rune}) | |
| 59 | - | } | |
| 60 | - | ||
| 61 | - | func TestCaptureRemotePreviewIgnoresBoundarySplitWideRuneWhenCompacting(t *testing.T) { | |
| 62 | - | d := newTestDaemon(t, nil, stubClock{}) | |
| 63 | - | sess := addControlSession(d, "work", "tab-1", "pane-1") | |
| 64 | - | sess.ephemeral = false | |
| 65 | - | sess.incarnation = remoteLifecycleForTest() | |
| 66 | - | pane := sess.tabs[0].focusedPane() | |
| 67 | - | pane.mu.Lock() | |
| 68 | - | pane.screen = vt.NewScreen(3, 3) | |
| 69 | - | pane.screen.Frame.Set(0, 1, renderer.Cell{Rune: 'a'}) | |
| 70 | - | pane.screen.Frame.Set(1, 2, renderer.Cell{Rune: '界'}) | |
| 71 | - | pane.screen.Frame.Set(2, 2, renderer.Cell{Continuation: true}) | |
| 72 | - | pane.mu.Unlock() | |
| 73 | - | ||
| 74 | - | target := domain.RemoteSessionTarget{ | |
| 75 | - | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, | |
| 76 | - | SessionName: "work", LiveTabID: "tab-1", | |
| 43 | + | type previewCell struct { | |
| 44 | + | index int | |
| 45 | + | rune rune | |
| 46 | + | style *renderer.Style | |
| 47 | + | } | |
| 48 | + | styledBlank := renderer.DefaultStyle() | |
| 49 | + | styledBlank.Background = 4 | |
| 50 | + | tests := []struct { | |
| 51 | + | name string | |
| 52 | + | screenWidth, screenHeight int | |
| 53 | + | cells []screenCell | |
| 54 | + | width, height uint16 | |
| 55 | + | wantHeight uint16 | |
| 56 | + | wantCells []previewCell | |
| 57 | + | }{ | |
| 58 | + | { | |
| 59 | + | name: "keeps short screen content visible", | |
| 60 | + | screenWidth: 2, | |
| 61 | + | screenHeight: 4, | |
| 62 | + | cells: []screenCell{{x: 0, y: 0, cell: renderer.Cell{Rune: 'v'}}}, | |
| 63 | + | width: 2, | |
| 64 | + | height: 2, | |
| 65 | + | wantHeight: 1, | |
| 66 | + | wantCells: []previewCell{{index: 0, rune: 'v'}, {index: 1, rune: ' '}}, | |
| 67 | + | }, | |
| 68 | + | { | |
| 69 | + | name: "ignores boundary split wide rune", | |
| 70 | + | screenWidth: 3, | |
| 71 | + | screenHeight: 3, | |
| 72 | + | cells: []screenCell{ | |
| 73 | + | {x: 0, y: 1, cell: renderer.Cell{Rune: 'a'}}, | |
| 74 | + | {x: 1, y: 2, cell: renderer.Cell{Rune: '界'}}, | |
| 75 | + | {x: 2, y: 2, cell: renderer.Cell{Continuation: true}}, | |
| 76 | + | }, | |
| 77 | + | width: 2, | |
| 78 | + | height: 2, | |
| 79 | + | wantHeight: 1, | |
| 80 | + | wantCells: []previewCell{{index: 0, rune: 'a'}, {index: 1, rune: ' '}}, | |
| 81 | + | }, | |
| 82 | + | { | |
| 83 | + | name: "preserves styled blank tail", | |
| 84 | + | screenWidth: 2, | |
| 85 | + | screenHeight: 4, | |
| 86 | + | cells: []screenCell{ | |
| 87 | + | {x: 0, y: 2, cell: renderer.Cell{Rune: 'a'}}, | |
| 88 | + | {x: 0, y: 3, cell: renderer.Cell{Rune: ' ', Style: styledBlank}}, | |
| 89 | + | }, | |
| 90 | + | width: 2, | |
| 91 | + | height: 2, | |
| 92 | + | wantHeight: 2, | |
| 93 | + | wantCells: []previewCell{{index: 0, rune: 'a'}, {index: 2, rune: ' ', style: &styledBlank}}, | |
| 94 | + | }, | |
| 77 | 95 | } | |
| 78 | - | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | |
| 79 | - | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: 2, Height: 2, | |
| 80 | - | }) | |
| 81 | - | require.NoError(t, err) | |
| 82 | - | require.Equal(t, uint16(1), preview.Height) | |
| 83 | - | require.Equal(t, []rune{'a', ' '}, []rune{preview.Cells[0].Rune, preview.Cells[1].Rune}) | |
| 84 | - | } | |
| 85 | 96 | ||
| 86 | - | func TestCaptureRemotePreviewPreservesStyledBlankTail(t *testing.T) { | |
| 87 | - | d := newTestDaemon(t, nil, stubClock{}) | |
| 88 | - | sess := addControlSession(d, "work", "tab-1", "pane-1") | |
| 89 | - | sess.ephemeral = false | |
| 90 | - | sess.incarnation = remoteLifecycleForTest() | |
| 91 | - | pane := sess.tabs[0].focusedPane() | |
| 92 | - | style := renderer.DefaultStyle() | |
| 93 | - | style.Background = 4 | |
| 94 | - | pane.mu.Lock() | |
| 95 | - | pane.screen = vt.NewScreen(2, 4) | |
| 96 | - | pane.screen.Frame.Set(0, 2, renderer.Cell{Rune: 'a'}) | |
| 97 | - | pane.screen.Frame.Set(0, 3, renderer.Cell{Rune: ' ', Style: style}) | |
| 98 | - | pane.mu.Unlock() | |
| 97 | + | for _, test := range tests { | |
| 98 | + | t.Run(test.name, func(t *testing.T) { | |
| 99 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 100 | + | sess := addControlSession(d, "work", "tab-1", "pane-1") | |
| 101 | + | sess.ephemeral = false | |
| 102 | + | sess.incarnation = remoteLifecycleForTest() | |
| 103 | + | pane := sess.tabs[0].focusedPane() | |
| 104 | + | pane.mu.Lock() | |
| 105 | + | pane.screen = vt.NewScreen(test.screenWidth, test.screenHeight) | |
| 106 | + | for _, cell := range test.cells { | |
| 107 | + | pane.screen.Frame.Set(cell.x, cell.y, cell.cell) | |
| 108 | + | } | |
| 109 | + | pane.mu.Unlock() | |
| 99 | 110 | ||
| 100 | - | target := domain.RemoteSessionTarget{ | |
| 101 | - | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, | |
| 102 | - | SessionName: "work", LiveTabID: "tab-1", | |
| 111 | + | target := domain.RemoteSessionTarget{ | |
| 112 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, | |
| 113 | + | SessionName: "work", LiveTabID: "tab-1", | |
| 114 | + | } | |
| 115 | + | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | |
| 116 | + | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: test.width, Height: test.height, | |
| 117 | + | }) | |
| 118 | + | require.NoError(t, err) | |
| 119 | + | require.Equal(t, test.wantHeight, preview.Height) | |
| 120 | + | for _, want := range test.wantCells { | |
| 121 | + | require.Equal(t, want.rune, preview.Cells[want.index].Rune) | |
| 122 | + | if want.style != nil { | |
| 123 | + | require.True(t, preview.Cells[want.index].Style.Equal(*want.style)) | |
| 124 | + | } | |
| 125 | + | } | |
| 126 | + | }) | |
| 103 | 127 | } | |
| 104 | - | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | |
| 105 | - | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: 2, Height: 2, | |
| 106 | - | }) | |
| 107 | - | require.NoError(t, err) | |
| 108 | - | require.Equal(t, uint16(2), preview.Height) | |
| 109 | - | require.Equal(t, rune('a'), preview.Cells[0].Rune) | |
| 110 | - | require.Equal(t, rune(' '), preview.Cells[2].Rune) | |
| 111 | - | require.True(t, preview.Cells[2].Style.Equal(style)) | |
| 112 | 128 | } | |
| 113 | 129 | ||
| 114 | 130 | func TestStaticRemotePickerPreviewUsesOnlyItsTextRow(t *testing.T) { | |
| @@ -35,80 +35,96 @@ func TestCaptureRemotePreviewUsesBottomRowsForShorterPreview(t *testing.T) { | |||||
| 35 | require.Equal(t, []rune{'b', 'c'}, []rune{preview.Cells[0].Rune, preview.Cells[1].Rune}) | 35 | require.Equal(t, []rune{'b', 'c'}, []rune{preview.Cells[0].Rune, preview.Cells[1].Rune}) | ||
| 36 | } | 36 | } | ||
| 37 | 37 | ||||
| 38 | - | func TestCaptureRemotePreviewCompactsBlankTailToKeepShortScreenContentVisible(t *testing.T) { | 38 | + | func TestCaptureRemotePreviewCompactsBlankTail(t *testing.T) { |
| 39 | - | d := newTestDaemon(t, nil, stubClock{}) | 39 | + | type screenCell struct { |
| 40 | - | sess := addControlSession(d, "work", "tab-1", "pane-1") | 40 | + | x, y int |
| 41 | - | sess.ephemeral = false | 41 | + | cell renderer.Cell |
| 42 | - | sess.incarnation = remoteLifecycleForTest() | |||
| 43 | - | pane := sess.tabs[0].focusedPane() | |||
| 44 | - | pane.mu.Lock() | |||
| 45 | - | pane.screen = vt.NewScreen(2, 4) | |||
| 46 | - | pane.screen.Frame.Set(0, 0, renderer.Cell{Rune: 'v'}) | |||
| 47 | - | pane.mu.Unlock() | |||
| 48 | - | ||||
| 49 | - | target := domain.RemoteSessionTarget{ | |||
| 50 | - | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, | |||
| 51 | - | SessionName: "work", LiveTabID: "tab-1", | |||
| 52 | } | 42 | } | ||
| 53 | - | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | 43 | + | type previewCell struct { |
| 54 | - | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: 2, Height: 2, | 44 | + | index int |
| 55 | - | }) | 45 | + | rune rune |
| 56 | - | require.NoError(t, err) | 46 | + | style *renderer.Style |
| 57 | - | require.Equal(t, uint16(1), preview.Height) | 47 | + | } |
| 58 | - | require.Equal(t, []rune{'v', ' '}, []rune{preview.Cells[0].Rune, preview.Cells[1].Rune}) | 48 | + | styledBlank := renderer.DefaultStyle() |
| 59 | - | } | 49 | + | styledBlank.Background = 4 |
| 60 | - | 50 | + | tests := []struct { | |
| 61 | - | func TestCaptureRemotePreviewIgnoresBoundarySplitWideRuneWhenCompacting(t *testing.T) { | 51 | + | name string |
| 62 | - | d := newTestDaemon(t, nil, stubClock{}) | 52 | + | screenWidth, screenHeight int |
| 63 | - | sess := addControlSession(d, "work", "tab-1", "pane-1") | 53 | + | cells []screenCell |
| 64 | - | sess.ephemeral = false | 54 | + | width, height uint16 |
| 65 | - | sess.incarnation = remoteLifecycleForTest() | 55 | + | wantHeight uint16 |
| 66 | - | pane := sess.tabs[0].focusedPane() | 56 | + | wantCells []previewCell |
| 67 | - | pane.mu.Lock() | 57 | + | }{ |
| 68 | - | pane.screen = vt.NewScreen(3, 3) | 58 | + | { |
| 69 | - | pane.screen.Frame.Set(0, 1, renderer.Cell{Rune: 'a'}) | 59 | + | name: "keeps short screen content visible", |
| 70 | - | pane.screen.Frame.Set(1, 2, renderer.Cell{Rune: '界'}) | 60 | + | screenWidth: 2, |
| 71 | - | pane.screen.Frame.Set(2, 2, renderer.Cell{Continuation: true}) | 61 | + | screenHeight: 4, |
| 72 | - | pane.mu.Unlock() | 62 | + | cells: []screenCell{{x: 0, y: 0, cell: renderer.Cell{Rune: 'v'}}}, |
| 73 | - | 63 | + | width: 2, | |
| 74 | - | target := domain.RemoteSessionTarget{ | 64 | + | height: 2, |
| 75 | - | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, | 65 | + | wantHeight: 1, |
| 76 | - | SessionName: "work", LiveTabID: "tab-1", | 66 | + | wantCells: []previewCell{{index: 0, rune: 'v'}, {index: 1, rune: ' '}}, |
| 67 | + | }, | |||
| 68 | + | { | |||
| 69 | + | name: "ignores boundary split wide rune", | |||
| 70 | + | screenWidth: 3, | |||
| 71 | + | screenHeight: 3, | |||
| 72 | + | cells: []screenCell{ | |||
| 73 | + | {x: 0, y: 1, cell: renderer.Cell{Rune: 'a'}}, | |||
| 74 | + | {x: 1, y: 2, cell: renderer.Cell{Rune: '界'}}, | |||
| 75 | + | {x: 2, y: 2, cell: renderer.Cell{Continuation: true}}, | |||
| 76 | + | }, | |||
| 77 | + | width: 2, | |||
| 78 | + | height: 2, | |||
| 79 | + | wantHeight: 1, | |||
| 80 | + | wantCells: []previewCell{{index: 0, rune: 'a'}, {index: 1, rune: ' '}}, | |||
| 81 | + | }, | |||
| 82 | + | { | |||
| 83 | + | name: "preserves styled blank tail", | |||
| 84 | + | screenWidth: 2, | |||
| 85 | + | screenHeight: 4, | |||
| 86 | + | cells: []screenCell{ | |||
| 87 | + | {x: 0, y: 2, cell: renderer.Cell{Rune: 'a'}}, | |||
| 88 | + | {x: 0, y: 3, cell: renderer.Cell{Rune: ' ', Style: styledBlank}}, | |||
| 89 | + | }, | |||
| 90 | + | width: 2, | |||
| 91 | + | height: 2, | |||
| 92 | + | wantHeight: 2, | |||
| 93 | + | wantCells: []previewCell{{index: 0, rune: 'a'}, {index: 2, rune: ' ', style: &styledBlank}}, | |||
| 94 | + | }, | |||
| 77 | } | 95 | } | ||
| 78 | - | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | |||
| 79 | - | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: 2, Height: 2, | |||
| 80 | - | }) | |||
| 81 | - | require.NoError(t, err) | |||
| 82 | - | require.Equal(t, uint16(1), preview.Height) | |||
| 83 | - | require.Equal(t, []rune{'a', ' '}, []rune{preview.Cells[0].Rune, preview.Cells[1].Rune}) | |||
| 84 | - | } | |||
| 85 | 96 | ||||
| 86 | - | func TestCaptureRemotePreviewPreservesStyledBlankTail(t *testing.T) { | 97 | + | for _, test := range tests { |
| 87 | - | d := newTestDaemon(t, nil, stubClock{}) | 98 | + | t.Run(test.name, func(t *testing.T) { |
| 88 | - | sess := addControlSession(d, "work", "tab-1", "pane-1") | 99 | + | d := newTestDaemon(t, nil, stubClock{}) |
| 89 | - | sess.ephemeral = false | 100 | + | sess := addControlSession(d, "work", "tab-1", "pane-1") |
| 90 | - | sess.incarnation = remoteLifecycleForTest() | 101 | + | sess.ephemeral = false |
| 91 | - | pane := sess.tabs[0].focusedPane() | 102 | + | sess.incarnation = remoteLifecycleForTest() |
| 92 | - | style := renderer.DefaultStyle() | 103 | + | pane := sess.tabs[0].focusedPane() |
| 93 | - | style.Background = 4 | 104 | + | pane.mu.Lock() |
| 94 | - | pane.mu.Lock() | 105 | + | pane.screen = vt.NewScreen(test.screenWidth, test.screenHeight) |
| 95 | - | pane.screen = vt.NewScreen(2, 4) | 106 | + | for _, cell := range test.cells { |
| 96 | - | pane.screen.Frame.Set(0, 2, renderer.Cell{Rune: 'a'}) | 107 | + | pane.screen.Frame.Set(cell.x, cell.y, cell.cell) |
| 97 | - | pane.screen.Frame.Set(0, 3, renderer.Cell{Rune: ' ', Style: style}) | 108 | + | } |
| 98 | - | pane.mu.Unlock() | 109 | + | pane.mu.Unlock() |
| 99 | 110 | ||||
| 100 | - | target := domain.RemoteSessionTarget{ | 111 | + | target := domain.RemoteSessionTarget{ |
| 101 | - | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, | 112 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, |
| 102 | - | SessionName: "work", LiveTabID: "tab-1", | 113 | + | SessionName: "work", LiveTabID: "tab-1", |
| 114 | + | } | |||
| 115 | + | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | |||
| 116 | + | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: test.width, Height: test.height, | |||
| 117 | + | }) | |||
| 118 | + | require.NoError(t, err) | |||
| 119 | + | require.Equal(t, test.wantHeight, preview.Height) | |||
| 120 | + | for _, want := range test.wantCells { | |||
| 121 | + | require.Equal(t, want.rune, preview.Cells[want.index].Rune) | |||
| 122 | + | if want.style != nil { | |||
| 123 | + | require.True(t, preview.Cells[want.index].Style.Equal(*want.style)) | |||
| 124 | + | } | |||
| 125 | + | } | |||
| 126 | + | }) | |||
| 103 | } | 127 | } | ||
| 104 | - | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | |||
| 105 | - | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: 2, Height: 2, | |||
| 106 | - | }) | |||
| 107 | - | require.NoError(t, err) | |||
| 108 | - | require.Equal(t, uint16(2), preview.Height) | |||
| 109 | - | require.Equal(t, rune('a'), preview.Cells[0].Rune) | |||
| 110 | - | require.Equal(t, rune(' '), preview.Cells[2].Rune) | |||
| 111 | - | require.True(t, preview.Cells[2].Style.Equal(style)) | |||
| 112 | } | 128 | } | ||
| 113 | 129 | ||||
| 114 | func TestStaticRemotePickerPreviewUsesOnlyItsTextRow(t *testing.T) { | 130 | func TestStaticRemotePickerPreviewUsesOnlyItsTextRow(t *testing.T) { | ||
M internal/usecase/daemon/status_test.go +20 -7
| @@ -110,15 +110,28 @@ func TestStatusCompositionGolden(t *testing.T) { | |||
| 110 | 110 | } | |
| 111 | 111 | ||
| 112 | 112 | func TestStatusSegmentsIncludesAttachmentRemoteOrigin(t *testing.T) { | |
| 113 | - | p, releasePTY := newBlockingPTY(t) | |
| 114 | - | _, sess, ac, _ := newManualSessionWithPTYs(t, p) | |
| 115 | - | defer releasePTY() | |
| 113 | + | tests := []struct { | |
| 114 | + | name string | |
| 115 | + | remoteOrigin string | |
| 116 | + | wantAttachmentBar string | |
| 117 | + | }{ | |
| 118 | + | {name: "remote origin", remoteOrigin: "arch", wantAttachmentBar: "vive at arch"}, | |
| 119 | + | {name: "local attachment", wantAttachmentBar: "vive"}, | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | for _, test := range tests { | |
| 123 | + | t.Run(test.name, func(t *testing.T) { | |
| 124 | + | p, releasePTY := newBlockingPTY(t) | |
| 125 | + | _, sess, ac, _ := newManualSessionWithPTYs(t, p) | |
| 126 | + | defer releasePTY() | |
| 116 | 127 | ||
| 117 | - | sess.name = "vive" | |
| 118 | - | ac.remoteOrigin = "arch" | |
| 128 | + | sess.name = "vive" | |
| 129 | + | ac.remoteOrigin = test.remoteOrigin | |
| 119 | 130 | ||
| 120 | - | require.Equal(t, "vive at arch", sess.statusSegmentsFor(ac, true).session) | |
| 121 | - | require.Equal(t, "vive", sess.statusSegments(true).session, "remote provenance belongs only to the selected attachment") | |
| 131 | + | require.Equal(t, test.wantAttachmentBar, sess.statusSegmentsFor(ac, true).session) | |
| 132 | + | require.Equal(t, "vive", sess.statusSegments(true).session, "remote provenance belongs only to the selected attachment") | |
| 133 | + | }) | |
| 134 | + | } | |
| 122 | 135 | } | |
| 123 | 136 | ||
| 124 | 137 | func TestStatusSegmentsIncludesFocusedPaneTitle(t *testing.T) { | |
| @@ -110,15 +110,28 @@ func TestStatusCompositionGolden(t *testing.T) { | |||||
| 110 | } | 110 | } | ||
| 111 | 111 | ||||
| 112 | func TestStatusSegmentsIncludesAttachmentRemoteOrigin(t *testing.T) { | 112 | func TestStatusSegmentsIncludesAttachmentRemoteOrigin(t *testing.T) { | ||
| 113 | - | p, releasePTY := newBlockingPTY(t) | 113 | + | tests := []struct { |
| 114 | - | _, sess, ac, _ := newManualSessionWithPTYs(t, p) | 114 | + | name string |
| 115 | - | defer releasePTY() | 115 | + | remoteOrigin string |
| 116 | + | wantAttachmentBar string | |||
| 117 | + | }{ | |||
| 118 | + | {name: "remote origin", remoteOrigin: "arch", wantAttachmentBar: "vive at arch"}, | |||
| 119 | + | {name: "local attachment", wantAttachmentBar: "vive"}, | |||
| 120 | + | } | |||
| 121 | + | ||||
| 122 | + | for _, test := range tests { | |||
| 123 | + | t.Run(test.name, func(t *testing.T) { | |||
| 124 | + | p, releasePTY := newBlockingPTY(t) | |||
| 125 | + | _, sess, ac, _ := newManualSessionWithPTYs(t, p) | |||
| 126 | + | defer releasePTY() | |||
| 116 | 127 | ||||
| 117 | - | sess.name = "vive" | 128 | + | sess.name = "vive" |
| 118 | - | ac.remoteOrigin = "arch" | 129 | + | ac.remoteOrigin = test.remoteOrigin |
| 119 | 130 | ||||
| 120 | - | require.Equal(t, "vive at arch", sess.statusSegmentsFor(ac, true).session) | 131 | + | require.Equal(t, test.wantAttachmentBar, sess.statusSegmentsFor(ac, true).session) |
| 121 | - | require.Equal(t, "vive", sess.statusSegments(true).session, "remote provenance belongs only to the selected attachment") | 132 | + | require.Equal(t, "vive", sess.statusSegments(true).session, "remote provenance belongs only to the selected attachment") |
| 133 | + | }) | |||
| 134 | + | } | |||
| 122 | } | 135 | } | ||
| 123 | 136 | ||||
| 124 | func TestStatusSegmentsIncludesFocusedPaneTitle(t *testing.T) { | 137 | func TestStatusSegmentsIncludesFocusedPaneTitle(t *testing.T) { | ||
M scripts/remote-picker-harness/main.go +66 -10
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package main | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "bytes" | |
| 4 | 5 | "context" | |
| 5 | 6 | "errors" | |
| 6 | 7 | "fmt" | |
| @@ -140,7 +141,7 @@ func runLiveStdioPhase(ctx context.Context, target string, catalog ports.RemoteC | |||
| 140 | 141 | // daemon container. It validates the seam that individual catalog, preview, | |
| 141 | 142 | // and attach tests cannot: catalog active-tab selection, live remote preview, | |
| 142 | 143 | // emitted rich handoff, and the remote attachment's visible provenance. | |
| 143 | - | func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, preview ports.RemotePreviewClient) error { | |
| 144 | + | func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, preview ports.RemotePreviewClient) (err error) { | |
| 144 | 145 | const ( | |
| 145 | 146 | localSession = "local-picker" | |
| 146 | 147 | remoteSession = "picker" | |
| @@ -151,13 +152,25 @@ func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports | |||
| 151 | 152 | if err != nil { | |
| 152 | 153 | return fmt.Errorf("open remote picker source: %w", err) | |
| 153 | 154 | } | |
| 154 | - | defer func() { _ = remote.Close() }() | |
| 155 | + | ownsRemoteSession := false | |
| 156 | + | defer func() { | |
| 157 | + | _ = remote.Close() | |
| 158 | + | if !ownsRemoteSession { | |
| 159 | + | return | |
| 160 | + | } | |
| 161 | + | cleanup, cancel := context.WithTimeout(context.Background(), 5*time.Second) | |
| 162 | + | defer cancel() | |
| 163 | + | if cleanupErr := runRemoteCommand(cleanup, target, "vev", "kill", remoteSession); cleanupErr != nil { | |
| 164 | + | err = errors.Join(err, fmt.Errorf("destroy remote picker session: %w", cleanupErr)) | |
| 165 | + | } | |
| 166 | + | }() | |
| 155 | 167 | if err := attachAndCheck(ctx, remote, ports.Hello{ | |
| 156 | 168 | Intent: ports.IntentNew, Name: remoteSession, Env: localEnvironment(), Cwd: "/tmp/remote-cwd", | |
| 157 | 169 | TermEnv: "xterm-256color", TrueColor: true, | |
| 158 | 170 | }); err != nil { | |
| 159 | 171 | return fmt.Errorf("attach remote picker source: %w", err) | |
| 160 | 172 | } | |
| 173 | + | ownsRemoteSession = true | |
| 161 | 174 | if err := sendCommand(ctx, remote, "new-tab"); err != nil { | |
| 162 | 175 | return fmt.Errorf("create remote picker tab: %w", err) | |
| 163 | 176 | } | |
| @@ -474,10 +487,48 @@ func openTransport(ctx context.Context, mode ports.RemoteTransportMode, target s | |||
| 474 | 487 | return newHarnessTransport(transport), nil | |
| 475 | 488 | } | |
| 476 | 489 | ||
| 490 | + | const commandStderrLimit = 4 << 10 | |
| 491 | + | ||
| 492 | + | type boundedStderr struct { | |
| 493 | + | bytes.Buffer | |
| 494 | + | truncated bool | |
| 495 | + | } | |
| 496 | + | ||
| 497 | + | func (b *boundedStderr) Write(p []byte) (int, error) { | |
| 498 | + | written := len(p) | |
| 499 | + | remaining := commandStderrLimit - b.Len() | |
| 500 | + | if remaining <= 0 { | |
| 501 | + | b.truncated = b.truncated || written != 0 | |
| 502 | + | return written, nil | |
| 503 | + | } | |
| 504 | + | if len(p) > remaining { | |
| 505 | + | p = p[:remaining] | |
| 506 | + | b.truncated = true | |
| 507 | + | } | |
| 508 | + | _, _ = b.Buffer.Write(p) | |
| 509 | + | return written, nil | |
| 510 | + | } | |
| 511 | + | ||
| 512 | + | func (b *boundedStderr) String() string { | |
| 513 | + | stderr := b.Buffer.String() | |
| 514 | + | if b.truncated { | |
| 515 | + | stderr += "\n[stderr truncated]" | |
| 516 | + | } | |
| 517 | + | return stderr | |
| 518 | + | } | |
| 519 | + | ||
| 520 | + | func commandErrorWithStderr(err error, stderr string) error { | |
| 521 | + | if stderr = strings.TrimSpace(stderr); stderr == "" { | |
| 522 | + | return err | |
| 523 | + | } | |
| 524 | + | return fmt.Errorf("%w: stderr: %s", err, stderr) | |
| 525 | + | } | |
| 526 | + | ||
| 477 | 527 | func startLocalDaemon(ctx context.Context) (*exec.Cmd, error) { | |
| 478 | 528 | cmd := exec.CommandContext(ctx, "vev", "--daemon") | |
| 529 | + | stderr := &boundedStderr{} | |
| 479 | 530 | cmd.Stdout = io.Discard | |
| 480 | - | cmd.Stderr = io.Discard | |
| 531 | + | cmd.Stderr = stderr | |
| 481 | 532 | if err := cmd.Start(); err != nil { | |
| 482 | 533 | return nil, err | |
| 483 | 534 | } | |
| @@ -493,7 +544,7 @@ func startLocalDaemon(ctx context.Context) (*exec.Cmd, error) { | |||
| 493 | 544 | select { | |
| 494 | 545 | case <-deadline.Done(): | |
| 495 | 546 | stopLocalProcess(cmd) | |
| 496 | - | return nil, deadline.Err() | |
| 547 | + | return nil, commandErrorWithStderr(deadline.Err(), stderr.String()) | |
| 497 | 548 | case <-time.After(25 * time.Millisecond): | |
| 498 | 549 | } | |
| 499 | 550 | } | |
| @@ -515,10 +566,18 @@ func stopLocalProcess(cmd *exec.Cmd) { | |||
| 515 | 566 | } | |
| 516 | 567 | ||
| 517 | 568 | func runLocalCommand(ctx context.Context, name string, args ...string) error { | |
| 569 | + | return runCapturedCommand(ctx, name, args...) | |
| 570 | + | } | |
| 571 | + | ||
| 572 | + | func runCapturedCommand(ctx context.Context, name string, args ...string) error { | |
| 518 | 573 | cmd := exec.CommandContext(ctx, name, args...) | |
| 574 | + | stderr := &boundedStderr{} | |
| 519 | 575 | cmd.Stdout = io.Discard | |
| 520 | - | cmd.Stderr = io.Discard | |
| 521 | - | return cmd.Run() | |
| 576 | + | cmd.Stderr = stderr | |
| 577 | + | if err := cmd.Run(); err != nil { | |
| 578 | + | return commandErrorWithStderr(err, stderr.String()) | |
| 579 | + | } | |
| 580 | + | return nil | |
| 522 | 581 | } | |
| 523 | 582 | ||
| 524 | 583 | func openLocalTransport(ctx context.Context) (*harnessTransport, error) { | |
| @@ -928,10 +987,7 @@ func touchRemoteFlag(ctx context.Context, target, path string) error { | |||
| 928 | 987 | ||
| 929 | 988 | func runRemoteCommand(ctx context.Context, target string, command ...string) error { | |
| 930 | 989 | spec := sshstdio.BuildCommandForRemoteCommand(target, command...) | |
| 931 | - | cmd := exec.CommandContext(ctx, spec.Path, spec.Args...) | |
| 932 | - | cmd.Stdout = io.Discard | |
| 933 | - | cmd.Stderr = io.Discard | |
| 934 | - | return cmd.Run() | |
| 990 | + | return runCapturedCommand(ctx, spec.Path, spec.Args...) | |
| 935 | 991 | } | |
| 936 | 992 | ||
| 937 | 993 | func receiveWithTimeout(ctx context.Context, tr *harnessTransport) (ports.Frame, error) { | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package main | 1 | package main | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "bytes" | |||
| 4 | "context" | 5 | "context" | ||
| 5 | "errors" | 6 | "errors" | ||
| 6 | "fmt" | 7 | "fmt" | ||
| @@ -140,7 +141,7 @@ func runLiveStdioPhase(ctx context.Context, target string, catalog ports.RemoteC | |||||
| 140 | // daemon container. It validates the seam that individual catalog, preview, | 141 | // daemon container. It validates the seam that individual catalog, preview, | ||
| 141 | // and attach tests cannot: catalog active-tab selection, live remote preview, | 142 | // and attach tests cannot: catalog active-tab selection, live remote preview, | ||
| 142 | // emitted rich handoff, and the remote attachment's visible provenance. | 143 | // emitted rich handoff, and the remote attachment's visible provenance. | ||
| 143 | - | func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, preview ports.RemotePreviewClient) error { | 144 | + | func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, preview ports.RemotePreviewClient) (err error) { |
| 144 | const ( | 145 | const ( | ||
| 145 | localSession = "local-picker" | 146 | localSession = "local-picker" | ||
| 146 | remoteSession = "picker" | 147 | remoteSession = "picker" | ||
| @@ -151,13 +152,25 @@ func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports | |||||
| 151 | if err != nil { | 152 | if err != nil { | ||
| 152 | return fmt.Errorf("open remote picker source: %w", err) | 153 | return fmt.Errorf("open remote picker source: %w", err) | ||
| 153 | } | 154 | } | ||
| 154 | - | defer func() { _ = remote.Close() }() | 155 | + | ownsRemoteSession := false |
| 156 | + | defer func() { | |||
| 157 | + | _ = remote.Close() | |||
| 158 | + | if !ownsRemoteSession { | |||
| 159 | + | return | |||
| 160 | + | } | |||
| 161 | + | cleanup, cancel := context.WithTimeout(context.Background(), 5*time.Second) | |||
| 162 | + | defer cancel() | |||
| 163 | + | if cleanupErr := runRemoteCommand(cleanup, target, "vev", "kill", remoteSession); cleanupErr != nil { | |||
| 164 | + | err = errors.Join(err, fmt.Errorf("destroy remote picker session: %w", cleanupErr)) | |||
| 165 | + | } | |||
| 166 | + | }() | |||
| 155 | if err := attachAndCheck(ctx, remote, ports.Hello{ | 167 | if err := attachAndCheck(ctx, remote, ports.Hello{ | ||
| 156 | Intent: ports.IntentNew, Name: remoteSession, Env: localEnvironment(), Cwd: "/tmp/remote-cwd", | 168 | Intent: ports.IntentNew, Name: remoteSession, Env: localEnvironment(), Cwd: "/tmp/remote-cwd", | ||
| 157 | TermEnv: "xterm-256color", TrueColor: true, | 169 | TermEnv: "xterm-256color", TrueColor: true, | ||
| 158 | }); err != nil { | 170 | }); err != nil { | ||
| 159 | return fmt.Errorf("attach remote picker source: %w", err) | 171 | return fmt.Errorf("attach remote picker source: %w", err) | ||
| 160 | } | 172 | } | ||
| 173 | + | ownsRemoteSession = true | |||
| 161 | if err := sendCommand(ctx, remote, "new-tab"); err != nil { | 174 | if err := sendCommand(ctx, remote, "new-tab"); err != nil { | ||
| 162 | return fmt.Errorf("create remote picker tab: %w", err) | 175 | return fmt.Errorf("create remote picker tab: %w", err) | ||
| 163 | } | 176 | } | ||
| @@ -474,10 +487,48 @@ func openTransport(ctx context.Context, mode ports.RemoteTransportMode, target s | |||||
| 474 | return newHarnessTransport(transport), nil | 487 | return newHarnessTransport(transport), nil | ||
| 475 | } | 488 | } | ||
| 476 | 489 | ||||
| 490 | + | const commandStderrLimit = 4 << 10 | |||
| 491 | + | ||||
| 492 | + | type boundedStderr struct { | |||
| 493 | + | bytes.Buffer | |||
| 494 | + | truncated bool | |||
| 495 | + | } | |||
| 496 | + | ||||
| 497 | + | func (b *boundedStderr) Write(p []byte) (int, error) { | |||
| 498 | + | written := len(p) | |||
| 499 | + | remaining := commandStderrLimit - b.Len() | |||
| 500 | + | if remaining <= 0 { | |||
| 501 | + | b.truncated = b.truncated || written != 0 | |||
| 502 | + | return written, nil | |||
| 503 | + | } | |||
| 504 | + | if len(p) > remaining { | |||
| 505 | + | p = p[:remaining] | |||
| 506 | + | b.truncated = true | |||
| 507 | + | } | |||
| 508 | + | _, _ = b.Buffer.Write(p) | |||
| 509 | + | return written, nil | |||
| 510 | + | } | |||
| 511 | + | ||||
| 512 | + | func (b *boundedStderr) String() string { | |||
| 513 | + | stderr := b.Buffer.String() | |||
| 514 | + | if b.truncated { | |||
| 515 | + | stderr += "\n[stderr truncated]" | |||
| 516 | + | } | |||
| 517 | + | return stderr | |||
| 518 | + | } | |||
| 519 | + | ||||
| 520 | + | func commandErrorWithStderr(err error, stderr string) error { | |||
| 521 | + | if stderr = strings.TrimSpace(stderr); stderr == "" { | |||
| 522 | + | return err | |||
| 523 | + | } | |||
| 524 | + | return fmt.Errorf("%w: stderr: %s", err, stderr) | |||
| 525 | + | } | |||
| 526 | + | ||||
| 477 | func startLocalDaemon(ctx context.Context) (*exec.Cmd, error) { | 527 | func startLocalDaemon(ctx context.Context) (*exec.Cmd, error) { | ||
| 478 | cmd := exec.CommandContext(ctx, "vev", "--daemon") | 528 | cmd := exec.CommandContext(ctx, "vev", "--daemon") | ||
| 529 | + | stderr := &boundedStderr{} | |||
| 479 | cmd.Stdout = io.Discard | 530 | cmd.Stdout = io.Discard | ||
| 480 | - | cmd.Stderr = io.Discard | 531 | + | cmd.Stderr = stderr |
| 481 | if err := cmd.Start(); err != nil { | 532 | if err := cmd.Start(); err != nil { | ||
| 482 | return nil, err | 533 | return nil, err | ||
| 483 | } | 534 | } | ||
| @@ -493,7 +544,7 @@ func startLocalDaemon(ctx context.Context) (*exec.Cmd, error) { | |||||
| 493 | select { | 544 | select { | ||
| 494 | case <-deadline.Done(): | 545 | case <-deadline.Done(): | ||
| 495 | stopLocalProcess(cmd) | 546 | stopLocalProcess(cmd) | ||
| 496 | - | return nil, deadline.Err() | 547 | + | return nil, commandErrorWithStderr(deadline.Err(), stderr.String()) |
| 497 | case <-time.After(25 * time.Millisecond): | 548 | case <-time.After(25 * time.Millisecond): | ||
| 498 | } | 549 | } | ||
| 499 | } | 550 | } | ||
| @@ -515,10 +566,18 @@ func stopLocalProcess(cmd *exec.Cmd) { | |||||
| 515 | } | 566 | } | ||
| 516 | 567 | ||||
| 517 | func runLocalCommand(ctx context.Context, name string, args ...string) error { | 568 | func runLocalCommand(ctx context.Context, name string, args ...string) error { | ||
| 569 | + | return runCapturedCommand(ctx, name, args...) | |||
| 570 | + | } | |||
| 571 | + | ||||
| 572 | + | func runCapturedCommand(ctx context.Context, name string, args ...string) error { | |||
| 518 | cmd := exec.CommandContext(ctx, name, args...) | 573 | cmd := exec.CommandContext(ctx, name, args...) | ||
| 574 | + | stderr := &boundedStderr{} | |||
| 519 | cmd.Stdout = io.Discard | 575 | cmd.Stdout = io.Discard | ||
| 520 | - | cmd.Stderr = io.Discard | 576 | + | cmd.Stderr = stderr |
| 521 | - | return cmd.Run() | 577 | + | if err := cmd.Run(); err != nil { |
| 578 | + | return commandErrorWithStderr(err, stderr.String()) | |||
| 579 | + | } | |||
| 580 | + | return nil | |||
| 522 | } | 581 | } | ||
| 523 | 582 | ||||
| 524 | func openLocalTransport(ctx context.Context) (*harnessTransport, error) { | 583 | func openLocalTransport(ctx context.Context) (*harnessTransport, error) { | ||
| @@ -928,10 +987,7 @@ func touchRemoteFlag(ctx context.Context, target, path string) error { | |||||
| 928 | 987 | ||||
| 929 | func runRemoteCommand(ctx context.Context, target string, command ...string) error { | 988 | func runRemoteCommand(ctx context.Context, target string, command ...string) error { | ||
| 930 | spec := sshstdio.BuildCommandForRemoteCommand(target, command...) | 989 | spec := sshstdio.BuildCommandForRemoteCommand(target, command...) | ||
| 931 | - | cmd := exec.CommandContext(ctx, spec.Path, spec.Args...) | 990 | + | return runCapturedCommand(ctx, spec.Path, spec.Args...) |
| 932 | - | cmd.Stdout = io.Discard | |||
| 933 | - | cmd.Stderr = io.Discard | |||
| 934 | - | return cmd.Run() | |||
| 935 | } | 991 | } | ||
| 936 | 992 | ||||
| 937 | func receiveWithTimeout(ctx context.Context, tr *harnessTransport) (ports.Frame, error) { | 993 | func receiveWithTimeout(ctx context.Context, tr *harnessTransport) (ports.Frame, error) { | ||
A scripts/remote-picker-harness/main_test.go +44 -0
| @@ -0,0 +1,44 @@ | |||
| 1 | + | package main | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "errors" | |
| 5 | + | "strings" | |
| 6 | + | "testing" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestBoundedStderrRetainsAtMostDiagnosticLimit(t *testing.T) { | |
| 10 | + | stderr := &boundedStderr{} | |
| 11 | + | input := strings.Repeat("x", commandStderrLimit+1) | |
| 12 | + | ||
| 13 | + | written, err := stderr.Write([]byte(input)) | |
| 14 | + | ||
| 15 | + | if err != nil { | |
| 16 | + | t.Fatal(err) | |
| 17 | + | } | |
| 18 | + | if written != len(input) { | |
| 19 | + | t.Fatalf("written = %d, want %d", written, len(input)) | |
| 20 | + | } | |
| 21 | + | if stderr.Len() != commandStderrLimit { | |
| 22 | + | t.Fatalf("stderr length = %d, want %d", stderr.Len(), commandStderrLimit) | |
| 23 | + | } | |
| 24 | + | if !strings.Contains(stderr.String(), "[stderr truncated]") { | |
| 25 | + | t.Fatalf("stderr = %q, want truncation marker", stderr.String()) | |
| 26 | + | } | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | func TestCommandErrorWithStderr(t *testing.T) { | |
| 30 | + | cause := errors.New("command failed") | |
| 31 | + | if got := commandErrorWithStderr(cause, " daemon could not start\n"); !errors.Is(got, cause) || !strings.Contains(got.Error(), "stderr: daemon could not start") { | |
| 32 | + | t.Fatalf("command error = %v", got) | |
| 33 | + | } | |
| 34 | + | if got := commandErrorWithStderr(cause, "\n"); got != cause { | |
| 35 | + | t.Fatalf("empty stderr error = %v, want original error", got) | |
| 36 | + | } | |
| 37 | + | } | |
| 38 | + | ||
| 39 | + | func TestRunCapturedCommandIncludesStderr(t *testing.T) { | |
| 40 | + | err := runCapturedCommand(t.Context(), "sh", "-c", "printf 'command diagnostics\\n' >&2; exit 1") | |
| 41 | + | if err == nil || !strings.Contains(err.Error(), "stderr: command diagnostics") { | |
| 42 | + | t.Fatalf("command error = %v", err) | |
| 43 | + | } | |
| 44 | + | } | |
| @@ -0,0 +1,44 @@ | |||||
| 1 | + | package main | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "errors" | |||
| 5 | + | "strings" | |||
| 6 | + | "testing" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestBoundedStderrRetainsAtMostDiagnosticLimit(t *testing.T) { | |||
| 10 | + | stderr := &boundedStderr{} | |||
| 11 | + | input := strings.Repeat("x", commandStderrLimit+1) | |||
| 12 | + | ||||
| 13 | + | written, err := stderr.Write([]byte(input)) | |||
| 14 | + | ||||
| 15 | + | if err != nil { | |||
| 16 | + | t.Fatal(err) | |||
| 17 | + | } | |||
| 18 | + | if written != len(input) { | |||
| 19 | + | t.Fatalf("written = %d, want %d", written, len(input)) | |||
| 20 | + | } | |||
| 21 | + | if stderr.Len() != commandStderrLimit { | |||
| 22 | + | t.Fatalf("stderr length = %d, want %d", stderr.Len(), commandStderrLimit) | |||
| 23 | + | } | |||
| 24 | + | if !strings.Contains(stderr.String(), "[stderr truncated]") { | |||
| 25 | + | t.Fatalf("stderr = %q, want truncation marker", stderr.String()) | |||
| 26 | + | } | |||
| 27 | + | } | |||
| 28 | + | ||||
| 29 | + | func TestCommandErrorWithStderr(t *testing.T) { | |||
| 30 | + | cause := errors.New("command failed") | |||
| 31 | + | if got := commandErrorWithStderr(cause, " daemon could not start\n"); !errors.Is(got, cause) || !strings.Contains(got.Error(), "stderr: daemon could not start") { | |||
| 32 | + | t.Fatalf("command error = %v", got) | |||
| 33 | + | } | |||
| 34 | + | if got := commandErrorWithStderr(cause, "\n"); got != cause { | |||
| 35 | + | t.Fatalf("empty stderr error = %v, want original error", got) | |||
| 36 | + | } | |||
| 37 | + | } | |||
| 38 | + | ||||
| 39 | + | func TestRunCapturedCommandIncludesStderr(t *testing.T) { | |||
| 40 | + | err := runCapturedCommand(t.Context(), "sh", "-c", "printf 'command diagnostics\\n' >&2; exit 1") | |||
| 41 | + | if err == nil || !strings.Contains(err.Error(), "stderr: command diagnostics") { | |||
| 42 | + | t.Fatalf("command error = %v", err) | |||
| 43 | + | } | |||
| 44 | + | } | |||