fix(remote): address picker parity review findings
- commit
- 9aa4936762c021c39c332e6bfd8a9fa47f7438cc
- author
- brice · · 2026-08-05 10:31 UTC
- parent
- 2d2ac84
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
35 files · +589 -238
M AGENTS.md +1 -1
| @@ -64,7 +64,7 @@ Before touching daemon teardown paths, read the lock-ordering notes at the top o | |||
| 64 | 64 | Wire payload types/codecs live in `internal/ports/frame.go` and `internal/ports/wire.go`. Connection framing lives in `internal/adapters/ipc/transport.go`. | |
| 65 | 65 | ||
| 66 | 66 | - IPC frames on a connection are 4-byte big-endian length, 1 type byte, then payload. | |
| 67 | - | - Client message types currently occupy `1–13` (`MsgOutputResetRequest` is `13`); server types occupy `16–23` (`MsgSessionMeta` is `23`) and `25` (`MsgAttachTarget`). Types `14–15` and `24` remain reserved. | |
| 67 | + | - Client message types occupy `1–13` (`MsgOutputResetRequest` is `13`) and `15` (`MsgRemotePreviewRequest`); server types occupy `16–23` (`MsgSessionMeta` is `23`) and `25–26` (`MsgAttachTarget` and `MsgRemotePreviewResponse`). Types `14` and `24` remain reserved. | |
| 68 | 68 | - Version negotiation requires strict equality. | |
| 69 | 69 | - `Hello.Version` and `CommandRequest.Version` must stay first so their version peekers work. | |
| 70 | 70 | - Bump `ProtocolVersion` for any message layout change. | |
| @@ -64,7 +64,7 @@ Before touching daemon teardown paths, read the lock-ordering notes at the top o | |||||
| 64 | Wire payload types/codecs live in `internal/ports/frame.go` and `internal/ports/wire.go`. Connection framing lives in `internal/adapters/ipc/transport.go`. | 64 | Wire payload types/codecs live in `internal/ports/frame.go` and `internal/ports/wire.go`. Connection framing lives in `internal/adapters/ipc/transport.go`. | ||
| 65 | 65 | ||||
| 66 | - IPC frames on a connection are 4-byte big-endian length, 1 type byte, then payload. | 66 | - IPC frames on a connection are 4-byte big-endian length, 1 type byte, then payload. | ||
| 67 | - | - Client message types currently occupy `1–13` (`MsgOutputResetRequest` is `13`); server types occupy `16–23` (`MsgSessionMeta` is `23`) and `25` (`MsgAttachTarget`). Types `14–15` and `24` remain reserved. | 67 | + | - Client message types occupy `1–13` (`MsgOutputResetRequest` is `13`) and `15` (`MsgRemotePreviewRequest`); server types occupy `16–23` (`MsgSessionMeta` is `23`) and `25–26` (`MsgAttachTarget` and `MsgRemotePreviewResponse`). Types `14` and `24` remain reserved. |
| 68 | - Version negotiation requires strict equality. | 68 | - Version negotiation requires strict equality. | ||
| 69 | - `Hello.Version` and `CommandRequest.Version` must stay first so their version peekers work. | 69 | - `Hello.Version` and `CommandRequest.Version` must stay first so their version peekers work. | ||
| 70 | - Bump `ProtocolVersion` for any message layout change. | 70 | - Bump `ProtocolVersion` for any message layout change. | ||
M docs/remote-resilience.md +8 -0
| @@ -25,6 +25,14 @@ owning daemon without creating a local shadow session. Picker-selected remote | |||
| 25 | 25 | attachments use the daemon's environment and persisted session working | |
| 26 | 26 | directory; direct CLI remote attaches retain their client-request semantics. | |
| 27 | 27 | ||
| 28 | + | ## Durable record compatibility | |
| 29 | + | ||
| 30 | + | The catalogue record format is currently version 4 because records now retain | |
| 31 | + | stable tab identity metadata. Readers accept version 3 for one-way upgrades, but | |
| 32 | + | older binaries reject records written at version 4. Back up the vev state | |
| 33 | + | before a rollback; downgrading after an upgrade can leave newer catalogue | |
| 34 | + | records unreadable and may require recreating those sessions. | |
| 35 | + | ||
| 28 | 36 | Remote catalogues are bounded and versioned. Complete catalogues contain ordered | |
| 29 | 37 | tab IDs, names, state, active tab, attachment state, and MRU sequence. Stopped | |
| 30 | 38 | tab metadata is retained across daemon restart; legacy count-only catalogues are | |
| @@ -25,6 +25,14 @@ owning daemon without creating a local shadow session. Picker-selected remote | |||||
| 25 | attachments use the daemon's environment and persisted session working | 25 | attachments use the daemon's environment and persisted session working | ||
| 26 | directory; direct CLI remote attaches retain their client-request semantics. | 26 | directory; direct CLI remote attaches retain their client-request semantics. | ||
| 27 | 27 | ||||
| 28 | + | ## Durable record compatibility | |||
| 29 | + | ||||
| 30 | + | The catalogue record format is currently version 4 because records now retain | |||
| 31 | + | stable tab identity metadata. Readers accept version 3 for one-way upgrades, but | |||
| 32 | + | older binaries reject records written at version 4. Back up the vev state | |||
| 33 | + | before a rollback; downgrading after an upgrade can leave newer catalogue | |||
| 34 | + | records unreadable and may require recreating those sessions. | |||
| 35 | + | ||||
| 28 | Remote catalogues are bounded and versioned. Complete catalogues contain ordered | 36 | Remote catalogues are bounded and versioned. Complete catalogues contain ordered | ||
| 29 | tab IDs, names, state, active tab, attachment state, and MRU sequence. Stopped | 37 | tab IDs, names, state, active tab, attachment state, and MRU sequence. Stopped | ||
| 30 | tab metadata is retained across daemon restart; legacy count-only catalogues are | 38 | tab metadata is retained across daemon restart; legacy count-only catalogues are | ||
M internal/adapters/remote/catalog_cache.go +2 -2
| @@ -17,7 +17,7 @@ import ( | |||
| 17 | 17 | ||
| 18 | 18 | const ( | |
| 19 | 19 | catalogCacheFileName = "remote-catalog-cache.json" | |
| 20 | - | catalogCacheFileVersion = 1 | |
| 20 | + | catalogCacheFileVersion = 2 | |
| 21 | 21 | ) | |
| 22 | 22 | ||
| 23 | 23 | // CatalogCachePath returns the canonical location of the remote catalog cache in stateDir. | |
| @@ -177,7 +177,7 @@ func (c *fileCatalogCache) Store(entries []ports.RemoteCatalogCacheEntry) error | |||
| 177 | 177 | copyTabs := append([]ports.RemoteCatalogTab(nil), tabs...) | |
| 178 | 178 | row.TabList = ©Tabs | |
| 179 | 179 | } else { | |
| 180 | - | count := uint16(ports.CatalogTabCount(session)) | |
| 180 | + | count := ports.SaturateUint16(ports.CatalogTabCount(session)) | |
| 181 | 181 | row.Tabs = &count | |
| 182 | 182 | } | |
| 183 | 183 | sessions = append(sessions, row) | |
| @@ -17,7 +17,7 @@ import ( | |||||
| 17 | 17 | ||||
| 18 | const ( | 18 | const ( | ||
| 19 | catalogCacheFileName = "remote-catalog-cache.json" | 19 | catalogCacheFileName = "remote-catalog-cache.json" | ||
| 20 | - | catalogCacheFileVersion = 1 | 20 | + | catalogCacheFileVersion = 2 |
| 21 | ) | 21 | ) | ||
| 22 | 22 | ||||
| 23 | // CatalogCachePath returns the canonical location of the remote catalog cache in stateDir. | 23 | // CatalogCachePath returns the canonical location of the remote catalog cache in stateDir. | ||
| @@ -177,7 +177,7 @@ func (c *fileCatalogCache) Store(entries []ports.RemoteCatalogCacheEntry) error | |||||
| 177 | copyTabs := append([]ports.RemoteCatalogTab(nil), tabs...) | 177 | copyTabs := append([]ports.RemoteCatalogTab(nil), tabs...) | ||
| 178 | row.TabList = ©Tabs | 178 | row.TabList = ©Tabs | ||
| 179 | } else { | 179 | } else { | ||
| 180 | - | count := uint16(ports.CatalogTabCount(session)) | 180 | + | count := ports.SaturateUint16(ports.CatalogTabCount(session)) |
| 181 | row.Tabs = &count | 181 | row.Tabs = &count | ||
| 182 | } | 182 | } | ||
| 183 | sessions = append(sessions, row) | 183 | sessions = append(sessions, row) | ||
M internal/adapters/remote/catalog_cache_test.go +1 -1
| @@ -115,7 +115,7 @@ func TestCatalogCacheLoadRejectsInvalidFilesWithoutReplacingThem(t *testing.T) { | |||
| 115 | 115 | }{ | |
| 116 | 116 | {name: "truncated", raw: []byte(`{"version":1,"hosts":[`)}, | |
| 117 | 117 | {name: "trailing JSON", raw: []byte(`{"version":1,"hosts":[]} {}`)}, | |
| 118 | - | {name: "unknown version", raw: []byte(`{"version":2,"hosts":[]}`)}, | |
| 118 | + | {name: "unknown version", raw: []byte(`{"version":3,"hosts":[]}`)}, | |
| 119 | 119 | {name: "missing hosts", raw: []byte(`{"version":1}`)}, | |
| 120 | 120 | {name: "null hosts", raw: []byte(`{"version":1,"hosts":null}`)}, | |
| 121 | 121 | {name: "null sessions", raw: []byte(`{"version":1,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":null}]}`)}, | |
| @@ -115,7 +115,7 @@ func TestCatalogCacheLoadRejectsInvalidFilesWithoutReplacingThem(t *testing.T) { | |||||
| 115 | }{ | 115 | }{ | ||
| 116 | {name: "truncated", raw: []byte(`{"version":1,"hosts":[`)}, | 116 | {name: "truncated", raw: []byte(`{"version":1,"hosts":[`)}, | ||
| 117 | {name: "trailing JSON", raw: []byte(`{"version":1,"hosts":[]} {}`)}, | 117 | {name: "trailing JSON", raw: []byte(`{"version":1,"hosts":[]} {}`)}, | ||
| 118 | - | {name: "unknown version", raw: []byte(`{"version":2,"hosts":[]}`)}, | 118 | + | {name: "unknown version", raw: []byte(`{"version":3,"hosts":[]}`)}, |
| 119 | {name: "missing hosts", raw: []byte(`{"version":1}`)}, | 119 | {name: "missing hosts", raw: []byte(`{"version":1}`)}, | ||
| 120 | {name: "null hosts", raw: []byte(`{"version":1,"hosts":null}`)}, | 120 | {name: "null hosts", raw: []byte(`{"version":1,"hosts":null}`)}, | ||
| 121 | {name: "null sessions", raw: []byte(`{"version":1,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":null}]}`)}, | 121 | {name: "null sessions", raw: []byte(`{"version":1,"hosts":[{"target":"arch","fetched_at_unix_nano":1,"sessions":null}]}`)}, | ||
M internal/adapters/remote/preview.go +7 -1
| @@ -60,11 +60,17 @@ func (c *PreviewClient) Preview(ctx context.Context, target domain.RemoteSession | |||
| 60 | 60 | cmd.WaitDelay = remotePreviewWaitDelay | |
| 61 | 61 | if err := cmd.Run(); err != nil { | |
| 62 | 62 | if ctxErr := runCtx.Err(); ctxErr != nil { | |
| 63 | - | return ports.RemotePreview{}, ctxErr | |
| 63 | + | if parentErr := ctx.Err(); parentErr != nil { | |
| 64 | + | return ports.RemotePreview{}, parentErr | |
| 65 | + | } | |
| 66 | + | return ports.RemotePreview{}, ports.ErrRemotePreviewTimeout | |
| 64 | 67 | } | |
| 65 | 68 | if stdout.overflow || stderr.overflow { | |
| 66 | 69 | return ports.RemotePreview{}, errRemotePreviewTooLarge | |
| 67 | 70 | } | |
| 71 | + | if diagnostic := sanitizeCatalogDiagnostic(string(stderr.Bytes())); diagnostic != "" { | |
| 72 | + | return ports.RemotePreview{}, fmt.Errorf("%w: %v: %s", errRemotePreviewSSH, err, diagnostic) | |
| 73 | + | } | |
| 68 | 74 | return ports.RemotePreview{}, fmt.Errorf("%w: %v", errRemotePreviewSSH, err) | |
| 69 | 75 | } | |
| 70 | 76 | if stdout.overflow || stderr.overflow { | |
| @@ -60,11 +60,17 @@ func (c *PreviewClient) Preview(ctx context.Context, target domain.RemoteSession | |||||
| 60 | cmd.WaitDelay = remotePreviewWaitDelay | 60 | cmd.WaitDelay = remotePreviewWaitDelay | ||
| 61 | if err := cmd.Run(); err != nil { | 61 | if err := cmd.Run(); err != nil { | ||
| 62 | if ctxErr := runCtx.Err(); ctxErr != nil { | 62 | if ctxErr := runCtx.Err(); ctxErr != nil { | ||
| 63 | - | return ports.RemotePreview{}, ctxErr | 63 | + | if parentErr := ctx.Err(); parentErr != nil { |
| 64 | + | return ports.RemotePreview{}, parentErr | |||
| 65 | + | } | |||
| 66 | + | return ports.RemotePreview{}, ports.ErrRemotePreviewTimeout | |||
| 64 | } | 67 | } | ||
| 65 | if stdout.overflow || stderr.overflow { | 68 | if stdout.overflow || stderr.overflow { | ||
| 66 | return ports.RemotePreview{}, errRemotePreviewTooLarge | 69 | return ports.RemotePreview{}, errRemotePreviewTooLarge | ||
| 67 | } | 70 | } | ||
| 71 | + | if diagnostic := sanitizeCatalogDiagnostic(string(stderr.Bytes())); diagnostic != "" { | |||
| 72 | + | return ports.RemotePreview{}, fmt.Errorf("%w: %v: %s", errRemotePreviewSSH, err, diagnostic) | |||
| 73 | + | } | |||
| 68 | return ports.RemotePreview{}, fmt.Errorf("%w: %v", errRemotePreviewSSH, err) | 74 | return ports.RemotePreview{}, fmt.Errorf("%w: %v", errRemotePreviewSSH, err) | ||
| 69 | } | 75 | } | ||
| 70 | if stdout.overflow || stderr.overflow { | 76 | if stdout.overflow || stderr.overflow { | ||
M internal/adapters/remote/preview_test.go +60 -0
| @@ -3,9 +3,12 @@ package remote | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/base64" | |
| 6 | + | "errors" | |
| 7 | + | "fmt" | |
| 6 | 8 | "os/exec" | |
| 7 | 9 | "strings" | |
| 8 | 10 | "testing" | |
| 11 | + | "time" | |
| 9 | 12 | ||
| 10 | 13 | "github.com/stretchr/testify/require" | |
| 11 | 14 | ||
| @@ -15,6 +18,12 @@ import ( | |||
| 15 | 18 | "github.com/bnema/vev/pkg/renderer" | |
| 16 | 19 | ) | |
| 17 | 20 | ||
| 21 | + | func previewClientTargetForTest() domain.RemoteSessionTarget { | |
| 22 | + | var lifecycle domain.SessionLifecycleID | |
| 23 | + | lifecycle[0] = 1 | |
| 24 | + | return domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | |
| 25 | + | } | |
| 26 | + | ||
| 18 | 27 | func TestRemotePreviewClientBuildsExactSSHCommandAndDecodesResponse(t *testing.T) { | |
| 19 | 28 | var lifecycle domain.SessionLifecycleID | |
| 20 | 29 | lifecycle[0] = 9 | |
| @@ -76,3 +85,54 @@ func TestRemotePreviewClientRejectsMalformedResponse(t *testing.T) { | |||
| 76 | 85 | _, err := client.Preview(context.Background(), target, 1, 1) | |
| 77 | 86 | require.Error(t, err) | |
| 78 | 87 | } | |
| 88 | + | ||
| 89 | + | func TestRemotePreviewClientRejectsOversizedOutput(t *testing.T) { | |
| 90 | + | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |
| 91 | + | return exec.CommandContext(ctx, "sh", "-c", fmt.Sprintf("head -c %d /dev/zero", ports.RemotePreviewMaxBytes+1)) | |
| 92 | + | }} | |
| 93 | + | target := previewClientTargetForTest() | |
| 94 | + | _, err := client.Preview(context.Background(), target, 1, 1) | |
| 95 | + | require.ErrorIs(t, err, errRemotePreviewTooLarge) | |
| 96 | + | } | |
| 97 | + | ||
| 98 | + | func TestRemotePreviewClientReturnsStderrDiagnostic(t *testing.T) { | |
| 99 | + | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |
| 100 | + | return exec.CommandContext(ctx, "sh", "-c", "printf 'connection refused' >&2; exit 1") | |
| 101 | + | }} | |
| 102 | + | target := previewClientTargetForTest() | |
| 103 | + | _, err := client.Preview(context.Background(), target, 1, 1) | |
| 104 | + | require.ErrorIs(t, err, errRemotePreviewSSH) | |
| 105 | + | require.Contains(t, err.Error(), "connection refused") | |
| 106 | + | } | |
| 107 | + | ||
| 108 | + | func TestRemotePreviewClientReturnsInternalTimeoutSentinel(t *testing.T) { | |
| 109 | + | client := &PreviewClient{ | |
| 110 | + | timeout: 20 * time.Millisecond, | |
| 111 | + | command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |
| 112 | + | return exec.CommandContext(ctx, "sleep", "10") | |
| 113 | + | }, | |
| 114 | + | } | |
| 115 | + | _, err := client.Preview(context.Background(), previewClientTargetForTest(), 1, 1) | |
| 116 | + | require.ErrorIs(t, err, ports.ErrRemotePreviewTimeout) | |
| 117 | + | } | |
| 118 | + | ||
| 119 | + | func TestRemotePreviewClientReturnsCallerCancellation(t *testing.T) { | |
| 120 | + | started := make(chan struct{}) | |
| 121 | + | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |
| 122 | + | close(started) | |
| 123 | + | return exec.CommandContext(ctx, "sleep", "10") | |
| 124 | + | }} | |
| 125 | + | target := previewClientTargetForTest() | |
| 126 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 127 | + | defer cancel() | |
| 128 | + | errCh := make(chan error, 1) | |
| 129 | + | go func() { | |
| 130 | + | _, err := client.Preview(ctx, target, 1, 1) | |
| 131 | + | errCh <- err | |
| 132 | + | }() | |
| 133 | + | <-started | |
| 134 | + | cancel() | |
| 135 | + | err := <-errCh | |
| 136 | + | require.ErrorIs(t, err, context.Canceled) | |
| 137 | + | require.False(t, errors.Is(err, errRemotePreviewSSH)) | |
| 138 | + | } | |
| @@ -3,9 +3,12 @@ package remote | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/base64" | 5 | "encoding/base64" | ||
| 6 | + | "errors" | |||
| 7 | + | "fmt" | |||
| 6 | "os/exec" | 8 | "os/exec" | ||
| 7 | "strings" | 9 | "strings" | ||
| 8 | "testing" | 10 | "testing" | ||
| 11 | + | "time" | |||
| 9 | 12 | ||||
| 10 | "github.com/stretchr/testify/require" | 13 | "github.com/stretchr/testify/require" | ||
| 11 | 14 | ||||
| @@ -15,6 +18,12 @@ import ( | |||||
| 15 | "github.com/bnema/vev/pkg/renderer" | 18 | "github.com/bnema/vev/pkg/renderer" | ||
| 16 | ) | 19 | ) | ||
| 17 | 20 | ||||
| 21 | + | func previewClientTargetForTest() domain.RemoteSessionTarget { | |||
| 22 | + | var lifecycle domain.SessionLifecycleID | |||
| 23 | + | lifecycle[0] = 1 | |||
| 24 | + | return domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | |||
| 25 | + | } | |||
| 26 | + | ||||
| 18 | func TestRemotePreviewClientBuildsExactSSHCommandAndDecodesResponse(t *testing.T) { | 27 | func TestRemotePreviewClientBuildsExactSSHCommandAndDecodesResponse(t *testing.T) { | ||
| 19 | var lifecycle domain.SessionLifecycleID | 28 | var lifecycle domain.SessionLifecycleID | ||
| 20 | lifecycle[0] = 9 | 29 | lifecycle[0] = 9 | ||
| @@ -76,3 +85,54 @@ func TestRemotePreviewClientRejectsMalformedResponse(t *testing.T) { | |||||
| 76 | _, err := client.Preview(context.Background(), target, 1, 1) | 85 | _, err := client.Preview(context.Background(), target, 1, 1) | ||
| 77 | require.Error(t, err) | 86 | require.Error(t, err) | ||
| 78 | } | 87 | } | ||
| 88 | + | ||||
| 89 | + | func TestRemotePreviewClientRejectsOversizedOutput(t *testing.T) { | |||
| 90 | + | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |||
| 91 | + | return exec.CommandContext(ctx, "sh", "-c", fmt.Sprintf("head -c %d /dev/zero", ports.RemotePreviewMaxBytes+1)) | |||
| 92 | + | }} | |||
| 93 | + | target := previewClientTargetForTest() | |||
| 94 | + | _, err := client.Preview(context.Background(), target, 1, 1) | |||
| 95 | + | require.ErrorIs(t, err, errRemotePreviewTooLarge) | |||
| 96 | + | } | |||
| 97 | + | ||||
| 98 | + | func TestRemotePreviewClientReturnsStderrDiagnostic(t *testing.T) { | |||
| 99 | + | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |||
| 100 | + | return exec.CommandContext(ctx, "sh", "-c", "printf 'connection refused' >&2; exit 1") | |||
| 101 | + | }} | |||
| 102 | + | target := previewClientTargetForTest() | |||
| 103 | + | _, err := client.Preview(context.Background(), target, 1, 1) | |||
| 104 | + | require.ErrorIs(t, err, errRemotePreviewSSH) | |||
| 105 | + | require.Contains(t, err.Error(), "connection refused") | |||
| 106 | + | } | |||
| 107 | + | ||||
| 108 | + | func TestRemotePreviewClientReturnsInternalTimeoutSentinel(t *testing.T) { | |||
| 109 | + | client := &PreviewClient{ | |||
| 110 | + | timeout: 20 * time.Millisecond, | |||
| 111 | + | command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |||
| 112 | + | return exec.CommandContext(ctx, "sleep", "10") | |||
| 113 | + | }, | |||
| 114 | + | } | |||
| 115 | + | _, err := client.Preview(context.Background(), previewClientTargetForTest(), 1, 1) | |||
| 116 | + | require.ErrorIs(t, err, ports.ErrRemotePreviewTimeout) | |||
| 117 | + | } | |||
| 118 | + | ||||
| 119 | + | func TestRemotePreviewClientReturnsCallerCancellation(t *testing.T) { | |||
| 120 | + | started := make(chan struct{}) | |||
| 121 | + | client := &PreviewClient{command: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { | |||
| 122 | + | close(started) | |||
| 123 | + | return exec.CommandContext(ctx, "sleep", "10") | |||
| 124 | + | }} | |||
| 125 | + | target := previewClientTargetForTest() | |||
| 126 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 127 | + | defer cancel() | |||
| 128 | + | errCh := make(chan error, 1) | |||
| 129 | + | go func() { | |||
| 130 | + | _, err := client.Preview(ctx, target, 1, 1) | |||
| 131 | + | errCh <- err | |||
| 132 | + | }() | |||
| 133 | + | <-started | |||
| 134 | + | cancel() | |||
| 135 | + | err := <-errCh | |||
| 136 | + | require.ErrorIs(t, err, context.Canceled) | |||
| 137 | + | require.False(t, errors.Is(err, errRemotePreviewSSH)) | |||
| 138 | + | } | |||
M internal/app/remote_preview.go +17 -2
| @@ -3,6 +3,7 @@ package app | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/base64" | |
| 6 | + | "errors" | |
| 6 | 7 | "fmt" | |
| 7 | 8 | "os" | |
| 8 | 9 | ||
| @@ -21,9 +22,12 @@ func runRemotePreview(ctx context.Context, encoded string) error { | |||
| 21 | 22 | if _, err := ports.UnmarshalRemotePreviewRequest(request); err != nil { | |
| 22 | 23 | return fmt.Errorf("vev: invalid remote preview request: %w", err) | |
| 23 | 24 | } | |
| 24 | - | transport, err := ensureDaemonWithLifecycle(ctx, ipc.SocketDir(), realDial, realSpawn, defaultBackoff) | |
| 25 | + | transport, err := ipc.DialContext(ctx, ipc.SocketDir()) | |
| 25 | 26 | if err != nil { | |
| 26 | - | return err | |
| 27 | + | if ctxErr := ctx.Err(); ctxErr != nil { | |
| 28 | + | return ctxErr | |
| 29 | + | } | |
| 30 | + | return writeRemotePreviewStatus(ports.RemotePreviewUnavailable) | |
| 27 | 31 | } | |
| 28 | 32 | defer func() { _ = transport.Close() }() | |
| 29 | 33 | if err := transport.Send(ports.Frame{Type: ports.MsgRemotePreviewRequest, Payload: request}); err != nil { | |
| @@ -58,3 +62,14 @@ func runRemotePreview(ctx context.Context, encoded string) error { | |||
| 58 | 62 | return err | |
| 59 | 63 | } | |
| 60 | 64 | } | |
| 65 | + | ||
| 66 | + | func writeRemotePreviewStatus(status ports.RemotePreviewStatus) error { | |
| 67 | + | payload := ports.MarshalRemotePreview(ports.RemotePreview{Version: ports.RemotePreviewSchemaVersion, Status: status}) | |
| 68 | + | if payload == nil { | |
| 69 | + | return errors.New("vev: failed to encode remote preview status") | |
| 70 | + | } | |
| 71 | + | if _, err := os.Stdout.Write(payload); err != nil { | |
| 72 | + | return fmt.Errorf("vev: writing remote preview status: %w", err) | |
| 73 | + | } | |
| 74 | + | return nil | |
| 75 | + | } | |
| @@ -3,6 +3,7 @@ package app | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/base64" | 5 | "encoding/base64" | ||
| 6 | + | "errors" | |||
| 6 | "fmt" | 7 | "fmt" | ||
| 7 | "os" | 8 | "os" | ||
| 8 | 9 | ||||
| @@ -21,9 +22,12 @@ func runRemotePreview(ctx context.Context, encoded string) error { | |||||
| 21 | if _, err := ports.UnmarshalRemotePreviewRequest(request); err != nil { | 22 | if _, err := ports.UnmarshalRemotePreviewRequest(request); err != nil { | ||
| 22 | return fmt.Errorf("vev: invalid remote preview request: %w", err) | 23 | return fmt.Errorf("vev: invalid remote preview request: %w", err) | ||
| 23 | } | 24 | } | ||
| 24 | - | transport, err := ensureDaemonWithLifecycle(ctx, ipc.SocketDir(), realDial, realSpawn, defaultBackoff) | 25 | + | transport, err := ipc.DialContext(ctx, ipc.SocketDir()) |
| 25 | if err != nil { | 26 | if err != nil { | ||
| 26 | - | return err | 27 | + | if ctxErr := ctx.Err(); ctxErr != nil { |
| 28 | + | return ctxErr | |||
| 29 | + | } | |||
| 30 | + | return writeRemotePreviewStatus(ports.RemotePreviewUnavailable) | |||
| 27 | } | 31 | } | ||
| 28 | defer func() { _ = transport.Close() }() | 32 | defer func() { _ = transport.Close() }() | ||
| 29 | if err := transport.Send(ports.Frame{Type: ports.MsgRemotePreviewRequest, Payload: request}); err != nil { | 33 | if err := transport.Send(ports.Frame{Type: ports.MsgRemotePreviewRequest, Payload: request}); err != nil { | ||
| @@ -58,3 +62,14 @@ func runRemotePreview(ctx context.Context, encoded string) error { | |||||
| 58 | return err | 62 | return err | ||
| 59 | } | 63 | } | ||
| 60 | } | 64 | } | ||
| 65 | + | ||||
| 66 | + | func writeRemotePreviewStatus(status ports.RemotePreviewStatus) error { | |||
| 67 | + | payload := ports.MarshalRemotePreview(ports.RemotePreview{Version: ports.RemotePreviewSchemaVersion, Status: status}) | |||
| 68 | + | if payload == nil { | |||
| 69 | + | return errors.New("vev: failed to encode remote preview status") | |||
| 70 | + | } | |||
| 71 | + | if _, err := os.Stdout.Write(payload); err != nil { | |||
| 72 | + | return fmt.Errorf("vev: writing remote preview status: %w", err) | |||
| 73 | + | } | |||
| 74 | + | return nil | |||
| 75 | + | } | |||
M internal/app/run.go +33 -21
| @@ -844,6 +844,8 @@ type attachHandoffKey struct { | |||
| 844 | 844 | stopped domain.TabSelector | |
| 845 | 845 | } | |
| 846 | 846 | ||
| 847 | + | const maxAttachHandoffHops = 32 | |
| 848 | + | ||
| 847 | 849 | func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, activeSession string, log *slog.Logger, deps runAttachDeps) error { | |
| 848 | 850 | if activeSession != "" { | |
| 849 | 851 | if remoteTarget == "" && intent == ports.IntentNew { | |
| @@ -865,6 +867,18 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac | |||
| 865 | 867 | var remoteSelection *domain.RemoteSessionTarget | |
| 866 | 868 | var remoteEnvironmentPolicy ports.EnvironmentPolicy | |
| 867 | 869 | seenHandoffs := make(map[attachHandoffKey]struct{}) | |
| 870 | + | handoffHops := 0 | |
| 871 | + | admitHandoff := func(next attachHandoffKey) error { | |
| 872 | + | if _, seen := seenHandoffs[next]; seen { | |
| 873 | + | return fmt.Errorf("vev: attach handoff loop for %q/%q", next.endpoint, next.session) | |
| 874 | + | } | |
| 875 | + | if handoffHops >= maxAttachHandoffHops { | |
| 876 | + | return fmt.Errorf("vev: attach handoff exceeded maximum of %d hops", maxAttachHandoffHops) | |
| 877 | + | } | |
| 878 | + | handoffHops++ | |
| 879 | + | seenHandoffs[next] = struct{}{} | |
| 880 | + | return nil | |
| 881 | + | } | |
| 868 | 882 | if remoteTarget != "" { | |
| 869 | 883 | if modeErr != nil { | |
| 870 | 884 | return modeErr | |
| @@ -894,10 +908,9 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac | |||
| 894 | 908 | next.liveTab = copyTarget.LiveTabID | |
| 895 | 909 | next.stopped = copyTarget.StoppedTab | |
| 896 | 910 | } | |
| 897 | - | if _, seen := seenHandoffs[next]; seen { | |
| 898 | - | return nil, client.AttachRequest{}, fmt.Errorf("vev: attach handoff loop for %q/%q", next.endpoint, next.session) | |
| 911 | + | if err := admitHandoff(next); err != nil { | |
| 912 | + | return nil, client.AttachRequest{}, err | |
| 899 | 913 | } | |
| 900 | - | seenHandoffs[next] = struct{}{} | |
| 901 | 914 | dialer, err := factory.DialerForRemote(target.Endpoint, target.Session, mode, log) | |
| 902 | 915 | if err != nil { | |
| 903 | 916 | return nil, client.AttachRequest{}, err | |
| @@ -946,33 +959,32 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac | |||
| 946 | 959 | }, client.AttachRequest{Intent: intent, SessionName: name}) | |
| 947 | 960 | } | |
| 948 | 961 | ||
| 949 | - | var handoff *client.AttachTargetError | |
| 950 | - | if !errors.As(err, &handoff) { | |
| 962 | + | var handoffErr *client.AttachTargetError | |
| 963 | + | if !errors.As(err, &handoffErr) { | |
| 951 | 964 | return err | |
| 952 | 965 | } | |
| 953 | - | if handoff == nil { | |
| 966 | + | if handoffErr == nil { | |
| 954 | 967 | return fmt.Errorf("vev: invalid remote attach handoff") | |
| 955 | 968 | } | |
| 956 | - | if err := validateRemoteAttachHandoff(handoff.Target); err != nil { | |
| 969 | + | if err := validateRemoteAttachHandoff(handoffErr.Target); err != nil { | |
| 957 | 970 | return fmt.Errorf("vev: invalid remote attach handoff: %w", err) | |
| 958 | 971 | } | |
| 959 | - | next := attachHandoffKey{endpoint: handoff.Target.Endpoint, session: handoff.Target.Session, intent: handoff.Target.Intent} | |
| 960 | - | if handoff.Target.RemoteTarget != nil { | |
| 961 | - | next.lifecycle = handoff.Target.RemoteTarget.LifecycleID | |
| 962 | - | next.liveTab = handoff.Target.RemoteTarget.LiveTabID | |
| 963 | - | next.stopped = handoff.Target.RemoteTarget.StoppedTab | |
| 972 | + | next := attachHandoffKey{endpoint: handoffErr.Target.Endpoint, session: handoffErr.Target.Session, intent: handoffErr.Target.Intent} | |
| 973 | + | if handoffErr.Target.RemoteTarget != nil { | |
| 974 | + | next.lifecycle = handoffErr.Target.RemoteTarget.LifecycleID | |
| 975 | + | next.liveTab = handoffErr.Target.RemoteTarget.LiveTabID | |
| 976 | + | next.stopped = handoffErr.Target.RemoteTarget.StoppedTab | |
| 964 | 977 | } | |
| 965 | - | if _, seen := seenHandoffs[next]; seen { | |
| 966 | - | return fmt.Errorf("vev: attach handoff loop for %q/%q", next.endpoint, next.session) | |
| 978 | + | if err := admitHandoff(next); err != nil { | |
| 979 | + | return err | |
| 967 | 980 | } | |
| 968 | - | seenHandoffs[next] = struct{}{} | |
| 969 | - | remoteTarget = handoff.Target.Endpoint | |
| 970 | - | name = handoff.Target.Session | |
| 971 | - | intent = handoff.Target.Intent | |
| 981 | + | remoteTarget = handoffErr.Target.Endpoint | |
| 982 | + | name = handoffErr.Target.Session | |
| 983 | + | intent = handoffErr.Target.Intent | |
| 972 | 984 | remoteSelection = nil | |
| 973 | - | remoteEnvironmentPolicy = handoff.Target.EnvironmentPolicy | |
| 974 | - | if handoff.Target.RemoteTarget != nil { | |
| 975 | - | copyTarget := *handoff.Target.RemoteTarget | |
| 985 | + | remoteEnvironmentPolicy = handoffErr.Target.EnvironmentPolicy | |
| 986 | + | if handoffErr.Target.RemoteTarget != nil { | |
| 987 | + | copyTarget := *handoffErr.Target.RemoteTarget | |
| 976 | 988 | remoteSelection = ©Target | |
| 977 | 989 | } | |
| 978 | 990 | if modeErr != nil { | |
| @@ -844,6 +844,8 @@ type attachHandoffKey struct { | |||||
| 844 | stopped domain.TabSelector | 844 | stopped domain.TabSelector | ||
| 845 | } | 845 | } | ||
| 846 | 846 | ||||
| 847 | + | const maxAttachHandoffHops = 32 | |||
| 848 | + | ||||
| 847 | func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, activeSession string, log *slog.Logger, deps runAttachDeps) error { | 849 | func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, activeSession string, log *slog.Logger, deps runAttachDeps) error { | ||
| 848 | if activeSession != "" { | 850 | if activeSession != "" { | ||
| 849 | if remoteTarget == "" && intent == ports.IntentNew { | 851 | if remoteTarget == "" && intent == ports.IntentNew { | ||
| @@ -865,6 +867,18 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac | |||||
| 865 | var remoteSelection *domain.RemoteSessionTarget | 867 | var remoteSelection *domain.RemoteSessionTarget | ||
| 866 | var remoteEnvironmentPolicy ports.EnvironmentPolicy | 868 | var remoteEnvironmentPolicy ports.EnvironmentPolicy | ||
| 867 | seenHandoffs := make(map[attachHandoffKey]struct{}) | 869 | seenHandoffs := make(map[attachHandoffKey]struct{}) | ||
| 870 | + | handoffHops := 0 | |||
| 871 | + | admitHandoff := func(next attachHandoffKey) error { | |||
| 872 | + | if _, seen := seenHandoffs[next]; seen { | |||
| 873 | + | return fmt.Errorf("vev: attach handoff loop for %q/%q", next.endpoint, next.session) | |||
| 874 | + | } | |||
| 875 | + | if handoffHops >= maxAttachHandoffHops { | |||
| 876 | + | return fmt.Errorf("vev: attach handoff exceeded maximum of %d hops", maxAttachHandoffHops) | |||
| 877 | + | } | |||
| 878 | + | handoffHops++ | |||
| 879 | + | seenHandoffs[next] = struct{}{} | |||
| 880 | + | return nil | |||
| 881 | + | } | |||
| 868 | if remoteTarget != "" { | 882 | if remoteTarget != "" { | ||
| 869 | if modeErr != nil { | 883 | if modeErr != nil { | ||
| 870 | return modeErr | 884 | return modeErr | ||
| @@ -894,10 +908,9 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac | |||||
| 894 | next.liveTab = copyTarget.LiveTabID | 908 | next.liveTab = copyTarget.LiveTabID | ||
| 895 | next.stopped = copyTarget.StoppedTab | 909 | next.stopped = copyTarget.StoppedTab | ||
| 896 | } | 910 | } | ||
| 897 | - | if _, seen := seenHandoffs[next]; seen { | 911 | + | if err := admitHandoff(next); err != nil { |
| 898 | - | return nil, client.AttachRequest{}, fmt.Errorf("vev: attach handoff loop for %q/%q", next.endpoint, next.session) | 912 | + | return nil, client.AttachRequest{}, err |
| 899 | } | 913 | } | ||
| 900 | - | seenHandoffs[next] = struct{}{} | |||
| 901 | dialer, err := factory.DialerForRemote(target.Endpoint, target.Session, mode, log) | 914 | dialer, err := factory.DialerForRemote(target.Endpoint, target.Session, mode, log) | ||
| 902 | if err != nil { | 915 | if err != nil { | ||
| 903 | return nil, client.AttachRequest{}, err | 916 | return nil, client.AttachRequest{}, err | ||
| @@ -946,33 +959,32 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac | |||||
| 946 | }, client.AttachRequest{Intent: intent, SessionName: name}) | 959 | }, client.AttachRequest{Intent: intent, SessionName: name}) | ||
| 947 | } | 960 | } | ||
| 948 | 961 | ||||
| 949 | - | var handoff *client.AttachTargetError | 962 | + | var handoffErr *client.AttachTargetError |
| 950 | - | if !errors.As(err, &handoff) { | 963 | + | if !errors.As(err, &handoffErr) { |
| 951 | return err | 964 | return err | ||
| 952 | } | 965 | } | ||
| 953 | - | if handoff == nil { | 966 | + | if handoffErr == nil { |
| 954 | return fmt.Errorf("vev: invalid remote attach handoff") | 967 | return fmt.Errorf("vev: invalid remote attach handoff") | ||
| 955 | } | 968 | } | ||
| 956 | - | if err := validateRemoteAttachHandoff(handoff.Target); err != nil { | 969 | + | if err := validateRemoteAttachHandoff(handoffErr.Target); err != nil { |
| 957 | return fmt.Errorf("vev: invalid remote attach handoff: %w", err) | 970 | return fmt.Errorf("vev: invalid remote attach handoff: %w", err) | ||
| 958 | } | 971 | } | ||
| 959 | - | next := attachHandoffKey{endpoint: handoff.Target.Endpoint, session: handoff.Target.Session, intent: handoff.Target.Intent} | 972 | + | next := attachHandoffKey{endpoint: handoffErr.Target.Endpoint, session: handoffErr.Target.Session, intent: handoffErr.Target.Intent} |
| 960 | - | if handoff.Target.RemoteTarget != nil { | 973 | + | if handoffErr.Target.RemoteTarget != nil { |
| 961 | - | next.lifecycle = handoff.Target.RemoteTarget.LifecycleID | 974 | + | next.lifecycle = handoffErr.Target.RemoteTarget.LifecycleID |
| 962 | - | next.liveTab = handoff.Target.RemoteTarget.LiveTabID | 975 | + | next.liveTab = handoffErr.Target.RemoteTarget.LiveTabID |
| 963 | - | next.stopped = handoff.Target.RemoteTarget.StoppedTab | 976 | + | next.stopped = handoffErr.Target.RemoteTarget.StoppedTab |
| 964 | } | 977 | } | ||
| 965 | - | if _, seen := seenHandoffs[next]; seen { | 978 | + | if err := admitHandoff(next); err != nil { |
| 966 | - | return fmt.Errorf("vev: attach handoff loop for %q/%q", next.endpoint, next.session) | 979 | + | return err |
| 967 | } | 980 | } | ||
| 968 | - | seenHandoffs[next] = struct{}{} | 981 | + | remoteTarget = handoffErr.Target.Endpoint |
| 969 | - | remoteTarget = handoff.Target.Endpoint | 982 | + | name = handoffErr.Target.Session |
| 970 | - | name = handoff.Target.Session | 983 | + | intent = handoffErr.Target.Intent |
| 971 | - | intent = handoff.Target.Intent | |||
| 972 | remoteSelection = nil | 984 | remoteSelection = nil | ||
| 973 | - | remoteEnvironmentPolicy = handoff.Target.EnvironmentPolicy | 985 | + | remoteEnvironmentPolicy = handoffErr.Target.EnvironmentPolicy |
| 974 | - | if handoff.Target.RemoteTarget != nil { | 986 | + | if handoffErr.Target.RemoteTarget != nil { |
| 975 | - | copyTarget := *handoff.Target.RemoteTarget | 987 | + | copyTarget := *handoffErr.Target.RemoteTarget |
| 976 | remoteSelection = ©Target | 988 | remoteSelection = ©Target | ||
| 977 | } | 989 | } | ||
| 978 | if modeErr != nil { | 990 | if modeErr != nil { | ||
M internal/app/run_test.go +17 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "context" | |
| 7 | 7 | "encoding/json" | |
| 8 | 8 | "errors" | |
| 9 | + | "fmt" | |
| 9 | 10 | "io" | |
| 10 | 11 | "log/slog" | |
| 11 | 12 | "net" | |
| @@ -906,6 +907,22 @@ func TestRunAttachWithDepsRejectsHandoffLoop(t *testing.T) { | |||
| 906 | 907 | require.Equal(t, 1, calls) | |
| 907 | 908 | } | |
| 908 | 909 | ||
| 910 | + | func TestRunAttachWithDepsCapsUniqueHandoffHops(t *testing.T) { | |
| 911 | + | factory := portsmocks.NewMockRemoteDialerFactory(t) | |
| 912 | + | factory.EXPECT().DialerForRemote(mock.Anything, mock.Anything, ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Times(maxAttachHandoffHops + 1) | |
| 913 | + | ||
| 914 | + | calls := 0 | |
| 915 | + | err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "remote.example", "", nil, runAttachDeps{ | |
| 916 | + | remoteDialerFactory: factory, | |
| 917 | + | runClient: func(_ context.Context, _ client.Dependencies, _ client.AttachRequest) error { | |
| 918 | + | calls++ | |
| 919 | + | return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: fmt.Sprintf("remote-%d.example", calls), Session: "work", Intent: ports.IntentAttach}} | |
| 920 | + | }, | |
| 921 | + | }) | |
| 922 | + | require.ErrorContains(t, err, "exceeded maximum") | |
| 923 | + | require.Equal(t, maxAttachHandoffHops+1, calls) | |
| 924 | + | } | |
| 925 | + | ||
| 909 | 926 | func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) { | |
| 910 | 927 | factory := portsmocks.NewMockRemoteDialerFactory(t) | |
| 911 | 928 | factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once() | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "context" | 6 | "context" | ||
| 7 | "encoding/json" | 7 | "encoding/json" | ||
| 8 | "errors" | 8 | "errors" | ||
| 9 | + | "fmt" | |||
| 9 | "io" | 10 | "io" | ||
| 10 | "log/slog" | 11 | "log/slog" | ||
| 11 | "net" | 12 | "net" | ||
| @@ -906,6 +907,22 @@ func TestRunAttachWithDepsRejectsHandoffLoop(t *testing.T) { | |||||
| 906 | require.Equal(t, 1, calls) | 907 | require.Equal(t, 1, calls) | ||
| 907 | } | 908 | } | ||
| 908 | 909 | ||||
| 910 | + | func TestRunAttachWithDepsCapsUniqueHandoffHops(t *testing.T) { | |||
| 911 | + | factory := portsmocks.NewMockRemoteDialerFactory(t) | |||
| 912 | + | factory.EXPECT().DialerForRemote(mock.Anything, mock.Anything, ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Times(maxAttachHandoffHops + 1) | |||
| 913 | + | ||||
| 914 | + | calls := 0 | |||
| 915 | + | err := runAttachWithDeps(context.Background(), ports.IntentAttach, "work", "remote.example", "", nil, runAttachDeps{ | |||
| 916 | + | remoteDialerFactory: factory, | |||
| 917 | + | runClient: func(_ context.Context, _ client.Dependencies, _ client.AttachRequest) error { | |||
| 918 | + | calls++ | |||
| 919 | + | return &client.AttachTargetError{Target: ports.AttachTarget{Endpoint: fmt.Sprintf("remote-%d.example", calls), Session: "work", Intent: ports.IntentAttach}} | |||
| 920 | + | }, | |||
| 921 | + | }) | |||
| 922 | + | require.ErrorContains(t, err, "exceeded maximum") | |||
| 923 | + | require.Equal(t, maxAttachHandoffHops+1, calls) | |||
| 924 | + | } | |||
| 925 | + | ||||
| 909 | func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) { | 926 | func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) { | ||
| 910 | factory := portsmocks.NewMockRemoteDialerFactory(t) | 927 | factory := portsmocks.NewMockRemoteDialerFactory(t) | ||
| 911 | factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once() | 928 | factory.EXPECT().DialerForRemote("selected.example", "picked", ports.RemoteTransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once() | ||
M internal/domain/lifecycle.go +1 -1
| @@ -81,7 +81,7 @@ type TabSelectorTab struct { | |||
| 81 | 81 | // Resolve returns the selected tab index only when the complete selector still | |
| 82 | 82 | // describes the supplied ordered tab metadata. | |
| 83 | 83 | func (s TabSelector) Resolve(tabs []TabSelectorTab) (int, bool) { | |
| 84 | - | if s.Validate() != nil || len(tabs) != int(s.ExpectedCount) && s.Kind == TabSelectorByOrdinal { | |
| 84 | + | if s.Validate() != nil || (len(tabs) != int(s.ExpectedCount) && s.Kind == TabSelectorByOrdinal) { | |
| 85 | 85 | return 0, false | |
| 86 | 86 | } | |
| 87 | 87 | switch s.Kind { | |
| @@ -81,7 +81,7 @@ type TabSelectorTab struct { | |||||
| 81 | // Resolve returns the selected tab index only when the complete selector still | 81 | // Resolve returns the selected tab index only when the complete selector still | ||
| 82 | // describes the supplied ordered tab metadata. | 82 | // describes the supplied ordered tab metadata. | ||
| 83 | func (s TabSelector) Resolve(tabs []TabSelectorTab) (int, bool) { | 83 | func (s TabSelector) Resolve(tabs []TabSelectorTab) (int, bool) { | ||
| 84 | - | if s.Validate() != nil || len(tabs) != int(s.ExpectedCount) && s.Kind == TabSelectorByOrdinal { | 84 | + | if s.Validate() != nil || (len(tabs) != int(s.ExpectedCount) && s.Kind == TabSelectorByOrdinal) { |
| 85 | return 0, false | 85 | return 0, false | ||
| 86 | } | 86 | } | ||
| 87 | switch s.Kind { | 87 | switch s.Kind { | ||
M internal/domain/lifecycle_test.go +29 -9
| @@ -3,18 +3,38 @@ package domain | |||
| 3 | 3 | import "testing" | |
| 4 | 4 | ||
| 5 | 5 | func TestOrdinalTabSelectorPreservesUnnamedTabs(t *testing.T) { | |
| 6 | - | selector := NewOrdinalTabSelector(1, "", 3) | |
| 7 | - | tabs := []TabSelectorTab{{Name: "first"}, {Name: ""}, {Name: "trailing"}} | |
| 8 | - | index, ok := selector.Resolve(tabs) | |
| 9 | - | if !ok || index != 1 { | |
| 10 | - | t.Fatalf("Resolve() = %d, %t, want 1, true", index, ok) | |
| 6 | + | tests := []struct { | |
| 7 | + | name string | |
| 8 | + | selector TabSelector | |
| 9 | + | tabs []TabSelectorTab | |
| 10 | + | want int | |
| 11 | + | }{ | |
| 12 | + | {name: "unnamed middle tab", selector: NewOrdinalTabSelector(1, "", 3), tabs: []TabSelectorTab{{Name: "first"}, {Name: ""}, {Name: "trailing"}}, want: 1}, | |
| 13 | + | } | |
| 14 | + | for _, test := range tests { | |
| 15 | + | t.Run(test.name, func(t *testing.T) { | |
| 16 | + | index, ok := test.selector.Resolve(test.tabs) | |
| 17 | + | if !ok || index != test.want { | |
| 18 | + | t.Fatalf("Resolve() = %d, %t, want %d, true", index, ok, test.want) | |
| 19 | + | } | |
| 20 | + | }) | |
| 11 | 21 | } | |
| 12 | 22 | } | |
| 13 | 23 | ||
| 14 | 24 | func TestValidateTabStableIDRejectsUnsafeIdentity(t *testing.T) { | |
| 15 | - | for _, id := range []TabStableID{"", "tab id", "tab\n1"} { | |
| 16 | - | if err := ValidateTabStableID(id); err == nil { | |
| 17 | - | t.Fatalf("ValidateTabStableID(%q) = nil, want error", id) | |
| 18 | - | } | |
| 25 | + | tests := []struct { | |
| 26 | + | name string | |
| 27 | + | id TabStableID | |
| 28 | + | }{ | |
| 29 | + | {name: "empty", id: ""}, | |
| 30 | + | {name: "space", id: "tab id"}, | |
| 31 | + | {name: "newline", id: "tab\n1"}, | |
| 32 | + | } | |
| 33 | + | for _, test := range tests { | |
| 34 | + | t.Run(test.name, func(t *testing.T) { | |
| 35 | + | if err := ValidateTabStableID(test.id); err == nil { | |
| 36 | + | t.Fatalf("ValidateTabStableID(%q) = nil, want error", test.id) | |
| 37 | + | } | |
| 38 | + | }) | |
| 19 | 39 | } | |
| 20 | 40 | } | |
| @@ -3,18 +3,38 @@ package domain | |||||
| 3 | import "testing" | 3 | import "testing" | ||
| 4 | 4 | ||||
| 5 | func TestOrdinalTabSelectorPreservesUnnamedTabs(t *testing.T) { | 5 | func TestOrdinalTabSelectorPreservesUnnamedTabs(t *testing.T) { | ||
| 6 | - | selector := NewOrdinalTabSelector(1, "", 3) | 6 | + | tests := []struct { |
| 7 | - | tabs := []TabSelectorTab{{Name: "first"}, {Name: ""}, {Name: "trailing"}} | 7 | + | name string |
| 8 | - | index, ok := selector.Resolve(tabs) | 8 | + | selector TabSelector |
| 9 | - | if !ok || index != 1 { | 9 | + | tabs []TabSelectorTab |
| 10 | - | t.Fatalf("Resolve() = %d, %t, want 1, true", index, ok) | 10 | + | want int |
| 11 | + | }{ | |||
| 12 | + | {name: "unnamed middle tab", selector: NewOrdinalTabSelector(1, "", 3), tabs: []TabSelectorTab{{Name: "first"}, {Name: ""}, {Name: "trailing"}}, want: 1}, | |||
| 13 | + | } | |||
| 14 | + | for _, test := range tests { | |||
| 15 | + | t.Run(test.name, func(t *testing.T) { | |||
| 16 | + | index, ok := test.selector.Resolve(test.tabs) | |||
| 17 | + | if !ok || index != test.want { | |||
| 18 | + | t.Fatalf("Resolve() = %d, %t, want %d, true", index, ok, test.want) | |||
| 19 | + | } | |||
| 20 | + | }) | |||
| 11 | } | 21 | } | ||
| 12 | } | 22 | } | ||
| 13 | 23 | ||||
| 14 | func TestValidateTabStableIDRejectsUnsafeIdentity(t *testing.T) { | 24 | func TestValidateTabStableIDRejectsUnsafeIdentity(t *testing.T) { | ||
| 15 | - | for _, id := range []TabStableID{"", "tab id", "tab\n1"} { | 25 | + | tests := []struct { |
| 16 | - | if err := ValidateTabStableID(id); err == nil { | 26 | + | name string |
| 17 | - | t.Fatalf("ValidateTabStableID(%q) = nil, want error", id) | 27 | + | id TabStableID |
| 18 | - | } | 28 | + | }{ |
| 29 | + | {name: "empty", id: ""}, | |||
| 30 | + | {name: "space", id: "tab id"}, | |||
| 31 | + | {name: "newline", id: "tab\n1"}, | |||
| 32 | + | } | |||
| 33 | + | for _, test := range tests { | |||
| 34 | + | t.Run(test.name, func(t *testing.T) { | |||
| 35 | + | if err := ValidateTabStableID(test.id); err == nil { | |||
| 36 | + | t.Fatalf("ValidateTabStableID(%q) = nil, want error", test.id) | |||
| 37 | + | } | |||
| 38 | + | }) | |||
| 19 | } | 39 | } | ||
| 20 | } | 40 | } | ||
M internal/domain/recovery.go +6 -1
| @@ -134,7 +134,12 @@ func (r CatalogueRecord) Validate() error { | |||
| 134 | 134 | } | |
| 135 | 135 | ||
| 136 | 136 | for _, tab := range r.TabRecords { | |
| 137 | - | if !utf8.ValidString(string(tab.StableID)) || !utf8.ValidString(tab.Name) { | |
| 137 | + | if tab.StableID != "" { | |
| 138 | + | if err := ValidateTabStableID(tab.StableID); err != nil { | |
| 139 | + | return errors.New("invalid catalogue tab metadata") | |
| 140 | + | } | |
| 141 | + | } | |
| 142 | + | if !utf8.ValidString(tab.Name) { | |
| 138 | 143 | return errors.New("invalid catalogue tab metadata") | |
| 139 | 144 | } | |
| 140 | 145 | } | |
| @@ -134,7 +134,12 @@ func (r CatalogueRecord) Validate() error { | |||||
| 134 | } | 134 | } | ||
| 135 | 135 | ||||
| 136 | for _, tab := range r.TabRecords { | 136 | for _, tab := range r.TabRecords { | ||
| 137 | - | if !utf8.ValidString(string(tab.StableID)) || !utf8.ValidString(tab.Name) { | 137 | + | if tab.StableID != "" { |
| 138 | + | if err := ValidateTabStableID(tab.StableID); err != nil { | |||
| 139 | + | return errors.New("invalid catalogue tab metadata") | |||
| 140 | + | } | |||
| 141 | + | } | |||
| 142 | + | if !utf8.ValidString(tab.Name) { | |||
| 138 | return errors.New("invalid catalogue tab metadata") | 143 | return errors.New("invalid catalogue tab metadata") | ||
| 139 | } | 144 | } | ||
| 140 | } | 145 | } | ||
M internal/ports/frame.go +5 -5
| @@ -16,9 +16,9 @@ const MaxFrameLen = 16 << 20 | |||
| 16 | 16 | // MsgType identifies the kind of payload carried by a Frame. | |
| 17 | 17 | type MsgType uint8 | |
| 18 | 18 | ||
| 19 | - | // Frame message types. Client-originated messages are numbered from 1 | |
| 20 | - | // through 15, server-originated messages from 16, with reserved values kept | |
| 21 | - | // for future extensions. | |
| 19 | + | // Frame message types. Client-originated messages occupy 1–13 and 15; | |
| 20 | + | // server-originated messages occupy 16–23 and 25–26. Values 14 and 24 remain | |
| 21 | + | // reserved for future extensions. | |
| 22 | 22 | const ( | |
| 23 | 23 | MsgHello MsgType = 1 | |
| 24 | 24 | MsgInput MsgType = 2 | |
| @@ -33,7 +33,7 @@ const ( | |||
| 33 | 33 | MsgClientNotice MsgType = 11 | |
| 34 | 34 | MsgCommand MsgType = 12 | |
| 35 | 35 | MsgOutputResetRequest MsgType = 13 | |
| 36 | - | MsgRemotePreviewRequest MsgType = 14 | |
| 36 | + | MsgRemotePreviewRequest MsgType = 15 | |
| 37 | 37 | ||
| 38 | 38 | MsgWelcome MsgType = 16 | |
| 39 | 39 | MsgError MsgType = 17 | |
| @@ -43,8 +43,8 @@ const ( | |||
| 43 | 43 | MsgSessions MsgType = 21 | |
| 44 | 44 | MsgCommandResult MsgType = 22 | |
| 45 | 45 | MsgSessionMeta MsgType = 23 | |
| 46 | - | MsgRemotePreviewResponse MsgType = 24 | |
| 47 | 46 | MsgAttachTarget MsgType = 25 | |
| 47 | + | MsgRemotePreviewResponse MsgType = 26 | |
| 48 | 48 | ) | |
| 49 | 49 | ||
| 50 | 50 | // Frame is the unit of exchange over a Transport: a typed, length-delimited | |
| @@ -16,9 +16,9 @@ const MaxFrameLen = 16 << 20 | |||||
| 16 | // MsgType identifies the kind of payload carried by a Frame. | 16 | // MsgType identifies the kind of payload carried by a Frame. | ||
| 17 | type MsgType uint8 | 17 | type MsgType uint8 | ||
| 18 | 18 | ||||
| 19 | - | // Frame message types. Client-originated messages are numbered from 1 | 19 | + | // Frame message types. Client-originated messages occupy 1–13 and 15; |
| 20 | - | // through 15, server-originated messages from 16, with reserved values kept | 20 | + | // server-originated messages occupy 16–23 and 25–26. Values 14 and 24 remain |
| 21 | - | // for future extensions. | 21 | + | // reserved for future extensions. |
| 22 | const ( | 22 | const ( | ||
| 23 | MsgHello MsgType = 1 | 23 | MsgHello MsgType = 1 | ||
| 24 | MsgInput MsgType = 2 | 24 | MsgInput MsgType = 2 | ||
| @@ -33,7 +33,7 @@ const ( | |||||
| 33 | MsgClientNotice MsgType = 11 | 33 | MsgClientNotice MsgType = 11 | ||
| 34 | MsgCommand MsgType = 12 | 34 | MsgCommand MsgType = 12 | ||
| 35 | MsgOutputResetRequest MsgType = 13 | 35 | MsgOutputResetRequest MsgType = 13 | ||
| 36 | - | MsgRemotePreviewRequest MsgType = 14 | 36 | + | MsgRemotePreviewRequest MsgType = 15 |
| 37 | 37 | ||||
| 38 | MsgWelcome MsgType = 16 | 38 | MsgWelcome MsgType = 16 | ||
| 39 | MsgError MsgType = 17 | 39 | MsgError MsgType = 17 | ||
| @@ -43,8 +43,8 @@ const ( | |||||
| 43 | MsgSessions MsgType = 21 | 43 | MsgSessions MsgType = 21 | ||
| 44 | MsgCommandResult MsgType = 22 | 44 | MsgCommandResult MsgType = 22 | ||
| 45 | MsgSessionMeta MsgType = 23 | 45 | MsgSessionMeta MsgType = 23 | ||
| 46 | - | MsgRemotePreviewResponse MsgType = 24 | |||
| 47 | MsgAttachTarget MsgType = 25 | 46 | MsgAttachTarget MsgType = 25 | ||
| 47 | + | MsgRemotePreviewResponse MsgType = 26 | |||
| 48 | ) | 48 | ) | ||
| 49 | 49 | ||||
| 50 | // Frame is the unit of exchange over a Transport: a typed, length-delimited | 50 | // Frame is the unit of exchange over a Transport: a typed, length-delimited | ||
M internal/ports/remote.go +1 -24
| @@ -85,25 +85,6 @@ type RemoteCatalogCacheEntry struct { | |||
| 85 | 85 | Sessions []RemoteCatalogSession | |
| 86 | 86 | } | |
| 87 | 87 | ||
| 88 | - | // RemoteCatalogCacheTab is the durable subset of a tab catalogue entry. | |
| 89 | - | type RemoteCatalogCacheTab struct { | |
| 90 | - | ID string `json:"id,omitempty"` | |
| 91 | - | Index uint16 `json:"index"` | |
| 92 | - | Name string `json:"name"` | |
| 93 | - | } | |
| 94 | - | ||
| 95 | - | // RemoteCatalogCacheSession is the durable subset of a remote session entry. | |
| 96 | - | type RemoteCatalogCacheSession struct { | |
| 97 | - | LifecycleID domain.SessionLifecycleID `json:"lifecycle_id,omitempty"` | |
| 98 | - | Name string `json:"name"` | |
| 99 | - | State string `json:"state"` | |
| 100 | - | Ephemeral bool `json:"ephemeral"` | |
| 101 | - | Attached bool `json:"attached"` | |
| 102 | - | LastUsedSeq uint64 `json:"last_used_seq,omitempty"` | |
| 103 | - | ActiveTabID string `json:"active_tab_id,omitempty"` | |
| 104 | - | Tabs []RemoteCatalogCacheTab `json:"tabs"` | |
| 105 | - | } | |
| 106 | - | ||
| 107 | 88 | // RemoteCatalogCache persists complete remote discovery snapshots independently | |
| 108 | 89 | // from the remote host registry. | |
| 109 | 90 | type RemoteCatalogCache interface { | |
| @@ -194,11 +175,7 @@ func (s *RemoteCatalogSession) UnmarshalJSON(data []byte) error { | |||
| 194 | 175 | if err := json.Unmarshal(raw.Tabs, &count); err != nil { | |
| 195 | 176 | return err | |
| 196 | 177 | } | |
| 197 | - | if count == math.MaxUint16 { | |
| 198 | - | s.Tabs = count | |
| 199 | - | } else { | |
| 200 | - | s.Tabs = int(count) | |
| 201 | - | } | |
| 178 | + | s.Tabs = int(count) | |
| 202 | 179 | return nil | |
| 203 | 180 | } | |
| 204 | 181 | ||
| @@ -85,25 +85,6 @@ type RemoteCatalogCacheEntry struct { | |||||
| 85 | Sessions []RemoteCatalogSession | 85 | Sessions []RemoteCatalogSession | ||
| 86 | } | 86 | } | ||
| 87 | 87 | ||||
| 88 | - | // RemoteCatalogCacheTab is the durable subset of a tab catalogue entry. | |||
| 89 | - | type RemoteCatalogCacheTab struct { | |||
| 90 | - | ID string `json:"id,omitempty"` | |||
| 91 | - | Index uint16 `json:"index"` | |||
| 92 | - | Name string `json:"name"` | |||
| 93 | - | } | |||
| 94 | - | ||||
| 95 | - | // RemoteCatalogCacheSession is the durable subset of a remote session entry. | |||
| 96 | - | type RemoteCatalogCacheSession struct { | |||
| 97 | - | LifecycleID domain.SessionLifecycleID `json:"lifecycle_id,omitempty"` | |||
| 98 | - | Name string `json:"name"` | |||
| 99 | - | State string `json:"state"` | |||
| 100 | - | Ephemeral bool `json:"ephemeral"` | |||
| 101 | - | Attached bool `json:"attached"` | |||
| 102 | - | LastUsedSeq uint64 `json:"last_used_seq,omitempty"` | |||
| 103 | - | ActiveTabID string `json:"active_tab_id,omitempty"` | |||
| 104 | - | Tabs []RemoteCatalogCacheTab `json:"tabs"` | |||
| 105 | - | } | |||
| 106 | - | ||||
| 107 | // RemoteCatalogCache persists complete remote discovery snapshots independently | 88 | // RemoteCatalogCache persists complete remote discovery snapshots independently | ||
| 108 | // from the remote host registry. | 89 | // from the remote host registry. | ||
| 109 | type RemoteCatalogCache interface { | 90 | type RemoteCatalogCache interface { | ||
| @@ -194,11 +175,7 @@ func (s *RemoteCatalogSession) UnmarshalJSON(data []byte) error { | |||||
| 194 | if err := json.Unmarshal(raw.Tabs, &count); err != nil { | 175 | if err := json.Unmarshal(raw.Tabs, &count); err != nil { | ||
| 195 | return err | 176 | return err | ||
| 196 | } | 177 | } | ||
| 197 | - | if count == math.MaxUint16 { | 178 | + | s.Tabs = int(count) |
| 198 | - | s.Tabs = count | |||
| 199 | - | } else { | |||
| 200 | - | s.Tabs = int(count) | |||
| 201 | - | } | |||
| 202 | return nil | 179 | return nil | ||
| 203 | } | 180 | } | ||
| 204 | 181 | ||||
M internal/ports/remote_preview.go +6 -2
| @@ -57,6 +57,7 @@ var ( | |||
| 57 | 57 | ErrInvalidRemotePreviewRequest = errors.New("ports: invalid remote preview request") | |
| 58 | 58 | ErrInvalidRemotePreview = errors.New("ports: invalid remote preview") | |
| 59 | 59 | ErrRemotePreviewTooLarge = errors.New("ports: remote preview exceeds size limit") | |
| 60 | + | ErrRemotePreviewTimeout = errors.New("ports: remote preview command timed out") | |
| 60 | 61 | ErrRemotePreviewUnsupportedStyle = errors.New("ports: remote preview has unsupported style") | |
| 61 | 62 | ) | |
| 62 | 63 | ||
| @@ -98,15 +99,18 @@ func ValidateRemotePreview(preview RemotePreview) error { | |||
| 98 | 99 | if want > RemotePreviewMaxCells || len(preview.Cells) != want { | |
| 99 | 100 | return ErrRemotePreviewTooLarge | |
| 100 | 101 | } | |
| 102 | + | width := int(preview.Width) | |
| 101 | 103 | for i, cell := range preview.Cells { | |
| 102 | 104 | if !utf8.ValidRune(cell.Rune) || !validRemotePreviewStyle(cell.Style) { | |
| 103 | 105 | return ErrInvalidRemotePreview | |
| 104 | 106 | } | |
| 107 | + | rowStart := (i / width) * width | |
| 108 | + | rowEnd := rowStart + width | |
| 105 | 109 | if cell.Continuation { | |
| 106 | - | if cell.Rune != 0 || i == 0 || preview.Cells[i-1].Continuation || renderer.RuneWidth(preview.Cells[i-1].Rune) != 2 { | |
| 110 | + | if cell.Rune != 0 || i == rowStart || preview.Cells[i-1].Continuation || renderer.RuneWidth(preview.Cells[i-1].Rune) != 2 { | |
| 107 | 111 | return ErrInvalidRemotePreview | |
| 108 | 112 | } | |
| 109 | - | } else if renderer.RuneWidth(cell.Rune) == 2 && (i+1 >= len(preview.Cells) || !preview.Cells[i+1].Continuation) { | |
| 113 | + | } else if renderer.RuneWidth(cell.Rune) == 2 && (i+1 >= rowEnd || !preview.Cells[i+1].Continuation) { | |
| 110 | 114 | return ErrInvalidRemotePreview | |
| 111 | 115 | } | |
| 112 | 116 | } | |
| @@ -57,6 +57,7 @@ var ( | |||||
| 57 | ErrInvalidRemotePreviewRequest = errors.New("ports: invalid remote preview request") | 57 | ErrInvalidRemotePreviewRequest = errors.New("ports: invalid remote preview request") | ||
| 58 | ErrInvalidRemotePreview = errors.New("ports: invalid remote preview") | 58 | ErrInvalidRemotePreview = errors.New("ports: invalid remote preview") | ||
| 59 | ErrRemotePreviewTooLarge = errors.New("ports: remote preview exceeds size limit") | 59 | ErrRemotePreviewTooLarge = errors.New("ports: remote preview exceeds size limit") | ||
| 60 | + | ErrRemotePreviewTimeout = errors.New("ports: remote preview command timed out") | |||
| 60 | ErrRemotePreviewUnsupportedStyle = errors.New("ports: remote preview has unsupported style") | 61 | ErrRemotePreviewUnsupportedStyle = errors.New("ports: remote preview has unsupported style") | ||
| 61 | ) | 62 | ) | ||
| 62 | 63 | ||||
| @@ -98,15 +99,18 @@ func ValidateRemotePreview(preview RemotePreview) error { | |||||
| 98 | if want > RemotePreviewMaxCells || len(preview.Cells) != want { | 99 | if want > RemotePreviewMaxCells || len(preview.Cells) != want { | ||
| 99 | return ErrRemotePreviewTooLarge | 100 | return ErrRemotePreviewTooLarge | ||
| 100 | } | 101 | } | ||
| 102 | + | width := int(preview.Width) | |||
| 101 | for i, cell := range preview.Cells { | 103 | for i, cell := range preview.Cells { | ||
| 102 | if !utf8.ValidRune(cell.Rune) || !validRemotePreviewStyle(cell.Style) { | 104 | if !utf8.ValidRune(cell.Rune) || !validRemotePreviewStyle(cell.Style) { | ||
| 103 | return ErrInvalidRemotePreview | 105 | return ErrInvalidRemotePreview | ||
| 104 | } | 106 | } | ||
| 107 | + | rowStart := (i / width) * width | |||
| 108 | + | rowEnd := rowStart + width | |||
| 105 | if cell.Continuation { | 109 | if cell.Continuation { | ||
| 106 | - | if cell.Rune != 0 || i == 0 || preview.Cells[i-1].Continuation || renderer.RuneWidth(preview.Cells[i-1].Rune) != 2 { | 110 | + | if cell.Rune != 0 || i == rowStart || preview.Cells[i-1].Continuation || renderer.RuneWidth(preview.Cells[i-1].Rune) != 2 { |
| 107 | return ErrInvalidRemotePreview | 111 | return ErrInvalidRemotePreview | ||
| 108 | } | 112 | } | ||
| 109 | - | } else if renderer.RuneWidth(cell.Rune) == 2 && (i+1 >= len(preview.Cells) || !preview.Cells[i+1].Continuation) { | 113 | + | } else if renderer.RuneWidth(cell.Rune) == 2 && (i+1 >= rowEnd || !preview.Cells[i+1].Continuation) { |
| 110 | return ErrInvalidRemotePreview | 114 | return ErrInvalidRemotePreview | ||
| 111 | } | 115 | } | ||
| 112 | } | 116 | } | ||
M internal/ports/remote_preview_hardening_test.go +16 -5
| @@ -10,6 +10,16 @@ import ( | |||
| 10 | 10 | "github.com/bnema/vev/pkg/renderer" | |
| 11 | 11 | ) | |
| 12 | 12 | ||
| 13 | + | const ( | |
| 14 | + | remotePreviewGoldenStatusOffset = 2 | |
| 15 | + | remotePreviewGoldenCellsOffset = 2 + 1 + 16 + 2 + len("tab-1") + 8 + 2 + 2 + 4 | |
| 16 | + | remotePreviewGoldenCellFlagsOffset = remotePreviewGoldenCellsOffset + 4 | |
| 17 | + | remotePreviewGoldenStyleFlagsOffset = remotePreviewGoldenCellsOffset + 5 | |
| 18 | + | remotePreviewGoldenStyleAttrsOffset = remotePreviewGoldenCellsOffset + 6 | |
| 19 | + | remotePreviewGoldenSecondCellOffset = remotePreviewGoldenCellsOffset + previewCellWireSize | |
| 20 | + | remotePreviewGoldenUnderlineStyleOffset = remotePreviewGoldenSecondCellOffset + 12 | |
| 21 | + | ) | |
| 22 | + | ||
| 13 | 23 | func remotePreviewGoldenPayload() []byte { | |
| 14 | 24 | return []byte{ | |
| 15 | 25 | 0x00, 0x01, 0x00, | |
| @@ -56,11 +66,11 @@ func TestRemotePreviewCodecRejectsInvalidStatusFlagsAndRanges(t *testing.T) { | |||
| 56 | 66 | name string | |
| 57 | 67 | mutate func([]byte) | |
| 58 | 68 | }{ | |
| 59 | - | {name: "status", mutate: func(b []byte) { b[2] = 0xff }}, | |
| 60 | - | {name: "cell flags", mutate: func(b []byte) { b[46] = 0x02 }}, | |
| 61 | - | {name: "style flags", mutate: func(b []byte) { b[47] = 0x80 }}, | |
| 62 | - | {name: "style attrs", mutate: func(b []byte) { b[48] = 0x80 }}, | |
| 63 | - | {name: "underline range", mutate: func(b []byte) { b[54] = byte(renderer.UnderlineDashed + 1) }}, | |
| 69 | + | {name: "status", mutate: func(b []byte) { b[remotePreviewGoldenStatusOffset] = 0xff }}, | |
| 70 | + | {name: "cell flags", mutate: func(b []byte) { b[remotePreviewGoldenCellFlagsOffset] = 0x02 }}, | |
| 71 | + | {name: "style flags", mutate: func(b []byte) { b[remotePreviewGoldenStyleFlagsOffset] = 0x80 }}, | |
| 72 | + | {name: "style attrs", mutate: func(b []byte) { b[remotePreviewGoldenStyleAttrsOffset] = 0x80 }}, | |
| 73 | + | {name: "underline range", mutate: func(b []byte) { b[remotePreviewGoldenUnderlineStyleOffset] = byte(renderer.UnderlineDashed + 1) }}, | |
| 64 | 74 | } | |
| 65 | 75 | for _, test := range tests { | |
| 66 | 76 | t.Run(test.name, func(t *testing.T) { | |
| @@ -88,6 +98,7 @@ func TestRemotePreviewCodecRejectsWideCellBoundaryAndInvalidIDs(t *testing.T) { | |||
| 88 | 98 | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 1, Height: 1, Cells: []renderer.Cell{{Rune: '界'}}}, | |
| 89 | 99 | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 2, Height: 1, Cells: []renderer.Cell{{Continuation: true}, {Rune: 'x'}}}, | |
| 90 | 100 | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 2, Height: 1, Cells: []renderer.Cell{{Rune: '界'}, {Continuation: true, Rune: 'x'}}}, | |
| 101 | + | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 2, Height: 2, Cells: []renderer.Cell{{Rune: 'x'}, {Rune: '界'}, {Continuation: true}, {Rune: 'y'}}}, | |
| 91 | 102 | } { | |
| 92 | 103 | if MarshalRemotePreview(malformed) != nil { | |
| 93 | 104 | t.Fatalf("malformed wide-cell preview marshaled: %#v", malformed.Cells) | |
| @@ -10,6 +10,16 @@ import ( | |||||
| 10 | "github.com/bnema/vev/pkg/renderer" | 10 | "github.com/bnema/vev/pkg/renderer" | ||
| 11 | ) | 11 | ) | ||
| 12 | 12 | ||||
| 13 | + | const ( | |||
| 14 | + | remotePreviewGoldenStatusOffset = 2 | |||
| 15 | + | remotePreviewGoldenCellsOffset = 2 + 1 + 16 + 2 + len("tab-1") + 8 + 2 + 2 + 4 | |||
| 16 | + | remotePreviewGoldenCellFlagsOffset = remotePreviewGoldenCellsOffset + 4 | |||
| 17 | + | remotePreviewGoldenStyleFlagsOffset = remotePreviewGoldenCellsOffset + 5 | |||
| 18 | + | remotePreviewGoldenStyleAttrsOffset = remotePreviewGoldenCellsOffset + 6 | |||
| 19 | + | remotePreviewGoldenSecondCellOffset = remotePreviewGoldenCellsOffset + previewCellWireSize | |||
| 20 | + | remotePreviewGoldenUnderlineStyleOffset = remotePreviewGoldenSecondCellOffset + 12 | |||
| 21 | + | ) | |||
| 22 | + | ||||
| 13 | func remotePreviewGoldenPayload() []byte { | 23 | func remotePreviewGoldenPayload() []byte { | ||
| 14 | return []byte{ | 24 | return []byte{ | ||
| 15 | 0x00, 0x01, 0x00, | 25 | 0x00, 0x01, 0x00, | ||
| @@ -56,11 +66,11 @@ func TestRemotePreviewCodecRejectsInvalidStatusFlagsAndRanges(t *testing.T) { | |||||
| 56 | name string | 66 | name string | ||
| 57 | mutate func([]byte) | 67 | mutate func([]byte) | ||
| 58 | }{ | 68 | }{ | ||
| 59 | - | {name: "status", mutate: func(b []byte) { b[2] = 0xff }}, | 69 | + | {name: "status", mutate: func(b []byte) { b[remotePreviewGoldenStatusOffset] = 0xff }}, |
| 60 | - | {name: "cell flags", mutate: func(b []byte) { b[46] = 0x02 }}, | 70 | + | {name: "cell flags", mutate: func(b []byte) { b[remotePreviewGoldenCellFlagsOffset] = 0x02 }}, |
| 61 | - | {name: "style flags", mutate: func(b []byte) { b[47] = 0x80 }}, | 71 | + | {name: "style flags", mutate: func(b []byte) { b[remotePreviewGoldenStyleFlagsOffset] = 0x80 }}, |
| 62 | - | {name: "style attrs", mutate: func(b []byte) { b[48] = 0x80 }}, | 72 | + | {name: "style attrs", mutate: func(b []byte) { b[remotePreviewGoldenStyleAttrsOffset] = 0x80 }}, |
| 63 | - | {name: "underline range", mutate: func(b []byte) { b[54] = byte(renderer.UnderlineDashed + 1) }}, | 73 | + | {name: "underline range", mutate: func(b []byte) { b[remotePreviewGoldenUnderlineStyleOffset] = byte(renderer.UnderlineDashed + 1) }}, |
| 64 | } | 74 | } | ||
| 65 | for _, test := range tests { | 75 | for _, test := range tests { | ||
| 66 | t.Run(test.name, func(t *testing.T) { | 76 | t.Run(test.name, func(t *testing.T) { | ||
| @@ -88,6 +98,7 @@ func TestRemotePreviewCodecRejectsWideCellBoundaryAndInvalidIDs(t *testing.T) { | |||||
| 88 | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 1, Height: 1, Cells: []renderer.Cell{{Rune: '界'}}}, | 98 | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 1, Height: 1, Cells: []renderer.Cell{{Rune: '界'}}}, | ||
| 89 | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 2, Height: 1, Cells: []renderer.Cell{{Continuation: true}, {Rune: 'x'}}}, | 99 | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 2, Height: 1, Cells: []renderer.Cell{{Continuation: true}, {Rune: 'x'}}}, | ||
| 90 | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 2, Height: 1, Cells: []renderer.Cell{{Rune: '界'}, {Continuation: true, Rune: 'x'}}}, | 100 | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 2, Height: 1, Cells: []renderer.Cell{{Rune: '界'}, {Continuation: true, Rune: 'x'}}}, | ||
| 101 | + | {Version: RemotePreviewSchemaVersion, Status: RemotePreviewOK, LifecycleID: lifecycle, TabID: "tab-1", Revision: 1, Width: 2, Height: 2, Cells: []renderer.Cell{{Rune: 'x'}, {Rune: '界'}, {Continuation: true}, {Rune: 'y'}}}, | |||
| 91 | } { | 102 | } { | ||
| 92 | if MarshalRemotePreview(malformed) != nil { | 103 | if MarshalRemotePreview(malformed) != nil { | ||
| 93 | t.Fatalf("malformed wide-cell preview marshaled: %#v", malformed.Cells) | 104 | t.Fatalf("malformed wide-cell preview marshaled: %#v", malformed.Cells) | ||
M internal/ports/remote_target_wire_test.go +8 -1
| @@ -29,7 +29,11 @@ func testRemoteTarget(stopped bool) *domain.RemoteSessionTarget { | |||
| 29 | 29 | ||
| 30 | 30 | func TestRemoteTargetWireRoundTripPreservesExactSelector(t *testing.T) { | |
| 31 | 31 | for _, stopped := range []bool{false, true} { | |
| 32 | - | t.Run(map[bool]string{false: "live", true: "stopped"}[stopped], func(t *testing.T) { | |
| 32 | + | name := "live" | |
| 33 | + | if stopped { | |
| 34 | + | name = "stopped" | |
| 35 | + | } | |
| 36 | + | t.Run(name, func(t *testing.T) { | |
| 33 | 37 | target := testRemoteTarget(stopped) | |
| 34 | 38 | msg := Hello{ | |
| 35 | 39 | Version: ProtocolVersion, | |
| @@ -61,6 +65,9 @@ func TestRemoteTargetWireRoundTripPreservesExactSelector(t *testing.T) { | |||
| 61 | 65 | RemoteTarget: target, | |
| 62 | 66 | EnvironmentPolicy: EnvironmentPolicyDaemonOwned, | |
| 63 | 67 | }) | |
| 68 | + | if attachPayload == nil { | |
| 69 | + | t.Fatal("MarshalAttachTarget returned nil") | |
| 70 | + | } | |
| 64 | 71 | attach, err := UnmarshalAttachTarget(attachPayload) | |
| 65 | 72 | if err != nil { | |
| 66 | 73 | t.Fatal(err) | |
| @@ -29,7 +29,11 @@ func testRemoteTarget(stopped bool) *domain.RemoteSessionTarget { | |||||
| 29 | 29 | ||||
| 30 | func TestRemoteTargetWireRoundTripPreservesExactSelector(t *testing.T) { | 30 | func TestRemoteTargetWireRoundTripPreservesExactSelector(t *testing.T) { | ||
| 31 | for _, stopped := range []bool{false, true} { | 31 | for _, stopped := range []bool{false, true} { | ||
| 32 | - | t.Run(map[bool]string{false: "live", true: "stopped"}[stopped], func(t *testing.T) { | 32 | + | name := "live" |
| 33 | + | if stopped { | |||
| 34 | + | name = "stopped" | |||
| 35 | + | } | |||
| 36 | + | t.Run(name, func(t *testing.T) { | |||
| 33 | target := testRemoteTarget(stopped) | 37 | target := testRemoteTarget(stopped) | ||
| 34 | msg := Hello{ | 38 | msg := Hello{ | ||
| 35 | Version: ProtocolVersion, | 39 | Version: ProtocolVersion, | ||
| @@ -61,6 +65,9 @@ func TestRemoteTargetWireRoundTripPreservesExactSelector(t *testing.T) { | |||||
| 61 | RemoteTarget: target, | 65 | RemoteTarget: target, | ||
| 62 | EnvironmentPolicy: EnvironmentPolicyDaemonOwned, | 66 | EnvironmentPolicy: EnvironmentPolicyDaemonOwned, | ||
| 63 | }) | 67 | }) | ||
| 68 | + | if attachPayload == nil { | |||
| 69 | + | t.Fatal("MarshalAttachTarget returned nil") | |||
| 70 | + | } | |||
| 64 | attach, err := UnmarshalAttachTarget(attachPayload) | 71 | attach, err := UnmarshalAttachTarget(attachPayload) | ||
| 65 | if err != nil { | 72 | if err != nil { | ||
| 66 | t.Fatal(err) | 73 | t.Fatal(err) | ||
M internal/usecase/client/client.go +24 -12
| @@ -226,20 +226,28 @@ func (r *Runner) terminalInput() *terminalInputPump { | |||
| 226 | 226 | return r.input | |
| 227 | 227 | } | |
| 228 | 228 | ||
| 229 | + | func validateAttachRequest(request AttachRequest) error { | |
| 230 | + | if request.RemoteTarget == nil { | |
| 231 | + | return nil | |
| 232 | + | } | |
| 233 | + | if err := request.RemoteTarget.Validate(); err != nil { | |
| 234 | + | return fmt.Errorf("vev: invalid remote attach target: %w", err) | |
| 235 | + | } | |
| 236 | + | if request.SessionName != request.RemoteTarget.SessionName { | |
| 237 | + | return errors.New("vev: remote attach target session mismatch") | |
| 238 | + | } | |
| 239 | + | if request.EnvironmentPolicy != ports.EnvironmentPolicyClientOwned && request.EnvironmentPolicy != ports.EnvironmentPolicyDaemonOwned { | |
| 240 | + | return errors.New("vev: invalid remote environment policy") | |
| 241 | + | } | |
| 242 | + | return nil | |
| 243 | + | } | |
| 244 | + | ||
| 229 | 245 | // Run connects and runs the attach client. It owns the terminal lifecycle | |
| 230 | 246 | // above attach attempts so raw mode remains active while a live client process | |
| 231 | 247 | // redials a lost link. | |
| 232 | 248 | func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) { | |
| 233 | - | if request.RemoteTarget != nil { | |
| 234 | - | if err := request.RemoteTarget.Validate(); err != nil { | |
| 235 | - | return fmt.Errorf("vev: invalid remote attach target: %w", err) | |
| 236 | - | } | |
| 237 | - | if request.SessionName != request.RemoteTarget.SessionName { | |
| 238 | - | return errors.New("vev: remote attach target session mismatch") | |
| 239 | - | } | |
| 240 | - | if request.EnvironmentPolicy != ports.EnvironmentPolicyClientOwned && request.EnvironmentPolicy != ports.EnvironmentPolicyDaemonOwned { | |
| 241 | - | return errors.New("vev: invalid remote environment policy") | |
| 242 | - | } | |
| 249 | + | if err := validateAttachRequest(request); err != nil { | |
| 250 | + | return err | |
| 243 | 251 | } | |
| 244 | 252 | ms := milestones{} | |
| 245 | 253 | ||
| @@ -267,6 +275,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 267 | 275 | ||
| 268 | 276 | resumeToken := uint64(0) | |
| 269 | 277 | attemptRequest := request | |
| 278 | + | dialer := r.dialer | |
| 270 | 279 | backoff := defaultReconnectBackoff.initial | |
| 271 | 280 | themeState := &terminalThemeState{} | |
| 272 | 281 | var rememberOnce sync.Once | |
| @@ -335,7 +344,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 335 | 344 | ||
| 336 | 345 | for { | |
| 337 | 346 | handshakeCtx, timedOut, finishHandshake := newHandshakeContext(ctx, r.clock) | |
| 338 | - | transport, err := boundedDial(handshakeCtx, r.dialer) | |
| 347 | + | transport, err := boundedDial(handshakeCtx, dialer) | |
| 339 | 348 | if err != nil { | |
| 340 | 349 | err = handshakeContextError(ctx, timedOut, err) | |
| 341 | 350 | finishHandshake() | |
| @@ -421,10 +430,13 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 421 | 430 | if handoffErr != nil { | |
| 422 | 431 | return handoffErr | |
| 423 | 432 | } | |
| 433 | + | if err := validateAttachRequest(nextRequest); err != nil { | |
| 434 | + | return fmt.Errorf("vev: invalid remote attach handoff request: %w", err) | |
| 435 | + | } | |
| 424 | 436 | if nextDialer == nil { | |
| 425 | 437 | return errors.New("vev: remote attach handoff returned nil dialer") | |
| 426 | 438 | } | |
| 427 | - | r.dialer = nextDialer | |
| 439 | + | dialer = nextDialer | |
| 428 | 440 | attemptRequest = nextRequest | |
| 429 | 441 | remote = attemptRequest.Remote || r.remote | |
| 430 | 442 | resumeToken = 0 | |
| @@ -226,20 +226,28 @@ func (r *Runner) terminalInput() *terminalInputPump { | |||||
| 226 | return r.input | 226 | return r.input | ||
| 227 | } | 227 | } | ||
| 228 | 228 | ||||
| 229 | + | func validateAttachRequest(request AttachRequest) error { | |||
| 230 | + | if request.RemoteTarget == nil { | |||
| 231 | + | return nil | |||
| 232 | + | } | |||
| 233 | + | if err := request.RemoteTarget.Validate(); err != nil { | |||
| 234 | + | return fmt.Errorf("vev: invalid remote attach target: %w", err) | |||
| 235 | + | } | |||
| 236 | + | if request.SessionName != request.RemoteTarget.SessionName { | |||
| 237 | + | return errors.New("vev: remote attach target session mismatch") | |||
| 238 | + | } | |||
| 239 | + | if request.EnvironmentPolicy != ports.EnvironmentPolicyClientOwned && request.EnvironmentPolicy != ports.EnvironmentPolicyDaemonOwned { | |||
| 240 | + | return errors.New("vev: invalid remote environment policy") | |||
| 241 | + | } | |||
| 242 | + | return nil | |||
| 243 | + | } | |||
| 244 | + | ||||
| 229 | // Run connects and runs the attach client. It owns the terminal lifecycle | 245 | // Run connects and runs the attach client. It owns the terminal lifecycle | ||
| 230 | // above attach attempts so raw mode remains active while a live client process | 246 | // above attach attempts so raw mode remains active while a live client process | ||
| 231 | // redials a lost link. | 247 | // redials a lost link. | ||
| 232 | func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) { | 248 | func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) { | ||
| 233 | - | if request.RemoteTarget != nil { | 249 | + | if err := validateAttachRequest(request); err != nil { |
| 234 | - | if err := request.RemoteTarget.Validate(); err != nil { | 250 | + | return err |
| 235 | - | return fmt.Errorf("vev: invalid remote attach target: %w", err) | |||
| 236 | - | } | |||
| 237 | - | if request.SessionName != request.RemoteTarget.SessionName { | |||
| 238 | - | return errors.New("vev: remote attach target session mismatch") | |||
| 239 | - | } | |||
| 240 | - | if request.EnvironmentPolicy != ports.EnvironmentPolicyClientOwned && request.EnvironmentPolicy != ports.EnvironmentPolicyDaemonOwned { | |||
| 241 | - | return errors.New("vev: invalid remote environment policy") | |||
| 242 | - | } | |||
| 243 | } | 251 | } | ||
| 244 | ms := milestones{} | 252 | ms := milestones{} | ||
| 245 | 253 | ||||
| @@ -267,6 +275,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 267 | 275 | ||||
| 268 | resumeToken := uint64(0) | 276 | resumeToken := uint64(0) | ||
| 269 | attemptRequest := request | 277 | attemptRequest := request | ||
| 278 | + | dialer := r.dialer | |||
| 270 | backoff := defaultReconnectBackoff.initial | 279 | backoff := defaultReconnectBackoff.initial | ||
| 271 | themeState := &terminalThemeState{} | 280 | themeState := &terminalThemeState{} | ||
| 272 | var rememberOnce sync.Once | 281 | var rememberOnce sync.Once | ||
| @@ -335,7 +344,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 335 | 344 | ||||
| 336 | for { | 345 | for { | ||
| 337 | handshakeCtx, timedOut, finishHandshake := newHandshakeContext(ctx, r.clock) | 346 | handshakeCtx, timedOut, finishHandshake := newHandshakeContext(ctx, r.clock) | ||
| 338 | - | transport, err := boundedDial(handshakeCtx, r.dialer) | 347 | + | transport, err := boundedDial(handshakeCtx, dialer) |
| 339 | if err != nil { | 348 | if err != nil { | ||
| 340 | err = handshakeContextError(ctx, timedOut, err) | 349 | err = handshakeContextError(ctx, timedOut, err) | ||
| 341 | finishHandshake() | 350 | finishHandshake() | ||
| @@ -421,10 +430,13 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 421 | if handoffErr != nil { | 430 | if handoffErr != nil { | ||
| 422 | return handoffErr | 431 | return handoffErr | ||
| 423 | } | 432 | } | ||
| 433 | + | if err := validateAttachRequest(nextRequest); err != nil { | |||
| 434 | + | return fmt.Errorf("vev: invalid remote attach handoff request: %w", err) | |||
| 435 | + | } | |||
| 424 | if nextDialer == nil { | 436 | if nextDialer == nil { | ||
| 425 | return errors.New("vev: remote attach handoff returned nil dialer") | 437 | return errors.New("vev: remote attach handoff returned nil dialer") | ||
| 426 | } | 438 | } | ||
| 427 | - | r.dialer = nextDialer | 439 | + | dialer = nextDialer |
| 428 | attemptRequest = nextRequest | 440 | attemptRequest = nextRequest | ||
| 429 | remote = attemptRequest.Remote || r.remote | 441 | remote = attemptRequest.Remote || r.remote | ||
| 430 | resumeToken = 0 | 442 | resumeToken = 0 | ||
M internal/usecase/daemon/control_test.go +1 -1
| @@ -797,7 +797,7 @@ func TestRemoteCatalogTabCountSaturates(t *testing.T) { | |||
| 797 | 797 | var catalog ports.RemoteCatalog | |
| 798 | 798 | require.NoError(t, json.Unmarshal([]byte(result.Output), &catalog)) | |
| 799 | 799 | require.Equal(t, []ports.RemoteCatalogSession{{ | |
| 800 | - | Name: "work", State: "running", Ephemeral: true, Tabs: ^uint16(0), Attached: false, | |
| 800 | + | Name: "work", State: "running", Ephemeral: true, Tabs: int(^uint16(0)), Attached: false, | |
| 801 | 801 | }}, catalog.Sessions) | |
| 802 | 802 | } | |
| 803 | 803 | ||
| @@ -797,7 +797,7 @@ func TestRemoteCatalogTabCountSaturates(t *testing.T) { | |||||
| 797 | var catalog ports.RemoteCatalog | 797 | var catalog ports.RemoteCatalog | ||
| 798 | require.NoError(t, json.Unmarshal([]byte(result.Output), &catalog)) | 798 | require.NoError(t, json.Unmarshal([]byte(result.Output), &catalog)) | ||
| 799 | require.Equal(t, []ports.RemoteCatalogSession{{ | 799 | require.Equal(t, []ports.RemoteCatalogSession{{ | ||
| 800 | - | Name: "work", State: "running", Ephemeral: true, Tabs: ^uint16(0), Attached: false, | 800 | + | Name: "work", State: "running", Ephemeral: true, Tabs: int(^uint16(0)), Attached: false, |
| 801 | }}, catalog.Sessions) | 801 | }}, catalog.Sessions) | ||
| 802 | } | 802 | } | ||
| 803 | 803 | ||||
M internal/usecase/daemon/picker.go +4 -1
| @@ -24,6 +24,8 @@ type pickerSortMode uint32 | |||
| 24 | 24 | const ( | |
| 25 | 25 | pickerSortRecent pickerSortMode = iota | |
| 26 | 26 | pickerSortGrouped | |
| 27 | + | ||
| 28 | + | remotePickerPreviewDebounce = 80 * time.Millisecond | |
| 27 | 29 | ) | |
| 28 | 30 | ||
| 29 | 31 | func pickerTitle(mode pickerSortMode) string { | |
| @@ -517,11 +519,12 @@ func (d *Daemon) startRemotePickerPreview(ac *attachedClient, target picker.Targ | |||
| 517 | 519 | } | |
| 518 | 520 | ac.overlays.pickerMu.Unlock() | |
| 519 | 521 | go func() { | |
| 522 | + | defer cancel() | |
| 520 | 523 | previewClock := d.clock | |
| 521 | 524 | if previewClock == nil { | |
| 522 | 525 | previewClock = systemClock{} | |
| 523 | 526 | } | |
| 524 | - | debounce := previewClock.NewTimer(80 * time.Millisecond) | |
| 527 | + | debounce := previewClock.NewTimer(remotePickerPreviewDebounce) | |
| 525 | 528 | defer debounce.Stop() | |
| 526 | 529 | select { | |
| 527 | 530 | case <-debounce.C(): | |
| @@ -24,6 +24,8 @@ type pickerSortMode uint32 | |||||
| 24 | const ( | 24 | const ( | ||
| 25 | pickerSortRecent pickerSortMode = iota | 25 | pickerSortRecent pickerSortMode = iota | ||
| 26 | pickerSortGrouped | 26 | pickerSortGrouped | ||
| 27 | + | ||||
| 28 | + | remotePickerPreviewDebounce = 80 * time.Millisecond | |||
| 27 | ) | 29 | ) | ||
| 28 | 30 | ||||
| 29 | func pickerTitle(mode pickerSortMode) string { | 31 | func pickerTitle(mode pickerSortMode) string { | ||
| @@ -517,11 +519,12 @@ func (d *Daemon) startRemotePickerPreview(ac *attachedClient, target picker.Targ | |||||
| 517 | } | 519 | } | ||
| 518 | ac.overlays.pickerMu.Unlock() | 520 | ac.overlays.pickerMu.Unlock() | ||
| 519 | go func() { | 521 | go func() { | ||
| 522 | + | defer cancel() | |||
| 520 | previewClock := d.clock | 523 | previewClock := d.clock | ||
| 521 | if previewClock == nil { | 524 | if previewClock == nil { | ||
| 522 | previewClock = systemClock{} | 525 | previewClock = systemClock{} | ||
| 523 | } | 526 | } | ||
| 524 | - | debounce := previewClock.NewTimer(80 * time.Millisecond) | 527 | + | debounce := previewClock.NewTimer(remotePickerPreviewDebounce) |
| 525 | defer debounce.Stop() | 528 | defer debounce.Stop() | ||
| 526 | select { | 529 | select { | ||
| 527 | case <-debounce.C(): | 530 | case <-debounce.C(): | ||
M internal/usecase/daemon/remote_attach.go +6 -4
| @@ -3,7 +3,6 @@ package daemon | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | - | "os" | |
| 7 | 6 | ||
| 8 | 7 | "github.com/bnema/vev/internal/domain" | |
| 9 | 8 | "github.com/bnema/vev/internal/ports" | |
| @@ -70,7 +69,7 @@ func (d *Daemon) routeRemoteTargetWithContext(ctx context.Context, h ports.Hello | |||
| 70 | 69 | // Picker handoffs use the daemon's own environment and persisted CWD. The | |
| 71 | 70 | // client's Env/Cwd fields remain present for wire compatibility but are not | |
| 72 | 71 | // trusted for this branch. | |
| 73 | - | env := copyEnvironment(os.Environ()) | |
| 72 | + | env := copyEnvironment(d.baseEnv) | |
| 74 | 73 | cwd := d.dirOrHome(stopped.cwd) | |
| 75 | 74 | sess, err := d.createSessionLockedWithMode(target.SessionName, false, cwd, h.Size, terminalEnv{TrueColor: h.TrueColor}, env, stopped.tabNames) | |
| 76 | 75 | if err != nil { | |
| @@ -89,12 +88,15 @@ func (d *Daemon) routeRemoteTargetWithContext(ctx context.Context, h ports.Hello | |||
| 89 | 88 | } | |
| 90 | 89 | ||
| 91 | 90 | func (d *Daemon) remoteTargetMatchesSession(sess *session, target domain.RemoteSessionTarget) bool { | |
| 92 | - | if sess == nil || sess.incarnation != target.LifecycleID || sess.name != target.SessionName { | |
| 91 | + | if sess == nil { | |
| 93 | 92 | return false | |
| 94 | 93 | } | |
| 95 | 94 | d.mu.Lock() | |
| 96 | 95 | defer d.mu.Unlock() | |
| 97 | - | if d.sessions[sess.id] != sess { | |
| 96 | + | sess.mu.Lock() | |
| 97 | + | matches := d.sessions[sess.id] == sess && sess.incarnation == target.LifecycleID && sess.name == target.SessionName | |
| 98 | + | sess.mu.Unlock() | |
| 99 | + | if !matches { | |
| 98 | 100 | return false | |
| 99 | 101 | } | |
| 100 | 102 | _, ok := remoteTargetTabIndexLocked(sess, target) | |
| @@ -3,7 +3,6 @@ package daemon | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | - | "os" | |||
| 7 | 6 | ||||
| 8 | "github.com/bnema/vev/internal/domain" | 7 | "github.com/bnema/vev/internal/domain" | ||
| 9 | "github.com/bnema/vev/internal/ports" | 8 | "github.com/bnema/vev/internal/ports" | ||
| @@ -70,7 +69,7 @@ func (d *Daemon) routeRemoteTargetWithContext(ctx context.Context, h ports.Hello | |||||
| 70 | // Picker handoffs use the daemon's own environment and persisted CWD. The | 69 | // Picker handoffs use the daemon's own environment and persisted CWD. The | ||
| 71 | // client's Env/Cwd fields remain present for wire compatibility but are not | 70 | // client's Env/Cwd fields remain present for wire compatibility but are not | ||
| 72 | // trusted for this branch. | 71 | // trusted for this branch. | ||
| 73 | - | env := copyEnvironment(os.Environ()) | 72 | + | env := copyEnvironment(d.baseEnv) |
| 74 | cwd := d.dirOrHome(stopped.cwd) | 73 | cwd := d.dirOrHome(stopped.cwd) | ||
| 75 | sess, err := d.createSessionLockedWithMode(target.SessionName, false, cwd, h.Size, terminalEnv{TrueColor: h.TrueColor}, env, stopped.tabNames) | 74 | sess, err := d.createSessionLockedWithMode(target.SessionName, false, cwd, h.Size, terminalEnv{TrueColor: h.TrueColor}, env, stopped.tabNames) | ||
| 76 | if err != nil { | 75 | if err != nil { | ||
| @@ -89,12 +88,15 @@ func (d *Daemon) routeRemoteTargetWithContext(ctx context.Context, h ports.Hello | |||||
| 89 | } | 88 | } | ||
| 90 | 89 | ||||
| 91 | func (d *Daemon) remoteTargetMatchesSession(sess *session, target domain.RemoteSessionTarget) bool { | 90 | func (d *Daemon) remoteTargetMatchesSession(sess *session, target domain.RemoteSessionTarget) bool { | ||
| 92 | - | if sess == nil || sess.incarnation != target.LifecycleID || sess.name != target.SessionName { | 91 | + | if sess == nil { |
| 93 | return false | 92 | return false | ||
| 94 | } | 93 | } | ||
| 95 | d.mu.Lock() | 94 | d.mu.Lock() | ||
| 96 | defer d.mu.Unlock() | 95 | defer d.mu.Unlock() | ||
| 97 | - | if d.sessions[sess.id] != sess { | 96 | + | sess.mu.Lock() |
| 97 | + | matches := d.sessions[sess.id] == sess && sess.incarnation == target.LifecycleID && sess.name == target.SessionName | |||
| 98 | + | sess.mu.Unlock() | |||
| 99 | + | if !matches { | |||
| 98 | return false | 100 | return false | ||
| 99 | } | 101 | } | ||
| 100 | _, ok := remoteTargetTabIndexLocked(sess, target) | 102 | _, ok := remoteTargetTabIndexLocked(sess, target) | ||
M internal/usecase/daemon/remote_attach_test.go +5 -3
| @@ -49,9 +49,11 @@ func TestRemotePickerRichHandoffCarriesLifecycleTabAndPolicy(t *testing.T) { | |||
| 49 | 49 | func TestRemotePickerRichHandoffRejectsReplacedLifecycle(t *testing.T) { | |
| 50 | 50 | d := newRemotePickerDaemon(nil) | |
| 51 | 51 | lifecycle := remoteLifecycleForTest() | |
| 52 | + | cachedLifecycle := lifecycle | |
| 53 | + | cachedLifecycle[0]++ | |
| 52 | 54 | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |
| 53 | 55 | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | |
| 54 | - | LifecycleID: lifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |
| 56 | + | LifecycleID: cachedLifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |
| 55 | 57 | }}, | |
| 56 | 58 | }}) | |
| 57 | 59 | d.remoteCatalog.mu.Lock() | |
| @@ -64,7 +66,7 @@ func TestRemotePickerRichHandoffRejectsReplacedLifecycle(t *testing.T) { | |||
| 64 | 66 | token.effect = effect | |
| 65 | 67 | defer effect.End() | |
| 66 | 68 | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle, DisplayOrigin: "arch"} | |
| 67 | - | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "replaced"} | |
| 68 | - | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "replaced"} | |
| 69 | + | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | |
| 70 | + | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "tab-1"} | |
| 69 | 71 | require.ErrorIs(t, d.sendRemoteAttachTargetForAttachment(token, target, key, sessionHandoffGuard{}, "picker-select"), errAttachmentTransition) | |
| 70 | 72 | } | |
| @@ -49,9 +49,11 @@ func TestRemotePickerRichHandoffCarriesLifecycleTabAndPolicy(t *testing.T) { | |||||
| 49 | func TestRemotePickerRichHandoffRejectsReplacedLifecycle(t *testing.T) { | 49 | func TestRemotePickerRichHandoffRejectsReplacedLifecycle(t *testing.T) { | ||
| 50 | d := newRemotePickerDaemon(nil) | 50 | d := newRemotePickerDaemon(nil) | ||
| 51 | lifecycle := remoteLifecycleForTest() | 51 | lifecycle := remoteLifecycleForTest() | ||
| 52 | + | cachedLifecycle := lifecycle | |||
| 53 | + | cachedLifecycle[0]++ | |||
| 52 | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | 54 | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | ||
| 53 | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | 55 | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | ||
| 54 | - | LifecycleID: lifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | 56 | + | LifecycleID: cachedLifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, |
| 55 | }}, | 57 | }}, | ||
| 56 | }}) | 58 | }}) | ||
| 57 | d.remoteCatalog.mu.Lock() | 59 | d.remoteCatalog.mu.Lock() | ||
| @@ -64,7 +66,7 @@ func TestRemotePickerRichHandoffRejectsReplacedLifecycle(t *testing.T) { | |||||
| 64 | token.effect = effect | 66 | token.effect = effect | ||
| 65 | defer effect.End() | 67 | defer effect.End() | ||
| 66 | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle, DisplayOrigin: "arch"} | 68 | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle, DisplayOrigin: "arch"} | ||
| 67 | - | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "replaced"} | 69 | + | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} |
| 68 | - | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "replaced"} | 70 | + | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "tab-1"} |
| 69 | require.ErrorIs(t, d.sendRemoteAttachTargetForAttachment(token, target, key, sessionHandoffGuard{}, "picker-select"), errAttachmentTransition) | 71 | require.ErrorIs(t, d.sendRemoteAttachTargetForAttachment(token, target, key, sessionHandoffGuard{}, "picker-select"), errAttachmentTransition) | ||
| 70 | } | 72 | } | ||
M internal/usecase/daemon/remote_catalog_state.go +2 -1
| @@ -2,6 +2,7 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "slices" | |
| 5 | 6 | "sort" | |
| 6 | 7 | "sync" | |
| 7 | 8 | "time" | |
| @@ -48,7 +49,7 @@ func cloneRemoteCatalogEntry(entry ports.RemoteCatalogCacheEntry) ports.RemoteCa | |||
| 48 | 49 | for i, session := range entry.Sessions { | |
| 49 | 50 | sessions[i] = session | |
| 50 | 51 | if tabs := ports.CatalogTabs(session); tabs != nil { | |
| 51 | - | sessions[i].Tabs = append([]ports.RemoteCatalogTab(nil), tabs...) | |
| 52 | + | sessions[i].Tabs = slices.Clone(tabs) | |
| 52 | 53 | } | |
| 53 | 54 | } | |
| 54 | 55 | return ports.RemoteCatalogCacheEntry{ | |
| @@ -2,6 +2,7 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "slices" | |||
| 5 | "sort" | 6 | "sort" | ||
| 6 | "sync" | 7 | "sync" | ||
| 7 | "time" | 8 | "time" | ||
| @@ -48,7 +49,7 @@ func cloneRemoteCatalogEntry(entry ports.RemoteCatalogCacheEntry) ports.RemoteCa | |||||
| 48 | for i, session := range entry.Sessions { | 49 | for i, session := range entry.Sessions { | ||
| 49 | sessions[i] = session | 50 | sessions[i] = session | ||
| 50 | if tabs := ports.CatalogTabs(session); tabs != nil { | 51 | if tabs := ports.CatalogTabs(session); tabs != nil { | ||
| 51 | - | sessions[i].Tabs = append([]ports.RemoteCatalogTab(nil), tabs...) | 52 | + | sessions[i].Tabs = slices.Clone(tabs) |
| 52 | } | 53 | } | ||
| 53 | } | 54 | } | ||
| 54 | return ports.RemoteCatalogCacheEntry{ | 55 | return ports.RemoteCatalogCacheEntry{ | ||
M internal/usecase/daemon/remote_picker.go +5 -2
| @@ -85,7 +85,7 @@ func (d *Daemon) remotePickerHostRanks() map[string]int { | |||
| 85 | 85 | } | |
| 86 | 86 | ||
| 87 | 87 | func remoteCatalogExpired(fetchedAt, now time.Time) bool { | |
| 88 | - | return fetchedAt.IsZero() || (!now.Before(fetchedAt) && now.Sub(fetchedAt) > remotePickerAttachTTL) | |
| 88 | + | return fetchedAt.IsZero() || (!now.Before(fetchedAt) && now.Sub(fetchedAt) >= remotePickerAttachTTL) | |
| 89 | 89 | } | |
| 90 | 90 | ||
| 91 | 91 | func remotePickerAvailability(status remoteHostStatus) picker.RemoteAvailability { | |
| @@ -522,7 +522,10 @@ func (d *Daemon) applyRemoteRefreshResult(generation uint64, host string, catalo | |||
| 522 | 522 | var mismatch *ports.RemoteCatalogVersionMismatchError | |
| 523 | 523 | if errors.As(listErr, &mismatch) { | |
| 524 | 524 | status = remoteHostVersionMismatch | |
| 525 | - | } else if !errors.Is(listErr, ports.ErrInvalidRemoteCatalog) && !errors.Is(listErr, ports.ErrRemoteCatalogTooLarge) { | |
| 525 | + | } else if !errors.Is(listErr, ports.ErrInvalidRemoteCatalog) && | |
| 526 | + | !errors.Is(listErr, ports.ErrRemoteCatalogTooLarge) && | |
| 527 | + | !errors.Is(listErr, ports.ErrRemoteCatalogUnknownState) && | |
| 528 | + | !errors.Is(listErr, ports.ErrRemoteCatalogInvalidReason) { | |
| 526 | 529 | status = remoteHostUnreachable | |
| 527 | 530 | } | |
| 528 | 531 | d.remoteCatalog.status[host] = status | |
| @@ -85,7 +85,7 @@ func (d *Daemon) remotePickerHostRanks() map[string]int { | |||||
| 85 | } | 85 | } | ||
| 86 | 86 | ||||
| 87 | func remoteCatalogExpired(fetchedAt, now time.Time) bool { | 87 | func remoteCatalogExpired(fetchedAt, now time.Time) bool { | ||
| 88 | - | return fetchedAt.IsZero() || (!now.Before(fetchedAt) && now.Sub(fetchedAt) > remotePickerAttachTTL) | 88 | + | return fetchedAt.IsZero() || (!now.Before(fetchedAt) && now.Sub(fetchedAt) >= remotePickerAttachTTL) |
| 89 | } | 89 | } | ||
| 90 | 90 | ||||
| 91 | func remotePickerAvailability(status remoteHostStatus) picker.RemoteAvailability { | 91 | func remotePickerAvailability(status remoteHostStatus) picker.RemoteAvailability { | ||
| @@ -522,7 +522,10 @@ func (d *Daemon) applyRemoteRefreshResult(generation uint64, host string, catalo | |||||
| 522 | var mismatch *ports.RemoteCatalogVersionMismatchError | 522 | var mismatch *ports.RemoteCatalogVersionMismatchError | ||
| 523 | if errors.As(listErr, &mismatch) { | 523 | if errors.As(listErr, &mismatch) { | ||
| 524 | status = remoteHostVersionMismatch | 524 | status = remoteHostVersionMismatch | ||
| 525 | - | } else if !errors.Is(listErr, ports.ErrInvalidRemoteCatalog) && !errors.Is(listErr, ports.ErrRemoteCatalogTooLarge) { | 525 | + | } else if !errors.Is(listErr, ports.ErrInvalidRemoteCatalog) && |
| 526 | + | !errors.Is(listErr, ports.ErrRemoteCatalogTooLarge) && | |||
| 527 | + | !errors.Is(listErr, ports.ErrRemoteCatalogUnknownState) && | |||
| 528 | + | !errors.Is(listErr, ports.ErrRemoteCatalogInvalidReason) { | |||
| 526 | status = remoteHostUnreachable | 529 | status = remoteHostUnreachable | ||
| 527 | } | 530 | } | ||
| 528 | d.remoteCatalog.status[host] = status | 531 | d.remoteCatalog.status[host] = status | ||
M internal/usecase/daemon/remote_picker_test.go +43 -0
| @@ -55,6 +55,27 @@ func newRemotePickerDaemon(store ports.RemoteHostStore) *Daemon { | |||
| 55 | 55 | return New(nil, stubClock{}, slog.New(slog.NewTextHandler(io.Discard, nil)), WithRemoteDiscovery(store, nil, nil, nil, ports.RemoteTransportUDP)) | |
| 56 | 56 | } | |
| 57 | 57 | ||
| 58 | + | func TestRemotePickerCatalogExpiresAtAttachTTL(t *testing.T) { | |
| 59 | + | now := time.Unix(100, 0) | |
| 60 | + | clock := &remotePreviewTestClock{now: now} | |
| 61 | + | d := newTestDaemon(t, nil, clock) | |
| 62 | + | lifecycle := remoteLifecycleForTest() | |
| 63 | + | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |
| 64 | + | Host: "arch", FetchedAt: now.Add(-remotePickerAttachTTL), Sessions: []ports.RemoteCatalogSession{{ | |
| 65 | + | LifecycleID: lifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |
| 66 | + | }}, | |
| 67 | + | }}) | |
| 68 | + | d.remoteCatalog.mu.Lock() | |
| 69 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 70 | + | d.remoteCatalog.mu.Unlock() | |
| 71 | + | ||
| 72 | + | entries := d.remotePickerCatalogSnapshot() | |
| 73 | + | require.Len(t, entries, 1) | |
| 74 | + | require.Equal(t, remoteHostStale, entries[0].status) | |
| 75 | + | target := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | |
| 76 | + | require.False(t, d.remotePickerTargetReadyTarget(target), "attachment must reject a catalogue exactly at the TTL boundary") | |
| 77 | + | } | |
| 78 | + | ||
| 58 | 79 | func TestRemotePickerAvailabilityUsesCachedRowsForFailures(t *testing.T) { | |
| 59 | 80 | tests := []struct { | |
| 60 | 81 | name string | |
| @@ -403,6 +424,28 @@ func TestRemoteRefreshRejectsRemovedHostAndLateGeneration(t *testing.T) { | |||
| 403 | 424 | d.remoteCatalog.mu.Unlock() | |
| 404 | 425 | } | |
| 405 | 426 | ||
| 427 | + | func TestRemoteRefreshValidationSentinelsRemainMalformed(t *testing.T) { | |
| 428 | + | lifecycle := remoteLifecycleForTest() | |
| 429 | + | for _, test := range []struct { | |
| 430 | + | name string | |
| 431 | + | session ports.RemoteCatalogSession | |
| 432 | + | }{ | |
| 433 | + | {name: "unknown state", session: ports.RemoteCatalogSession{LifecycleID: lifecycle, Name: "work", State: "unknown"}}, | |
| 434 | + | {name: "invalid reason", session: ports.RemoteCatalogSession{LifecycleID: lifecycle, Name: "work", State: "running", Reason: "bogus"}}, | |
| 435 | + | } { | |
| 436 | + | t.Run(test.name, func(t *testing.T) { | |
| 437 | + | hosts := &remoteRefreshHostStore{hosts: []string{"arch"}} | |
| 438 | + | d, _, _ := newRemoteRefreshDaemon(t, hosts, time.Unix(400, 0)) | |
| 439 | + | d.remoteCatalog.refresh = 1 | |
| 440 | + | catalog := ports.RemoteCatalog{ProtocolVersion: ports.ProtocolVersion, SchemaVersion: ports.RemoteCatalogSchemaVersion, Sessions: []ports.RemoteCatalogSession{test.session}} | |
| 441 | + | require.False(t, d.applyRemoteRefreshResult(1, "arch", catalog, nil)) | |
| 442 | + | d.remoteCatalog.mu.Lock() | |
| 443 | + | require.Equal(t, remoteHostMalformed, d.remoteCatalog.status["arch"]) | |
| 444 | + | d.remoteCatalog.mu.Unlock() | |
| 445 | + | }) | |
| 446 | + | } | |
| 447 | + | } | |
| 448 | + | ||
| 406 | 449 | func TestRemoteRefreshVersionMismatchPreservesStaleRows(t *testing.T) { | |
| 407 | 450 | hosts := &remoteRefreshHostStore{hosts: []string{"arch"}} | |
| 408 | 451 | d, _, _ := newRemoteRefreshDaemon(t, hosts, time.Unix(400, 0)) | |
| @@ -55,6 +55,27 @@ func newRemotePickerDaemon(store ports.RemoteHostStore) *Daemon { | |||||
| 55 | return New(nil, stubClock{}, slog.New(slog.NewTextHandler(io.Discard, nil)), WithRemoteDiscovery(store, nil, nil, nil, ports.RemoteTransportUDP)) | 55 | return New(nil, stubClock{}, slog.New(slog.NewTextHandler(io.Discard, nil)), WithRemoteDiscovery(store, nil, nil, nil, ports.RemoteTransportUDP)) | ||
| 56 | } | 56 | } | ||
| 57 | 57 | ||||
| 58 | + | func TestRemotePickerCatalogExpiresAtAttachTTL(t *testing.T) { | |||
| 59 | + | now := time.Unix(100, 0) | |||
| 60 | + | clock := &remotePreviewTestClock{now: now} | |||
| 61 | + | d := newTestDaemon(t, nil, clock) | |||
| 62 | + | lifecycle := remoteLifecycleForTest() | |||
| 63 | + | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |||
| 64 | + | Host: "arch", FetchedAt: now.Add(-remotePickerAttachTTL), Sessions: []ports.RemoteCatalogSession{{ | |||
| 65 | + | LifecycleID: lifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |||
| 66 | + | }}, | |||
| 67 | + | }}) | |||
| 68 | + | d.remoteCatalog.mu.Lock() | |||
| 69 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |||
| 70 | + | d.remoteCatalog.mu.Unlock() | |||
| 71 | + | ||||
| 72 | + | entries := d.remotePickerCatalogSnapshot() | |||
| 73 | + | require.Len(t, entries, 1) | |||
| 74 | + | require.Equal(t, remoteHostStale, entries[0].status) | |||
| 75 | + | target := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | |||
| 76 | + | require.False(t, d.remotePickerTargetReadyTarget(target), "attachment must reject a catalogue exactly at the TTL boundary") | |||
| 77 | + | } | |||
| 78 | + | ||||
| 58 | func TestRemotePickerAvailabilityUsesCachedRowsForFailures(t *testing.T) { | 79 | func TestRemotePickerAvailabilityUsesCachedRowsForFailures(t *testing.T) { | ||
| 59 | tests := []struct { | 80 | tests := []struct { | ||
| 60 | name string | 81 | name string | ||
| @@ -403,6 +424,28 @@ func TestRemoteRefreshRejectsRemovedHostAndLateGeneration(t *testing.T) { | |||||
| 403 | d.remoteCatalog.mu.Unlock() | 424 | d.remoteCatalog.mu.Unlock() | ||
| 404 | } | 425 | } | ||
| 405 | 426 | ||||
| 427 | + | func TestRemoteRefreshValidationSentinelsRemainMalformed(t *testing.T) { | |||
| 428 | + | lifecycle := remoteLifecycleForTest() | |||
| 429 | + | for _, test := range []struct { | |||
| 430 | + | name string | |||
| 431 | + | session ports.RemoteCatalogSession | |||
| 432 | + | }{ | |||
| 433 | + | {name: "unknown state", session: ports.RemoteCatalogSession{LifecycleID: lifecycle, Name: "work", State: "unknown"}}, | |||
| 434 | + | {name: "invalid reason", session: ports.RemoteCatalogSession{LifecycleID: lifecycle, Name: "work", State: "running", Reason: "bogus"}}, | |||
| 435 | + | } { | |||
| 436 | + | t.Run(test.name, func(t *testing.T) { | |||
| 437 | + | hosts := &remoteRefreshHostStore{hosts: []string{"arch"}} | |||
| 438 | + | d, _, _ := newRemoteRefreshDaemon(t, hosts, time.Unix(400, 0)) | |||
| 439 | + | d.remoteCatalog.refresh = 1 | |||
| 440 | + | catalog := ports.RemoteCatalog{ProtocolVersion: ports.ProtocolVersion, SchemaVersion: ports.RemoteCatalogSchemaVersion, Sessions: []ports.RemoteCatalogSession{test.session}} | |||
| 441 | + | require.False(t, d.applyRemoteRefreshResult(1, "arch", catalog, nil)) | |||
| 442 | + | d.remoteCatalog.mu.Lock() | |||
| 443 | + | require.Equal(t, remoteHostMalformed, d.remoteCatalog.status["arch"]) | |||
| 444 | + | d.remoteCatalog.mu.Unlock() | |||
| 445 | + | }) | |||
| 446 | + | } | |||
| 447 | + | } | |||
| 448 | + | ||||
| 406 | func TestRemoteRefreshVersionMismatchPreservesStaleRows(t *testing.T) { | 449 | func TestRemoteRefreshVersionMismatchPreservesStaleRows(t *testing.T) { | ||
| 407 | hosts := &remoteRefreshHostStore{hosts: []string{"arch"}} | 450 | hosts := &remoteRefreshHostStore{hosts: []string{"arch"}} | ||
| 408 | d, _, _ := newRemoteRefreshDaemon(t, hosts, time.Unix(400, 0)) | 451 | d, _, _ := newRemoteRefreshDaemon(t, hosts, time.Unix(400, 0)) | ||
M internal/usecase/daemon/remote_preview.go +5 -0
| @@ -3,6 +3,7 @@ package daemon | |||
| 3 | 3 | import ( | |
| 4 | 4 | "errors" | |
| 5 | 5 | ||
| 6 | + | "github.com/bnema/vev/internal/domain" | |
| 6 | 7 | "github.com/bnema/vev/internal/ports" | |
| 7 | 8 | "github.com/bnema/vev/pkg/renderer" | |
| 8 | 9 | ) | |
| @@ -54,6 +55,10 @@ func (d *Daemon) captureRemotePreview(request ports.RemotePreviewRequest) (ports | |||
| 54 | 55 | return ports.RemotePreview{}, errRemotePreviewNoSuchTarget | |
| 55 | 56 | } | |
| 56 | 57 | tab := sess.tabs[index] | |
| 58 | + | if tab == nil || domain.TabStableID(tab.stableID) != target.LiveTabID { | |
| 59 | + | sess.mu.Unlock() | |
| 60 | + | return ports.RemotePreview{}, errRemotePreviewNoSuchTarget | |
| 61 | + | } | |
| 57 | 62 | sess.mu.Unlock() | |
| 58 | 63 | tab.mu.Lock() | |
| 59 | 64 | pane := tab.focusedPane() | |
| @@ -3,6 +3,7 @@ package daemon | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "errors" | 4 | "errors" | ||
| 5 | 5 | ||||
| 6 | + | "github.com/bnema/vev/internal/domain" | |||
| 6 | "github.com/bnema/vev/internal/ports" | 7 | "github.com/bnema/vev/internal/ports" | ||
| 7 | "github.com/bnema/vev/pkg/renderer" | 8 | "github.com/bnema/vev/pkg/renderer" | ||
| 8 | ) | 9 | ) | ||
| @@ -54,6 +55,10 @@ func (d *Daemon) captureRemotePreview(request ports.RemotePreviewRequest) (ports | |||||
| 54 | return ports.RemotePreview{}, errRemotePreviewNoSuchTarget | 55 | return ports.RemotePreview{}, errRemotePreviewNoSuchTarget | ||
| 55 | } | 56 | } | ||
| 56 | tab := sess.tabs[index] | 57 | tab := sess.tabs[index] | ||
| 58 | + | if tab == nil || domain.TabStableID(tab.stableID) != target.LiveTabID { | |||
| 59 | + | sess.mu.Unlock() | |||
| 60 | + | return ports.RemotePreview{}, errRemotePreviewNoSuchTarget | |||
| 61 | + | } | |||
| 57 | sess.mu.Unlock() | 62 | sess.mu.Unlock() | ||
| 58 | tab.mu.Lock() | 63 | tab.mu.Lock() | ||
| 59 | pane := tab.focusedPane() | 64 | pane := tab.focusedPane() | ||
M internal/usecase/daemon/remote_preview_cache.go +23 -7
| @@ -191,8 +191,23 @@ func (d *Daemon) runRemotePreviewFlight(ctx context.Context, key remotePreviewCa | |||
| 191 | 191 | d.finishRemotePreviewFlight(key, flight, ports.RemotePreview{}, ctx.Err()) | |
| 192 | 192 | return | |
| 193 | 193 | } | |
| 194 | - | preview, err := d.remotePreviewClient.Preview(ctx, target, width, height) | |
| 195 | - | <-d.remotePreview.slots | |
| 194 | + | var preview ports.RemotePreview | |
| 195 | + | var err error | |
| 196 | + | defer func() { | |
| 197 | + | if recovered := recover(); recovered != nil { | |
| 198 | + | err = fmt.Errorf("remote preview flight panicked: %v", recovered) | |
| 199 | + | preview = ports.RemotePreview{} | |
| 200 | + | d.finishRemotePreviewFlight(key, flight, preview, err) | |
| 201 | + | panic(recovered) | |
| 202 | + | } | |
| 203 | + | if err != nil { | |
| 204 | + | preview = ports.RemotePreview{} | |
| 205 | + | } | |
| 206 | + | d.finishRemotePreviewFlight(key, flight, preview, err) | |
| 207 | + | }() | |
| 208 | + | defer func() { <-d.remotePreview.slots }() | |
| 209 | + | ||
| 210 | + | preview, err = d.remotePreviewClient.Preview(ctx, target, width, height) | |
| 196 | 211 | if err == nil { | |
| 197 | 212 | if validationErr := ports.ValidateRemotePreview(preview); validationErr != nil { | |
| 198 | 213 | err = validationErr | |
| @@ -202,15 +217,16 @@ func (d *Daemon) runRemotePreviewFlight(ctx context.Context, key remotePreviewCa | |||
| 202 | 217 | err = errors.New("remote preview identity changed") | |
| 203 | 218 | } | |
| 204 | 219 | } | |
| 205 | - | if err != nil { | |
| 206 | - | preview = ports.RemotePreview{} | |
| 207 | - | } | |
| 208 | - | d.finishRemotePreviewFlight(key, flight, preview, err) | |
| 209 | 220 | } | |
| 210 | 221 | ||
| 211 | 222 | func (d *Daemon) finishRemotePreviewFlight(key remotePreviewCacheKey, flight *remotePreviewFlight, preview ports.RemotePreview, err error) { | |
| 212 | 223 | now := d.clock.Now() | |
| 213 | 224 | d.remotePreview.mu.Lock() | |
| 225 | + | for cooldownKey, until := range d.remotePreview.cooldowns { | |
| 226 | + | if !now.Before(until) { | |
| 227 | + | delete(d.remotePreview.cooldowns, cooldownKey) | |
| 228 | + | } | |
| 229 | + | } | |
| 214 | 230 | flight.preview = cloneRemotePreview(preview) | |
| 215 | 231 | flight.err = err | |
| 216 | 232 | delete(d.remotePreview.flights, key) | |
| @@ -233,7 +249,7 @@ func (d *Daemon) finishRemotePreviewFlight(key remotePreviewCacheKey, flight *re | |||
| 233 | 249 | } | |
| 234 | 250 | delete(d.remotePreview.cache, oldestKey) | |
| 235 | 251 | } | |
| 236 | - | } else if !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) { | |
| 252 | + | } else if !errors.Is(err, context.Canceled) { | |
| 237 | 253 | d.remotePreview.cooldowns[key] = now.Add(remotePreviewCooldown) | |
| 238 | 254 | } | |
| 239 | 255 | close(flight.done) | |
| @@ -191,8 +191,23 @@ func (d *Daemon) runRemotePreviewFlight(ctx context.Context, key remotePreviewCa | |||||
| 191 | d.finishRemotePreviewFlight(key, flight, ports.RemotePreview{}, ctx.Err()) | 191 | d.finishRemotePreviewFlight(key, flight, ports.RemotePreview{}, ctx.Err()) | ||
| 192 | return | 192 | return | ||
| 193 | } | 193 | } | ||
| 194 | - | preview, err := d.remotePreviewClient.Preview(ctx, target, width, height) | 194 | + | var preview ports.RemotePreview |
| 195 | - | <-d.remotePreview.slots | 195 | + | var err error |
| 196 | + | defer func() { | |||
| 197 | + | if recovered := recover(); recovered != nil { | |||
| 198 | + | err = fmt.Errorf("remote preview flight panicked: %v", recovered) | |||
| 199 | + | preview = ports.RemotePreview{} | |||
| 200 | + | d.finishRemotePreviewFlight(key, flight, preview, err) | |||
| 201 | + | panic(recovered) | |||
| 202 | + | } | |||
| 203 | + | if err != nil { | |||
| 204 | + | preview = ports.RemotePreview{} | |||
| 205 | + | } | |||
| 206 | + | d.finishRemotePreviewFlight(key, flight, preview, err) | |||
| 207 | + | }() | |||
| 208 | + | defer func() { <-d.remotePreview.slots }() | |||
| 209 | + | ||||
| 210 | + | preview, err = d.remotePreviewClient.Preview(ctx, target, width, height) | |||
| 196 | if err == nil { | 211 | if err == nil { | ||
| 197 | if validationErr := ports.ValidateRemotePreview(preview); validationErr != nil { | 212 | if validationErr := ports.ValidateRemotePreview(preview); validationErr != nil { | ||
| 198 | err = validationErr | 213 | err = validationErr | ||
| @@ -202,15 +217,16 @@ func (d *Daemon) runRemotePreviewFlight(ctx context.Context, key remotePreviewCa | |||||
| 202 | err = errors.New("remote preview identity changed") | 217 | err = errors.New("remote preview identity changed") | ||
| 203 | } | 218 | } | ||
| 204 | } | 219 | } | ||
| 205 | - | if err != nil { | |||
| 206 | - | preview = ports.RemotePreview{} | |||
| 207 | - | } | |||
| 208 | - | d.finishRemotePreviewFlight(key, flight, preview, err) | |||
| 209 | } | 220 | } | ||
| 210 | 221 | ||||
| 211 | func (d *Daemon) finishRemotePreviewFlight(key remotePreviewCacheKey, flight *remotePreviewFlight, preview ports.RemotePreview, err error) { | 222 | func (d *Daemon) finishRemotePreviewFlight(key remotePreviewCacheKey, flight *remotePreviewFlight, preview ports.RemotePreview, err error) { | ||
| 212 | now := d.clock.Now() | 223 | now := d.clock.Now() | ||
| 213 | d.remotePreview.mu.Lock() | 224 | d.remotePreview.mu.Lock() | ||
| 225 | + | for cooldownKey, until := range d.remotePreview.cooldowns { | |||
| 226 | + | if !now.Before(until) { | |||
| 227 | + | delete(d.remotePreview.cooldowns, cooldownKey) | |||
| 228 | + | } | |||
| 229 | + | } | |||
| 214 | flight.preview = cloneRemotePreview(preview) | 230 | flight.preview = cloneRemotePreview(preview) | ||
| 215 | flight.err = err | 231 | flight.err = err | ||
| 216 | delete(d.remotePreview.flights, key) | 232 | delete(d.remotePreview.flights, key) | ||
| @@ -233,7 +249,7 @@ func (d *Daemon) finishRemotePreviewFlight(key remotePreviewCacheKey, flight *re | |||||
| 233 | } | 249 | } | ||
| 234 | delete(d.remotePreview.cache, oldestKey) | 250 | delete(d.remotePreview.cache, oldestKey) | ||
| 235 | } | 251 | } | ||
| 236 | - | } else if !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) { | 252 | + | } else if !errors.Is(err, context.Canceled) { |
| 237 | d.remotePreview.cooldowns[key] = now.Add(remotePreviewCooldown) | 253 | d.remotePreview.cooldowns[key] = now.Add(remotePreviewCooldown) | ||
| 238 | } | 254 | } | ||
| 239 | close(flight.done) | 255 | close(flight.done) | ||
M internal/usecase/daemon/remote_preview_cache_test.go +27 -8
| @@ -112,11 +112,15 @@ func remotePreviewCacheTarget() domain.RemoteSessionTarget { | |||
| 112 | 112 | } | |
| 113 | 113 | ||
| 114 | 114 | func remotePreviewCacheResult(target domain.RemoteSessionTarget, revision uint64) ports.RemotePreview { | |
| 115 | + | return remotePreviewCacheResultRune(target, revision, 'x') | |
| 116 | + | } | |
| 117 | + | ||
| 118 | + | func remotePreviewCacheResultRune(target domain.RemoteSessionTarget, revision uint64, value rune) ports.RemotePreview { | |
| 115 | 119 | return ports.RemotePreview{ | |
| 116 | 120 | Version: ports.RemotePreviewSchemaVersion, Status: ports.RemotePreviewOK, | |
| 117 | 121 | LifecycleID: target.LifecycleID, TabID: target.LiveTabID, | |
| 118 | 122 | Revision: revision, Width: 1, Height: 1, | |
| 119 | - | Cells: []renderer.Cell{{Rune: 'x', Style: renderer.DefaultStyle()}}, | |
| 123 | + | Cells: []renderer.Cell{{Rune: value, Style: renderer.DefaultStyle()}}, | |
| 120 | 124 | } | |
| 121 | 125 | } | |
| 122 | 126 | ||
| @@ -162,6 +166,20 @@ func TestFetchRemotePreviewSingleFlightAndCopiesCache(t *testing.T) { | |||
| 162 | 166 | require.Equal(t, 1, client.Calls()) | |
| 163 | 167 | } | |
| 164 | 168 | ||
| 169 | + | func TestFetchRemotePreviewAdapterTimeoutAppliesCooldown(t *testing.T) { | |
| 170 | + | clock := &remotePreviewTestClock{now: time.Unix(250, 0)} | |
| 171 | + | target := remotePreviewCacheTarget() | |
| 172 | + | client := &remotePreviewTestClient{err: ports.ErrRemotePreviewTimeout} | |
| 173 | + | d := newTestDaemon(t, nil, clock) | |
| 174 | + | d.remotePreviewClient = client | |
| 175 | + | ||
| 176 | + | _, firstErr := d.fetchRemotePreview(context.Background(), target, 1, 1) | |
| 177 | + | require.ErrorIs(t, firstErr, ports.ErrRemotePreviewTimeout) | |
| 178 | + | _, secondErr := d.fetchRemotePreview(context.Background(), target, 1, 1) | |
| 179 | + | require.ErrorIs(t, secondErr, errRemotePreviewCooldown) | |
| 180 | + | require.Equal(t, 1, client.Calls()) | |
| 181 | + | } | |
| 182 | + | ||
| 165 | 183 | func TestFetchRemotePreviewRejectsMalformedResponseAndAppliesCooldown(t *testing.T) { | |
| 166 | 184 | clock := &remotePreviewTestClock{now: time.Unix(200, 0)} | |
| 167 | 185 | target := remotePreviewCacheTarget() | |
| @@ -205,7 +223,7 @@ func TestFetchRemotePreviewServesStaleWhileRefreshing(t *testing.T) { | |||
| 205 | 223 | require.NoError(t, err) | |
| 206 | 224 | require.Equal(t, uint64(1), stale.Revision, "expired content remains available during revalidation") | |
| 207 | 225 | <-client.started | |
| 208 | - | key := remotePreviewCacheKey{Endpoint: target.Endpoint, DisplayOrigin: target.DisplayOrigin, LifecycleID: target.LifecycleID, SessionName: target.SessionName, LiveTabID: target.LiveTabID, Width: 1, Height: 1} | |
| 226 | + | key := remotePreviewKeyFor(target, 1, 1) | |
| 209 | 227 | d.remotePreview.mu.Lock() | |
| 210 | 228 | flight := d.remotePreview.flights[key] | |
| 211 | 229 | d.remotePreview.mu.Unlock() | |
| @@ -272,7 +290,7 @@ func TestRemotePickerPreviewLateResponseCannotPublishAfterCloseAndReopen(t *test | |||
| 272 | 290 | secondTarget.LifecycleID[0]++ | |
| 273 | 291 | secondTarget.LiveTabID = "tab-2" | |
| 274 | 292 | client.mu.Lock() | |
| 275 | - | client.result = remotePreviewCacheResult(secondTarget, 2) | |
| 293 | + | client.result = remotePreviewCacheResultRune(secondTarget, 2, 'y') | |
| 276 | 294 | client.started = make(chan struct{}) | |
| 277 | 295 | client.release = make(chan struct{}) | |
| 278 | 296 | client.startedSignal = false | |
| @@ -300,9 +318,10 @@ func TestRemotePickerPreviewLateResponseCannotPublishAfterCloseAndReopen(t *test | |||
| 300 | 318 | close(firstRelease) | |
| 301 | 319 | close(client.release) | |
| 302 | 320 | width, height := remotePickerPreviewSize(ac.sizeSnapshot()) | |
| 303 | - | firstKeyForCache := remotePreviewCacheKey{Endpoint: firstTarget.Endpoint, DisplayOrigin: firstTarget.DisplayOrigin, LifecycleID: firstTarget.LifecycleID, SessionName: firstTarget.SessionName, LiveTabID: firstTarget.LiveTabID, Width: width, Height: height} | |
| 304 | - | secondKeyForCache := remotePreviewCacheKey{Endpoint: secondTarget.Endpoint, DisplayOrigin: secondTarget.DisplayOrigin, LifecycleID: secondTarget.LifecycleID, SessionName: secondTarget.SessionName, LiveTabID: secondTarget.LiveTabID, Width: width, Height: height} | |
| 305 | - | for _, key := range []remotePreviewCacheKey{firstKeyForCache, secondKeyForCache} { | |
| 321 | + | for _, key := range []remotePreviewCacheKey{ | |
| 322 | + | remotePreviewKeyFor(firstTarget, width, height), | |
| 323 | + | remotePreviewKeyFor(secondTarget, width, height), | |
| 324 | + | } { | |
| 306 | 325 | d.remotePreview.mu.Lock() | |
| 307 | 326 | flight := d.remotePreview.flights[key] | |
| 308 | 327 | d.remotePreview.mu.Unlock() | |
| @@ -316,9 +335,9 @@ func TestRemotePickerPreviewLateResponseCannotPublishAfterCloseAndReopen(t *test | |||
| 316 | 335 | preview = ac.overlays.pickerRemotePreview | |
| 317 | 336 | currentGeneration := ac.overlays.pickerPreviewGeneration | |
| 318 | 337 | ac.overlays.pickerMu.Unlock() | |
| 319 | - | return currentGeneration == secondGeneration && len(preview.Rows) != 0 && len(preview.Rows[0]) != 0 && preview.Rows[0][0].Rune == 'x' | |
| 338 | + | return currentGeneration == secondGeneration && len(preview.Rows) != 0 && len(preview.Rows[0]) != 0 && preview.Rows[0][0].Rune == 'y' | |
| 320 | 339 | }, time.Second, time.Millisecond) | |
| 321 | - | require.Equal(t, rune('x'), preview.Rows[0][0].Rune) | |
| 340 | + | require.Equal(t, rune('y'), preview.Rows[0][0].Rune) | |
| 322 | 341 | require.Equal(t, 2, client.Calls()) | |
| 323 | 342 | } | |
| 324 | 343 | ||
| @@ -112,11 +112,15 @@ func remotePreviewCacheTarget() domain.RemoteSessionTarget { | |||||
| 112 | } | 112 | } | ||
| 113 | 113 | ||||
| 114 | func remotePreviewCacheResult(target domain.RemoteSessionTarget, revision uint64) ports.RemotePreview { | 114 | func remotePreviewCacheResult(target domain.RemoteSessionTarget, revision uint64) ports.RemotePreview { | ||
| 115 | + | return remotePreviewCacheResultRune(target, revision, 'x') | |||
| 116 | + | } | |||
| 117 | + | ||||
| 118 | + | func remotePreviewCacheResultRune(target domain.RemoteSessionTarget, revision uint64, value rune) ports.RemotePreview { | |||
| 115 | return ports.RemotePreview{ | 119 | return ports.RemotePreview{ | ||
| 116 | Version: ports.RemotePreviewSchemaVersion, Status: ports.RemotePreviewOK, | 120 | Version: ports.RemotePreviewSchemaVersion, Status: ports.RemotePreviewOK, | ||
| 117 | LifecycleID: target.LifecycleID, TabID: target.LiveTabID, | 121 | LifecycleID: target.LifecycleID, TabID: target.LiveTabID, | ||
| 118 | Revision: revision, Width: 1, Height: 1, | 122 | Revision: revision, Width: 1, Height: 1, | ||
| 119 | - | Cells: []renderer.Cell{{Rune: 'x', Style: renderer.DefaultStyle()}}, | 123 | + | Cells: []renderer.Cell{{Rune: value, Style: renderer.DefaultStyle()}}, |
| 120 | } | 124 | } | ||
| 121 | } | 125 | } | ||
| 122 | 126 | ||||
| @@ -162,6 +166,20 @@ func TestFetchRemotePreviewSingleFlightAndCopiesCache(t *testing.T) { | |||||
| 162 | require.Equal(t, 1, client.Calls()) | 166 | require.Equal(t, 1, client.Calls()) | ||
| 163 | } | 167 | } | ||
| 164 | 168 | ||||
| 169 | + | func TestFetchRemotePreviewAdapterTimeoutAppliesCooldown(t *testing.T) { | |||
| 170 | + | clock := &remotePreviewTestClock{now: time.Unix(250, 0)} | |||
| 171 | + | target := remotePreviewCacheTarget() | |||
| 172 | + | client := &remotePreviewTestClient{err: ports.ErrRemotePreviewTimeout} | |||
| 173 | + | d := newTestDaemon(t, nil, clock) | |||
| 174 | + | d.remotePreviewClient = client | |||
| 175 | + | ||||
| 176 | + | _, firstErr := d.fetchRemotePreview(context.Background(), target, 1, 1) | |||
| 177 | + | require.ErrorIs(t, firstErr, ports.ErrRemotePreviewTimeout) | |||
| 178 | + | _, secondErr := d.fetchRemotePreview(context.Background(), target, 1, 1) | |||
| 179 | + | require.ErrorIs(t, secondErr, errRemotePreviewCooldown) | |||
| 180 | + | require.Equal(t, 1, client.Calls()) | |||
| 181 | + | } | |||
| 182 | + | ||||
| 165 | func TestFetchRemotePreviewRejectsMalformedResponseAndAppliesCooldown(t *testing.T) { | 183 | func TestFetchRemotePreviewRejectsMalformedResponseAndAppliesCooldown(t *testing.T) { | ||
| 166 | clock := &remotePreviewTestClock{now: time.Unix(200, 0)} | 184 | clock := &remotePreviewTestClock{now: time.Unix(200, 0)} | ||
| 167 | target := remotePreviewCacheTarget() | 185 | target := remotePreviewCacheTarget() | ||
| @@ -205,7 +223,7 @@ func TestFetchRemotePreviewServesStaleWhileRefreshing(t *testing.T) { | |||||
| 205 | require.NoError(t, err) | 223 | require.NoError(t, err) | ||
| 206 | require.Equal(t, uint64(1), stale.Revision, "expired content remains available during revalidation") | 224 | require.Equal(t, uint64(1), stale.Revision, "expired content remains available during revalidation") | ||
| 207 | <-client.started | 225 | <-client.started | ||
| 208 | - | key := remotePreviewCacheKey{Endpoint: target.Endpoint, DisplayOrigin: target.DisplayOrigin, LifecycleID: target.LifecycleID, SessionName: target.SessionName, LiveTabID: target.LiveTabID, Width: 1, Height: 1} | 226 | + | key := remotePreviewKeyFor(target, 1, 1) |
| 209 | d.remotePreview.mu.Lock() | 227 | d.remotePreview.mu.Lock() | ||
| 210 | flight := d.remotePreview.flights[key] | 228 | flight := d.remotePreview.flights[key] | ||
| 211 | d.remotePreview.mu.Unlock() | 229 | d.remotePreview.mu.Unlock() | ||
| @@ -272,7 +290,7 @@ func TestRemotePickerPreviewLateResponseCannotPublishAfterCloseAndReopen(t *test | |||||
| 272 | secondTarget.LifecycleID[0]++ | 290 | secondTarget.LifecycleID[0]++ | ||
| 273 | secondTarget.LiveTabID = "tab-2" | 291 | secondTarget.LiveTabID = "tab-2" | ||
| 274 | client.mu.Lock() | 292 | client.mu.Lock() | ||
| 275 | - | client.result = remotePreviewCacheResult(secondTarget, 2) | 293 | + | client.result = remotePreviewCacheResultRune(secondTarget, 2, 'y') |
| 276 | client.started = make(chan struct{}) | 294 | client.started = make(chan struct{}) | ||
| 277 | client.release = make(chan struct{}) | 295 | client.release = make(chan struct{}) | ||
| 278 | client.startedSignal = false | 296 | client.startedSignal = false | ||
| @@ -300,9 +318,10 @@ func TestRemotePickerPreviewLateResponseCannotPublishAfterCloseAndReopen(t *test | |||||
| 300 | close(firstRelease) | 318 | close(firstRelease) | ||
| 301 | close(client.release) | 319 | close(client.release) | ||
| 302 | width, height := remotePickerPreviewSize(ac.sizeSnapshot()) | 320 | width, height := remotePickerPreviewSize(ac.sizeSnapshot()) | ||
| 303 | - | firstKeyForCache := remotePreviewCacheKey{Endpoint: firstTarget.Endpoint, DisplayOrigin: firstTarget.DisplayOrigin, LifecycleID: firstTarget.LifecycleID, SessionName: firstTarget.SessionName, LiveTabID: firstTarget.LiveTabID, Width: width, Height: height} | 321 | + | for _, key := range []remotePreviewCacheKey{ |
| 304 | - | secondKeyForCache := remotePreviewCacheKey{Endpoint: secondTarget.Endpoint, DisplayOrigin: secondTarget.DisplayOrigin, LifecycleID: secondTarget.LifecycleID, SessionName: secondTarget.SessionName, LiveTabID: secondTarget.LiveTabID, Width: width, Height: height} | 322 | + | remotePreviewKeyFor(firstTarget, width, height), |
| 305 | - | for _, key := range []remotePreviewCacheKey{firstKeyForCache, secondKeyForCache} { | 323 | + | remotePreviewKeyFor(secondTarget, width, height), |
| 324 | + | } { | |||
| 306 | d.remotePreview.mu.Lock() | 325 | d.remotePreview.mu.Lock() | ||
| 307 | flight := d.remotePreview.flights[key] | 326 | flight := d.remotePreview.flights[key] | ||
| 308 | d.remotePreview.mu.Unlock() | 327 | d.remotePreview.mu.Unlock() | ||
| @@ -316,9 +335,9 @@ func TestRemotePickerPreviewLateResponseCannotPublishAfterCloseAndReopen(t *test | |||||
| 316 | preview = ac.overlays.pickerRemotePreview | 335 | preview = ac.overlays.pickerRemotePreview | ||
| 317 | currentGeneration := ac.overlays.pickerPreviewGeneration | 336 | currentGeneration := ac.overlays.pickerPreviewGeneration | ||
| 318 | ac.overlays.pickerMu.Unlock() | 337 | ac.overlays.pickerMu.Unlock() | ||
| 319 | - | return currentGeneration == secondGeneration && len(preview.Rows) != 0 && len(preview.Rows[0]) != 0 && preview.Rows[0][0].Rune == 'x' | 338 | + | return currentGeneration == secondGeneration && len(preview.Rows) != 0 && len(preview.Rows[0]) != 0 && preview.Rows[0][0].Rune == 'y' |
| 320 | }, time.Second, time.Millisecond) | 339 | }, time.Second, time.Millisecond) | ||
| 321 | - | require.Equal(t, rune('x'), preview.Rows[0][0].Rune) | 340 | + | require.Equal(t, rune('y'), preview.Rows[0][0].Rune) |
| 322 | require.Equal(t, 2, client.Calls()) | 341 | require.Equal(t, 2, client.Calls()) | ||
| 323 | } | 342 | } | ||
| 324 | 343 | ||||
M internal/usecase/daemon/remote_preview_capture_test.go +24 -0
| @@ -11,6 +11,30 @@ import ( | |||
| 11 | 11 | "github.com/bnema/vev/pkg/vt" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | + | func TestCaptureRemotePreviewUsesBottomRowsForShorterPreview(t *testing.T) { | |
| 15 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 16 | + | sess := addControlSession(d, "work", "tab-1", "pane-1") | |
| 17 | + | sess.ephemeral = false | |
| 18 | + | sess.incarnation = remoteLifecycleForTest() | |
| 19 | + | pane := sess.tabs[0].focusedPane() | |
| 20 | + | pane.mu.Lock() | |
| 21 | + | pane.screen = vt.NewScreen(2, 3) | |
| 22 | + | pane.screen.Frame.Set(0, 0, renderer.Cell{Rune: 'a'}) | |
| 23 | + | pane.screen.Frame.Set(0, 1, renderer.Cell{Rune: 'b'}) | |
| 24 | + | pane.screen.Frame.Set(0, 2, renderer.Cell{Rune: 'c'}) | |
| 25 | + | pane.mu.Unlock() | |
| 26 | + | ||
| 27 | + | target := domain.RemoteSessionTarget{ | |
| 28 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, | |
| 29 | + | SessionName: "work", LiveTabID: "tab-1", | |
| 30 | + | } | |
| 31 | + | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | |
| 32 | + | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: 1, Height: 2, | |
| 33 | + | }) | |
| 34 | + | require.NoError(t, err) | |
| 35 | + | require.Equal(t, []rune{'b', 'c'}, []rune{preview.Cells[0].Rune, preview.Cells[1].Rune}) | |
| 36 | + | } | |
| 37 | + | ||
| 14 | 38 | func TestCaptureRemotePreviewDoesNotSplitWideRuneAtCropBoundary(t *testing.T) { | |
| 15 | 39 | d := newTestDaemon(t, nil, stubClock{}) | |
| 16 | 40 | sess := addControlSession(d, "work", "tab-1", "pane-1") | |
| @@ -11,6 +11,30 @@ import ( | |||||
| 11 | "github.com/bnema/vev/pkg/vt" | 11 | "github.com/bnema/vev/pkg/vt" | ||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | + | func TestCaptureRemotePreviewUsesBottomRowsForShorterPreview(t *testing.T) { | |||
| 15 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 16 | + | sess := addControlSession(d, "work", "tab-1", "pane-1") | |||
| 17 | + | sess.ephemeral = false | |||
| 18 | + | sess.incarnation = remoteLifecycleForTest() | |||
| 19 | + | pane := sess.tabs[0].focusedPane() | |||
| 20 | + | pane.mu.Lock() | |||
| 21 | + | pane.screen = vt.NewScreen(2, 3) | |||
| 22 | + | pane.screen.Frame.Set(0, 0, renderer.Cell{Rune: 'a'}) | |||
| 23 | + | pane.screen.Frame.Set(0, 1, renderer.Cell{Rune: 'b'}) | |||
| 24 | + | pane.screen.Frame.Set(0, 2, renderer.Cell{Rune: 'c'}) | |||
| 25 | + | pane.mu.Unlock() | |||
| 26 | + | ||||
| 27 | + | target := domain.RemoteSessionTarget{ | |||
| 28 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: sess.incarnation, | |||
| 29 | + | SessionName: "work", LiveTabID: "tab-1", | |||
| 30 | + | } | |||
| 31 | + | preview, err := d.captureRemotePreview(ports.RemotePreviewRequest{ | |||
| 32 | + | Version: ports.RemotePreviewSchemaVersion, Target: target, Width: 1, Height: 2, | |||
| 33 | + | }) | |||
| 34 | + | require.NoError(t, err) | |||
| 35 | + | require.Equal(t, []rune{'b', 'c'}, []rune{preview.Cells[0].Rune, preview.Cells[1].Rune}) | |||
| 36 | + | } | |||
| 37 | + | ||||
| 14 | func TestCaptureRemotePreviewDoesNotSplitWideRuneAtCropBoundary(t *testing.T) { | 38 | func TestCaptureRemotePreviewDoesNotSplitWideRuneAtCropBoundary(t *testing.T) { | ||
| 15 | d := newTestDaemon(t, nil, stubClock{}) | 39 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 16 | sess := addControlSession(d, "work", "tab-1", "pane-1") | 40 | sess := addControlSession(d, "work", "tab-1", "pane-1") | ||
M internal/usecase/picker/model.go +3 -2
| @@ -323,8 +323,9 @@ func rowsForSession(session SessionView, config SelectionConfig) []row { | |||
| 323 | 323 | tabRow.hasRemoteTarget = true | |
| 324 | 324 | } | |
| 325 | 325 | if !tabRow.hasRemoteTarget { | |
| 326 | - | tabRow.focusable = false | |
| 327 | - | tabRow.selectable = false | |
| 326 | + | legacyRemote := common.hasRemoteKey && config.Mode == SelectNavigationTab && remoteSelectable(session) | |
| 327 | + | tabRow.focusable = legacyRemote | |
| 328 | + | tabRow.selectable = legacyRemote | |
| 328 | 329 | } else { | |
| 329 | 330 | tabRow.focusable = true | |
| 330 | 331 | tabRow.selectable = tabRow.remoteTarget.Validate() == nil && config.Mode == SelectNavigationTab && remoteSelectable(session) | |
| @@ -323,8 +323,9 @@ func rowsForSession(session SessionView, config SelectionConfig) []row { | |||||
| 323 | tabRow.hasRemoteTarget = true | 323 | tabRow.hasRemoteTarget = true | ||
| 324 | } | 324 | } | ||
| 325 | if !tabRow.hasRemoteTarget { | 325 | if !tabRow.hasRemoteTarget { | ||
| 326 | - | tabRow.focusable = false | 326 | + | legacyRemote := common.hasRemoteKey && config.Mode == SelectNavigationTab && remoteSelectable(session) |
| 327 | - | tabRow.selectable = false | 327 | + | tabRow.focusable = legacyRemote |
| 328 | + | tabRow.selectable = legacyRemote | |||
| 328 | } else { | 329 | } else { | ||
| 329 | tabRow.focusable = true | 330 | tabRow.focusable = true | ||
| 330 | tabRow.selectable = tabRow.remoteTarget.Validate() == nil && config.Mode == SelectNavigationTab && remoteSelectable(session) | 331 | tabRow.selectable = tabRow.remoteTarget.Validate() == nil && config.Mode == SelectNavigationTab && remoteSelectable(session) | ||
M internal/usecase/picker/remote_parity_test.go +22 -14
| @@ -32,18 +32,26 @@ func TestRemoteRowsWithDuplicateLabelsKeepDistinctRoutingIdentity(t *testing.T) | |||
| 32 | 32 | }, | |
| 33 | 33 | }, SelectionConfig{Mode: SelectNavigationTab}) | |
| 34 | 34 | ||
| 35 | - | first, ok := model.Selected() | |
| 36 | - | require.True(t, ok) | |
| 37 | - | require.Equal(t, firstKey.ID(), first.Session) | |
| 38 | - | require.NotNil(t, first.RemoteTarget) | |
| 39 | - | require.Equal(t, firstLifecycle, first.RemoteTarget.LifecycleID) | |
| 40 | - | require.Equal(t, domain.TabStableID("tab-a"), first.RemoteTarget.LiveTabID) | |
| 41 | - | ||
| 42 | - | model.Down() | |
| 43 | - | second, ok := model.Selected() | |
| 44 | - | require.True(t, ok) | |
| 45 | - | require.Equal(t, secondKey.ID(), second.Session) | |
| 46 | - | require.NotNil(t, second.RemoteTarget) | |
| 47 | - | require.Equal(t, secondLifecycle, second.RemoteTarget.LifecycleID) | |
| 48 | - | require.Equal(t, domain.TabStableID("tab-b"), second.RemoteTarget.LiveTabID) | |
| 35 | + | expected := []struct { | |
| 36 | + | name string | |
| 37 | + | session domain.SessionID | |
| 38 | + | lifecycle domain.SessionLifecycleID | |
| 39 | + | tabID domain.TabStableID | |
| 40 | + | }{ | |
| 41 | + | {name: "first", session: firstKey.ID(), lifecycle: firstLifecycle, tabID: "tab-a"}, | |
| 42 | + | {name: "second", session: secondKey.ID(), lifecycle: secondLifecycle, tabID: "tab-b"}, | |
| 43 | + | } | |
| 44 | + | for i, want := range expected { | |
| 45 | + | t.Run(want.name, func(t *testing.T) { | |
| 46 | + | selected, ok := model.Selected() | |
| 47 | + | require.True(t, ok) | |
| 48 | + | require.Equal(t, want.session, selected.Session) | |
| 49 | + | require.NotNil(t, selected.RemoteTarget) | |
| 50 | + | require.Equal(t, want.lifecycle, selected.RemoteTarget.LifecycleID) | |
| 51 | + | require.Equal(t, want.tabID, selected.RemoteTarget.LiveTabID) | |
| 52 | + | }) | |
| 53 | + | if i+1 < len(expected) { | |
| 54 | + | model.Down() | |
| 55 | + | } | |
| 56 | + | } | |
| 49 | 57 | } | |
| @@ -32,18 +32,26 @@ func TestRemoteRowsWithDuplicateLabelsKeepDistinctRoutingIdentity(t *testing.T) | |||||
| 32 | }, | 32 | }, | ||
| 33 | }, SelectionConfig{Mode: SelectNavigationTab}) | 33 | }, SelectionConfig{Mode: SelectNavigationTab}) | ||
| 34 | 34 | ||||
| 35 | - | first, ok := model.Selected() | 35 | + | expected := []struct { |
| 36 | - | require.True(t, ok) | 36 | + | name string |
| 37 | - | require.Equal(t, firstKey.ID(), first.Session) | 37 | + | session domain.SessionID |
| 38 | - | require.NotNil(t, first.RemoteTarget) | 38 | + | lifecycle domain.SessionLifecycleID |
| 39 | - | require.Equal(t, firstLifecycle, first.RemoteTarget.LifecycleID) | 39 | + | tabID domain.TabStableID |
| 40 | - | require.Equal(t, domain.TabStableID("tab-a"), first.RemoteTarget.LiveTabID) | 40 | + | }{ |
| 41 | - | 41 | + | {name: "first", session: firstKey.ID(), lifecycle: firstLifecycle, tabID: "tab-a"}, | |
| 42 | - | model.Down() | 42 | + | {name: "second", session: secondKey.ID(), lifecycle: secondLifecycle, tabID: "tab-b"}, |
| 43 | - | second, ok := model.Selected() | 43 | + | } |
| 44 | - | require.True(t, ok) | 44 | + | for i, want := range expected { |
| 45 | - | require.Equal(t, secondKey.ID(), second.Session) | 45 | + | t.Run(want.name, func(t *testing.T) { |
| 46 | - | require.NotNil(t, second.RemoteTarget) | 46 | + | selected, ok := model.Selected() |
| 47 | - | require.Equal(t, secondLifecycle, second.RemoteTarget.LifecycleID) | 47 | + | require.True(t, ok) |
| 48 | - | require.Equal(t, domain.TabStableID("tab-b"), second.RemoteTarget.LiveTabID) | 48 | + | require.Equal(t, want.session, selected.Session) |
| 49 | + | require.NotNil(t, selected.RemoteTarget) | |||
| 50 | + | require.Equal(t, want.lifecycle, selected.RemoteTarget.LifecycleID) | |||
| 51 | + | require.Equal(t, want.tabID, selected.RemoteTarget.LiveTabID) | |||
| 52 | + | }) | |||
| 53 | + | if i+1 < len(expected) { | |||
| 54 | + | model.Down() | |||
| 55 | + | } | |||
| 56 | + | } | |||
| 49 | } | 57 | } | ||
M scripts/remote-picker-harness/README.md +7 -0
| @@ -12,6 +12,13 @@ all resources when it exits. Docker uses the normal CLI configuration, so | |||
| 12 | 12 | `DOCKER_HOST` or the active context selects a rootless daemon; the harness does | |
| 13 | 13 | not assume a socket pathname. | |
| 14 | 14 | ||
| 15 | + | Environment overrides: | |
| 16 | + | ||
| 17 | + | - `VEV_HARNESS_BASE_IMAGE` selects the Docker base image used for both | |
| 18 | + | containers (default: `ubuntu:24.04`). | |
| 19 | + | - `VEV_HARNESS_TARGET` selects the SSH target passed to the harness binary | |
| 20 | + | (default: `test@remote`). | |
| 21 | + | ||
| 15 | 22 | The run exercises real SSH-stdio and UDP attachment, typed live catalog and | |
| 16 | 23 | preview requests, tab creation/removal fencing, exact lifecycle fencing after | |
| 17 | 24 | replacement, daemon restart resume, slow-preview cancellation, version | |
| @@ -12,6 +12,13 @@ all resources when it exits. Docker uses the normal CLI configuration, so | |||||
| 12 | `DOCKER_HOST` or the active context selects a rootless daemon; the harness does | 12 | `DOCKER_HOST` or the active context selects a rootless daemon; the harness does | ||
| 13 | not assume a socket pathname. | 13 | not assume a socket pathname. | ||
| 14 | 14 | ||||
| 15 | + | Environment overrides: | |||
| 16 | + | ||||
| 17 | + | - `VEV_HARNESS_BASE_IMAGE` selects the Docker base image used for both | |||
| 18 | + | containers (default: `ubuntu:24.04`). | |||
| 19 | + | - `VEV_HARNESS_TARGET` selects the SSH target passed to the harness binary | |||
| 20 | + | (default: `test@remote`). | |||
| 21 | + | ||||
| 15 | The run exercises real SSH-stdio and UDP attachment, typed live catalog and | 22 | The run exercises real SSH-stdio and UDP attachment, typed live catalog and | ||
| 16 | preview requests, tab creation/removal fencing, exact lifecycle fencing after | 23 | preview requests, tab creation/removal fencing, exact lifecycle fencing after | ||
| 17 | replacement, daemon restart resume, slow-preview cancellation, version | 24 | replacement, daemon restart resume, slow-preview cancellation, version | ||
M scripts/remote-picker-harness/main.go +154 -104
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | "os" | |
| 10 | 10 | "os/exec" | |
| 11 | 11 | "strings" | |
| 12 | + | "sync" | |
| 12 | 13 | "sync/atomic" | |
| 13 | 14 | "time" | |
| 14 | 15 | ||
| @@ -44,175 +45,172 @@ func run() error { | |||
| 44 | 45 | ||
| 45 | 46 | catalog := remoteadapter.NewCatalogClient() | |
| 46 | 47 | preview := remoteadapter.NewPreviewClient() | |
| 48 | + | if err := runLiveStdioPhase(ctx, target, catalog, preview); err != nil { | |
| 49 | + | return err | |
| 50 | + | } | |
| 51 | + | resumedTarget, err := runRestartResumePhase(ctx, target, catalog, preview) | |
| 52 | + | if err != nil { | |
| 53 | + | return err | |
| 54 | + | } | |
| 55 | + | if err := runUDPPhase(ctx, target); err != nil { | |
| 56 | + | return err | |
| 57 | + | } | |
| 58 | + | if err := runCatalogFailurePhase(ctx, target, catalog); err != nil { | |
| 59 | + | return err | |
| 60 | + | } | |
| 61 | + | if _, err := catalog.List(ctx, "test@unreachable.invalid"); err == nil { | |
| 62 | + | return errors.New("unreachable catalog unexpectedly succeeded") | |
| 63 | + | } | |
| 64 | + | return runStaleTargetPhase(ctx, target, catalog, resumedTarget) | |
| 65 | + | } | |
| 47 | 66 | ||
| 67 | + | func runLiveStdioPhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, preview ports.RemotePreviewClient) error { | |
| 48 | 68 | stdio, err := openTransport(ctx, ports.RemoteTransportStdio, target) | |
| 49 | 69 | if err != nil { | |
| 50 | 70 | return fmt.Errorf("open stdio transport: %w", err) | |
| 51 | 71 | } | |
| 72 | + | defer func() { _ = stdio.Close() }() | |
| 52 | 73 | if err := attachAndCheck(ctx, stdio, ports.Hello{ | |
| 53 | - | Intent: ports.IntentNew, | |
| 54 | - | Name: sessionName, | |
| 55 | - | Env: localEnvironment(), | |
| 56 | - | Cwd: "/tmp/local-cwd", | |
| 57 | - | TermEnv: "xterm-256color", | |
| 58 | - | TrueColor: true, | |
| 74 | + | Intent: ports.IntentNew, Name: sessionName, Env: localEnvironment(), Cwd: "/tmp/local-cwd", | |
| 75 | + | TermEnv: "xterm-256color", TrueColor: true, | |
| 59 | 76 | }); err != nil { | |
| 60 | - | _ = stdio.Close() | |
| 61 | 77 | return fmt.Errorf("stdio live attach: %w", err) | |
| 62 | 78 | } | |
| 63 | 79 | if err := sendInputAndAwait(ctx, stdio, "printf 'VEV_REMOTE_HARNESS_ENV=%s:%s:%s:%s\\n' \"$VEV_TEST_ENV\" \"$XDG_RUNTIME_DIR\" \"$WAYLAND_DISPLAY\" \"$VEV\"", "local:/run/local:wayland-local:"); err != nil { | |
| 64 | - | _ = stdio.Close() | |
| 65 | 80 | return fmt.Errorf("direct client environment: %w", err) | |
| 66 | 81 | } | |
| 67 | - | ||
| 68 | 82 | live, err := waitForSession(ctx, catalog, target, sessionName, "running") | |
| 69 | 83 | if err != nil { | |
| 70 | - | _ = stdio.Close() | |
| 71 | 84 | return fmt.Errorf("live catalog: %w", err) | |
| 72 | 85 | } | |
| 73 | 86 | liveTarget, err := targetForSession(target, live, false) | |
| 74 | 87 | if err != nil { | |
| 75 | - | _ = stdio.Close() | |
| 76 | 88 | return fmt.Errorf("live target: %w", err) | |
| 77 | 89 | } | |
| 78 | 90 | if _, err := preview.Preview(ctx, liveTarget, 40, 8); err != nil { | |
| 79 | - | _ = stdio.Close() | |
| 80 | 91 | return fmt.Errorf("live preview: %w", err) | |
| 81 | 92 | } | |
| 82 | - | ||
| 83 | 93 | if err := sendCommand(ctx, stdio, "new-tab"); err != nil { | |
| 84 | - | _ = stdio.Close() | |
| 85 | 94 | return fmt.Errorf("create second tab: %w", err) | |
| 86 | 95 | } | |
| 87 | 96 | liveWithTabs, err := waitForSessionWithTabCount(ctx, catalog, target, sessionName, "running", 2) | |
| 88 | 97 | if err != nil { | |
| 89 | - | _ = stdio.Close() | |
| 90 | 98 | return fmt.Errorf("multi-tab catalog: %w", err) | |
| 91 | 99 | } | |
| 92 | - | removedTargets := make([]domain.RemoteSessionTarget, 0, len(ports.CatalogTabs(liveWithTabs))) | |
| 93 | - | for _, tab := range ports.CatalogTabs(liveWithTabs) { | |
| 94 | - | candidate := liveWithTabs | |
| 95 | - | candidate.ActiveTabID = tab.ID | |
| 96 | - | removed, targetErr := targetForSession(target, candidate, false) | |
| 97 | - | if targetErr != nil { | |
| 98 | - | _ = stdio.Close() | |
| 99 | - | return fmt.Errorf("removed-tab target: %w", targetErr) | |
| 100 | - | } | |
| 101 | - | removedTargets = append(removedTargets, removed) | |
| 102 | - | } | |
| 100 | + | beforeTabs := ports.CatalogTabs(liveWithTabs) | |
| 103 | 101 | if err := sendCommand(ctx, stdio, "close-tab"); err != nil { | |
| 104 | - | _ = stdio.Close() | |
| 105 | 102 | return fmt.Errorf("remove tab: %w", err) | |
| 106 | 103 | } | |
| 107 | - | if _, err := waitForSessionWithTabCount(ctx, catalog, target, sessionName, "running", 1); err != nil { | |
| 108 | - | _ = stdio.Close() | |
| 104 | + | after, err := waitForSessionWithTabCount(ctx, catalog, target, sessionName, "running", 1) | |
| 105 | + | if err != nil { | |
| 109 | 106 | return fmt.Errorf("removed-tab catalog: %w", err) | |
| 110 | 107 | } | |
| 111 | - | removed := false | |
| 112 | - | for _, removedTarget := range removedTargets { | |
| 113 | - | if _, previewErr := preview.Preview(ctx, removedTarget, 40, 8); previewErr != nil { | |
| 114 | - | removed = true | |
| 115 | - | break | |
| 116 | - | } | |
| 108 | + | removedID, ok := removedCatalogTabID(beforeTabs, ports.CatalogTabs(after)) | |
| 109 | + | if !ok { | |
| 110 | + | return errors.New("removed-tab catalog did not identify exactly one removed tab") | |
| 111 | + | } | |
| 112 | + | removedSession := liveWithTabs | |
| 113 | + | removedSession.ActiveTabID = removedID | |
| 114 | + | removedTarget, err := targetForSession(target, removedSession, false) | |
| 115 | + | if err != nil { | |
| 116 | + | return fmt.Errorf("removed-tab target: %w", err) | |
| 117 | 117 | } | |
| 118 | - | if !removed { | |
| 119 | - | _ = stdio.Close() | |
| 118 | + | if _, err := preview.Preview(ctx, removedTarget, 40, 8); err == nil { | |
| 120 | 119 | return errors.New("removed tab unexpectedly produced a preview") | |
| 121 | 120 | } | |
| 122 | - | ||
| 121 | + | survivingTarget, err := targetForSession(target, after, false) | |
| 122 | + | if err != nil { | |
| 123 | + | return fmt.Errorf("surviving-tab target: %w", err) | |
| 124 | + | } | |
| 125 | + | if _, err := preview.Preview(ctx, survivingTarget, 40, 8); err != nil { | |
| 126 | + | return fmt.Errorf("surviving tab preview: %w", err) | |
| 127 | + | } | |
| 123 | 128 | if err := detach(ctx, stdio); err != nil { | |
| 124 | - | _ = stdio.Close() | |
| 125 | 129 | return fmt.Errorf("stop session: %w", err) | |
| 126 | 130 | } | |
| 127 | - | _ = stdio.Close() | |
| 131 | + | return nil | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | func runRestartResumePhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, preview ports.RemotePreviewClient) (domain.RemoteSessionTarget, error) { | |
| 128 | 135 | if err := runRemoteCommand(ctx, target, "vev", "kill", "--daemon"); err != nil { | |
| 129 | - | return fmt.Errorf("stop remote daemon: %w", err) | |
| 136 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("stop remote daemon: %w", err) | |
| 130 | 137 | } | |
| 131 | 138 | if err := runRemoteCommand(ctx, target, "vev", "--daemon-launcher"); err != nil { | |
| 132 | - | return fmt.Errorf("restart remote daemon: %w", err) | |
| 139 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("restart remote daemon: %w", err) | |
| 133 | 140 | } | |
| 134 | - | ||
| 135 | 141 | resumedCatalog, err := waitForSession(ctx, catalog, target, sessionName, "running") | |
| 136 | 142 | if err != nil { | |
| 137 | - | return fmt.Errorf("resumed catalog: %w", err) | |
| 143 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("resumed catalog: %w", err) | |
| 138 | 144 | } | |
| 139 | 145 | resumedTarget, err := targetForSession(target, resumedCatalog, false) | |
| 140 | 146 | if err != nil { | |
| 141 | - | return fmt.Errorf("resumed target: %w", err) | |
| 147 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("resumed target: %w", err) | |
| 142 | 148 | } | |
| 143 | 149 | resumed, err := openTransport(ctx, ports.RemoteTransportStdio, target) | |
| 144 | 150 | if err != nil { | |
| 145 | - | return fmt.Errorf("open restart stdio transport: %w", err) | |
| 151 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("open restart stdio transport: %w", err) | |
| 146 | 152 | } | |
| 153 | + | defer func() { _ = resumed.Close() }() | |
| 147 | 154 | if err := attachAndCheck(ctx, resumed, ports.Hello{ | |
| 148 | - | Intent: ports.IntentResume, | |
| 149 | - | Name: sessionName, | |
| 150 | - | Env: localEnvironment(), | |
| 151 | - | Cwd: "/tmp/local-cwd", | |
| 152 | - | TermEnv: "xterm-256color", | |
| 153 | - | TrueColor: true, | |
| 154 | - | RemoteTarget: &resumedTarget, | |
| 155 | + | Intent: ports.IntentResume, Name: sessionName, Env: localEnvironment(), Cwd: "/tmp/local-cwd", | |
| 156 | + | TermEnv: "xterm-256color", TrueColor: true, RemoteTarget: &resumedTarget, | |
| 155 | 157 | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |
| 156 | 158 | }); err != nil { | |
| 157 | - | _ = resumed.Close() | |
| 158 | - | return fmt.Errorf("exact restart resume: %w", err) | |
| 159 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("exact restart resume: %w", err) | |
| 159 | 160 | } | |
| 160 | 161 | if _, err := preview.Preview(ctx, resumedTarget, 40, 8); err != nil { | |
| 161 | - | _ = resumed.Close() | |
| 162 | - | return fmt.Errorf("restart preview: %w", err) | |
| 162 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("restart preview: %w", err) | |
| 163 | 163 | } | |
| 164 | 164 | if err := touchRemoteFlag(ctx, target, "/tmp/vev-slow-preview"); err != nil { | |
| 165 | - | _ = resumed.Close() | |
| 166 | - | return fmt.Errorf("enable slow preview: %w", err) | |
| 165 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("enable slow preview: %w", err) | |
| 167 | 166 | } | |
| 168 | 167 | slowCtx, slowCancel := context.WithTimeout(ctx, 150*time.Millisecond) | |
| 169 | 168 | _, slowErr := preview.Preview(slowCtx, resumedTarget, 40, 8) | |
| 170 | 169 | slowCancel() | |
| 170 | + | if err := touchRemoteFlag(ctx, target, "/tmp/vev-slow-preview-off"); err != nil { | |
| 171 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("disable slow preview: %w", err) | |
| 172 | + | } | |
| 171 | 173 | if slowErr == nil { | |
| 172 | - | _ = resumed.Close() | |
| 173 | - | return errors.New("slow preview did not honor cancellation") | |
| 174 | + | return domain.RemoteSessionTarget{}, errors.New("slow preview did not honor cancellation") | |
| 174 | 175 | } | |
| 175 | - | _ = touchRemoteFlag(ctx, target, "/tmp/vev-slow-preview-off") | |
| 176 | - | ||
| 177 | 176 | if err := detach(ctx, resumed); err != nil { | |
| 178 | - | _ = resumed.Close() | |
| 179 | - | return fmt.Errorf("stop resumed session: %w", err) | |
| 177 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("stop resumed session: %w", err) | |
| 180 | 178 | } | |
| 181 | - | _ = resumed.Close() | |
| 179 | + | return resumedTarget, nil | |
| 180 | + | } | |
| 182 | 181 | ||
| 182 | + | func runUDPPhase(ctx context.Context, target string) error { | |
| 183 | 183 | udp, err := openTransport(ctx, ports.RemoteTransportUDP, target) | |
| 184 | 184 | if err != nil { | |
| 185 | 185 | return fmt.Errorf("open UDP transport: %w", err) | |
| 186 | 186 | } | |
| 187 | + | defer func() { _ = udp.Close() }() | |
| 187 | 188 | if err := attachAndCheck(ctx, udp, ports.Hello{ | |
| 188 | - | Intent: ports.IntentNew, | |
| 189 | - | Name: "udp-harness", | |
| 190 | - | Env: localEnvironment(), | |
| 191 | - | Cwd: "/tmp/local-cwd", | |
| 192 | - | TermEnv: "xterm-256color", | |
| 193 | - | TrueColor: true, | |
| 189 | + | Intent: ports.IntentNew, Name: "udp-harness", Env: localEnvironment(), Cwd: "/tmp/local-cwd", | |
| 190 | + | TermEnv: "xterm-256color", TrueColor: true, | |
| 194 | 191 | }); err != nil { | |
| 195 | - | _ = udp.Close() | |
| 196 | 192 | return fmt.Errorf("UDP attach: %w", err) | |
| 197 | 193 | } | |
| 198 | 194 | if err := detach(ctx, udp); err != nil { | |
| 199 | - | _ = udp.Close() | |
| 200 | 195 | return fmt.Errorf("UDP detach: %w", err) | |
| 201 | 196 | } | |
| 202 | - | _ = udp.Close() | |
| 197 | + | return nil | |
| 198 | + | } | |
| 203 | 199 | ||
| 200 | + | func runCatalogFailurePhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient) error { | |
| 204 | 201 | if err := touchRemoteFlag(ctx, target, "/tmp/vev-bad-catalog"); err != nil { | |
| 205 | 202 | return fmt.Errorf("enable mismatched catalog: %w", err) | |
| 206 | 203 | } | |
| 207 | 204 | if _, err := catalog.List(ctx, target); err == nil { | |
| 208 | 205 | return errors.New("version-mismatched catalog unexpectedly decoded") | |
| 209 | 206 | } | |
| 210 | - | _ = touchRemoteFlag(ctx, target, "/tmp/vev-bad-catalog-off") | |
| 211 | - | ||
| 212 | - | if _, err := catalog.List(ctx, "test@unreachable.invalid"); err == nil { | |
| 213 | - | return errors.New("unreachable catalog unexpectedly succeeded") | |
| 207 | + | if err := touchRemoteFlag(ctx, target, "/tmp/vev-bad-catalog-off"); err != nil { | |
| 208 | + | return fmt.Errorf("disable mismatched catalog: %w", err) | |
| 214 | 209 | } | |
| 210 | + | return nil | |
| 211 | + | } | |
| 215 | 212 | ||
| 213 | + | func runStaleTargetPhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, staleTarget domain.RemoteSessionTarget) error { | |
| 216 | 214 | if err := runRemoteCommand(ctx, target, "vev", "kill", sessionName); err != nil { | |
| 217 | 215 | return fmt.Errorf("destroy session for stale-target test: %w", err) | |
| 218 | 216 | } | |
| @@ -223,15 +221,31 @@ func run() error { | |||
| 223 | 221 | if err != nil { | |
| 224 | 222 | return fmt.Errorf("open stale-target transport: %w", err) | |
| 225 | 223 | } | |
| 226 | - | if err := expectNoSuchTarget(ctx, stale, resumedTarget); err != nil { | |
| 227 | - | _ = stale.Close() | |
| 224 | + | defer func() { _ = stale.Close() }() | |
| 225 | + | if err := expectNoSuchTarget(ctx, stale, staleTarget); err != nil { | |
| 228 | 226 | return fmt.Errorf("stale target fence: %w", err) | |
| 229 | 227 | } | |
| 230 | - | _ = stale.Close() | |
| 231 | - | ||
| 232 | 228 | return nil | |
| 233 | 229 | } | |
| 234 | 230 | ||
| 231 | + | func removedCatalogTabID(before, after []ports.RemoteCatalogTab) (string, bool) { | |
| 232 | + | remaining := make(map[string]struct{}, len(after)) | |
| 233 | + | for _, tab := range after { | |
| 234 | + | remaining[tab.ID] = struct{}{} | |
| 235 | + | } | |
| 236 | + | var removed string | |
| 237 | + | for _, tab := range before { | |
| 238 | + | if _, exists := remaining[tab.ID]; exists { | |
| 239 | + | continue | |
| 240 | + | } | |
| 241 | + | if removed != "" { | |
| 242 | + | return "", false | |
| 243 | + | } | |
| 244 | + | removed = tab.ID | |
| 245 | + | } | |
| 246 | + | return removed, removed != "" | |
| 247 | + | } | |
| 248 | + | ||
| 235 | 249 | func localEnvironment() []string { | |
| 236 | 250 | return []string{ | |
| 237 | 251 | "VEV_TEST_ENV=local", | |
| @@ -243,21 +257,68 @@ func localEnvironment() []string { | |||
| 243 | 257 | } | |
| 244 | 258 | } | |
| 245 | 259 | ||
| 246 | - | func openTransport(ctx context.Context, mode ports.RemoteTransportMode, target string) (ports.Transport, error) { | |
| 260 | + | type harnessFrame struct { | |
| 261 | + | frame ports.Frame | |
| 262 | + | err error | |
| 263 | + | } | |
| 264 | + | ||
| 265 | + | type harnessTransport struct { | |
| 266 | + | ports.Transport | |
| 267 | + | frames chan harnessFrame | |
| 268 | + | stop chan struct{} | |
| 269 | + | closeOnce sync.Once | |
| 270 | + | closeErr error | |
| 271 | + | } | |
| 272 | + | ||
| 273 | + | func newHarnessTransport(transport ports.Transport) *harnessTransport { | |
| 274 | + | h := &harnessTransport{Transport: transport, frames: make(chan harnessFrame, 16), stop: make(chan struct{})} | |
| 275 | + | go h.receiveLoop() | |
| 276 | + | return h | |
| 277 | + | } | |
| 278 | + | ||
| 279 | + | func (h *harnessTransport) receiveLoop() { | |
| 280 | + | for { | |
| 281 | + | frame, err := h.Transport.Recv() | |
| 282 | + | select { | |
| 283 | + | case h.frames <- harnessFrame{frame: frame, err: err}: | |
| 284 | + | case <-h.stop: | |
| 285 | + | return | |
| 286 | + | } | |
| 287 | + | if err != nil { | |
| 288 | + | return | |
| 289 | + | } | |
| 290 | + | } | |
| 291 | + | } | |
| 292 | + | ||
| 293 | + | func (h *harnessTransport) Close() error { | |
| 294 | + | h.closeOnce.Do(func() { | |
| 295 | + | close(h.stop) | |
| 296 | + | h.closeErr = h.Transport.Close() | |
| 297 | + | }) | |
| 298 | + | return h.closeErr | |
| 299 | + | } | |
| 300 | + | ||
| 301 | + | func openTransport(ctx context.Context, mode ports.RemoteTransportMode, target string) (*harnessTransport, error) { | |
| 302 | + | var transport ports.Transport | |
| 303 | + | var err error | |
| 247 | 304 | switch mode { | |
| 248 | 305 | case ports.RemoteTransportStdio: | |
| 249 | - | return sshstdio.DialContext(ctx, target, "", slog.New(slog.DiscardHandler)) | |
| 306 | + | transport, err = sshstdio.DialContext(ctx, target, "", slog.New(slog.DiscardHandler)) | |
| 250 | 307 | case ports.RemoteTransportUDP: | |
| 251 | 308 | dialer := dgram.NewRemoteDialerWithLogger(target, "", slog.New(slog.DiscardHandler)) | |
| 252 | 309 | dialer.BootstrapTimeout = 8 * time.Second | |
| 253 | 310 | dialer.ProbeTimeout = 3 * time.Second | |
| 254 | - | return dialer.Dial(ctx) | |
| 311 | + | transport, err = dialer.Dial(ctx) | |
| 255 | 312 | default: | |
| 256 | 313 | return nil, fmt.Errorf("unsupported transport %q", mode) | |
| 257 | 314 | } | |
| 315 | + | if err != nil { | |
| 316 | + | return nil, err | |
| 317 | + | } | |
| 318 | + | return newHarnessTransport(transport), nil | |
| 258 | 319 | } | |
| 259 | 320 | ||
| 260 | - | func attachAndCheck(ctx context.Context, tr ports.Transport, hello ports.Hello) error { | |
| 321 | + | func attachAndCheck(ctx context.Context, tr *harnessTransport, hello ports.Hello) error { | |
| 261 | 322 | hello.Version = ports.ProtocolVersion | |
| 262 | 323 | hello.Size = domain.Size{Cols: 80, Rows: 24} | |
| 263 | 324 | if hello.EnvironmentPolicy == 0 { | |
| @@ -287,7 +348,7 @@ func attachAndCheck(ctx context.Context, tr ports.Transport, hello ports.Hello) | |||
| 287 | 348 | return nil | |
| 288 | 349 | } | |
| 289 | 350 | ||
| 290 | - | func sendInputAndAwait(ctx context.Context, tr ports.Transport, command, want string) error { | |
| 351 | + | func sendInputAndAwait(ctx context.Context, tr *harnessTransport, command, want string) error { | |
| 291 | 352 | if err := tr.Send(ports.Frame{Type: ports.MsgInput, Payload: ports.MarshalInput(ports.Input{Data: []byte(command + "\n")})}); err != nil { | |
| 292 | 353 | return err | |
| 293 | 354 | } | |
| @@ -331,7 +392,7 @@ func sendInputAndAwait(ctx context.Context, tr ports.Transport, command, want st | |||
| 331 | 392 | } | |
| 332 | 393 | } | |
| 333 | 394 | ||
| 334 | - | func sendCommand(ctx context.Context, tr ports.Transport, slug string) error { | |
| 395 | + | func sendCommand(ctx context.Context, tr *harnessTransport, slug string) error { | |
| 335 | 396 | id := requestID.Add(1) | |
| 336 | 397 | request := ports.CommandRequest{ | |
| 337 | 398 | Version: ports.ProtocolVersion, | |
| @@ -367,7 +428,7 @@ func sendCommand(ctx context.Context, tr ports.Transport, slug string) error { | |||
| 367 | 428 | } | |
| 368 | 429 | } | |
| 369 | 430 | ||
| 370 | - | func detach(ctx context.Context, tr ports.Transport) error { | |
| 431 | + | func detach(ctx context.Context, tr *harnessTransport) error { | |
| 371 | 432 | if err := tr.Send(ports.Frame{Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}); err != nil { | |
| 372 | 433 | return err | |
| 373 | 434 | } | |
| @@ -384,7 +445,7 @@ func detach(ctx context.Context, tr ports.Transport) error { | |||
| 384 | 445 | } | |
| 385 | 446 | } | |
| 386 | 447 | ||
| 387 | - | func expectNoSuchTarget(ctx context.Context, tr ports.Transport, target domain.RemoteSessionTarget) error { | |
| 448 | + | func expectNoSuchTarget(ctx context.Context, tr *harnessTransport, target domain.RemoteSessionTarget) error { | |
| 388 | 449 | target.Endpoint = targetEndpoint(target.Endpoint) | |
| 389 | 450 | target.DisplayOrigin = "remote" | |
| 390 | 451 | target.SessionName = sessionName | |
| @@ -528,22 +589,11 @@ func runRemoteCommand(ctx context.Context, target string, command ...string) err | |||
| 528 | 589 | return cmd.Run() | |
| 529 | 590 | } | |
| 530 | 591 | ||
| 531 | - | func receiveWithTimeout(ctx context.Context, tr ports.Transport) (ports.Frame, error) { | |
| 532 | - | result := make(chan struct { | |
| 533 | - | frame ports.Frame | |
| 534 | - | err error | |
| 535 | - | }, 1) | |
| 536 | - | go func() { | |
| 537 | - | frame, err := tr.Recv() | |
| 538 | - | result <- struct { | |
| 539 | - | frame ports.Frame | |
| 540 | - | err error | |
| 541 | - | }{frame: frame, err: err} | |
| 542 | - | }() | |
| 592 | + | func receiveWithTimeout(ctx context.Context, tr *harnessTransport) (ports.Frame, error) { | |
| 543 | 593 | select { | |
| 544 | 594 | case <-ctx.Done(): | |
| 545 | 595 | return ports.Frame{}, ctx.Err() | |
| 546 | - | case outcome := <-result: | |
| 596 | + | case outcome := <-tr.frames: | |
| 547 | 597 | return outcome.frame, outcome.err | |
| 548 | 598 | } | |
| 549 | 599 | } | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | "os" | 9 | "os" | ||
| 10 | "os/exec" | 10 | "os/exec" | ||
| 11 | "strings" | 11 | "strings" | ||
| 12 | + | "sync" | |||
| 12 | "sync/atomic" | 13 | "sync/atomic" | ||
| 13 | "time" | 14 | "time" | ||
| 14 | 15 | ||||
| @@ -44,175 +45,172 @@ func run() error { | |||||
| 44 | 45 | ||||
| 45 | catalog := remoteadapter.NewCatalogClient() | 46 | catalog := remoteadapter.NewCatalogClient() | ||
| 46 | preview := remoteadapter.NewPreviewClient() | 47 | preview := remoteadapter.NewPreviewClient() | ||
| 48 | + | if err := runLiveStdioPhase(ctx, target, catalog, preview); err != nil { | |||
| 49 | + | return err | |||
| 50 | + | } | |||
| 51 | + | resumedTarget, err := runRestartResumePhase(ctx, target, catalog, preview) | |||
| 52 | + | if err != nil { | |||
| 53 | + | return err | |||
| 54 | + | } | |||
| 55 | + | if err := runUDPPhase(ctx, target); err != nil { | |||
| 56 | + | return err | |||
| 57 | + | } | |||
| 58 | + | if err := runCatalogFailurePhase(ctx, target, catalog); err != nil { | |||
| 59 | + | return err | |||
| 60 | + | } | |||
| 61 | + | if _, err := catalog.List(ctx, "test@unreachable.invalid"); err == nil { | |||
| 62 | + | return errors.New("unreachable catalog unexpectedly succeeded") | |||
| 63 | + | } | |||
| 64 | + | return runStaleTargetPhase(ctx, target, catalog, resumedTarget) | |||
| 65 | + | } | |||
| 47 | 66 | ||||
| 67 | + | func runLiveStdioPhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, preview ports.RemotePreviewClient) error { | |||
| 48 | stdio, err := openTransport(ctx, ports.RemoteTransportStdio, target) | 68 | stdio, err := openTransport(ctx, ports.RemoteTransportStdio, target) | ||
| 49 | if err != nil { | 69 | if err != nil { | ||
| 50 | return fmt.Errorf("open stdio transport: %w", err) | 70 | return fmt.Errorf("open stdio transport: %w", err) | ||
| 51 | } | 71 | } | ||
| 72 | + | defer func() { _ = stdio.Close() }() | |||
| 52 | if err := attachAndCheck(ctx, stdio, ports.Hello{ | 73 | if err := attachAndCheck(ctx, stdio, ports.Hello{ | ||
| 53 | - | Intent: ports.IntentNew, | 74 | + | Intent: ports.IntentNew, Name: sessionName, Env: localEnvironment(), Cwd: "/tmp/local-cwd", |
| 54 | - | Name: sessionName, | 75 | + | TermEnv: "xterm-256color", TrueColor: true, |
| 55 | - | Env: localEnvironment(), | |||
| 56 | - | Cwd: "/tmp/local-cwd", | |||
| 57 | - | TermEnv: "xterm-256color", | |||
| 58 | - | TrueColor: true, | |||
| 59 | }); err != nil { | 76 | }); err != nil { | ||
| 60 | - | _ = stdio.Close() | |||
| 61 | return fmt.Errorf("stdio live attach: %w", err) | 77 | return fmt.Errorf("stdio live attach: %w", err) | ||
| 62 | } | 78 | } | ||
| 63 | if err := sendInputAndAwait(ctx, stdio, "printf 'VEV_REMOTE_HARNESS_ENV=%s:%s:%s:%s\\n' \"$VEV_TEST_ENV\" \"$XDG_RUNTIME_DIR\" \"$WAYLAND_DISPLAY\" \"$VEV\"", "local:/run/local:wayland-local:"); err != nil { | 79 | if err := sendInputAndAwait(ctx, stdio, "printf 'VEV_REMOTE_HARNESS_ENV=%s:%s:%s:%s\\n' \"$VEV_TEST_ENV\" \"$XDG_RUNTIME_DIR\" \"$WAYLAND_DISPLAY\" \"$VEV\"", "local:/run/local:wayland-local:"); err != nil { | ||
| 64 | - | _ = stdio.Close() | |||
| 65 | return fmt.Errorf("direct client environment: %w", err) | 80 | return fmt.Errorf("direct client environment: %w", err) | ||
| 66 | } | 81 | } | ||
| 67 | - | ||||
| 68 | live, err := waitForSession(ctx, catalog, target, sessionName, "running") | 82 | live, err := waitForSession(ctx, catalog, target, sessionName, "running") | ||
| 69 | if err != nil { | 83 | if err != nil { | ||
| 70 | - | _ = stdio.Close() | |||
| 71 | return fmt.Errorf("live catalog: %w", err) | 84 | return fmt.Errorf("live catalog: %w", err) | ||
| 72 | } | 85 | } | ||
| 73 | liveTarget, err := targetForSession(target, live, false) | 86 | liveTarget, err := targetForSession(target, live, false) | ||
| 74 | if err != nil { | 87 | if err != nil { | ||
| 75 | - | _ = stdio.Close() | |||
| 76 | return fmt.Errorf("live target: %w", err) | 88 | return fmt.Errorf("live target: %w", err) | ||
| 77 | } | 89 | } | ||
| 78 | if _, err := preview.Preview(ctx, liveTarget, 40, 8); err != nil { | 90 | if _, err := preview.Preview(ctx, liveTarget, 40, 8); err != nil { | ||
| 79 | - | _ = stdio.Close() | |||
| 80 | return fmt.Errorf("live preview: %w", err) | 91 | return fmt.Errorf("live preview: %w", err) | ||
| 81 | } | 92 | } | ||
| 82 | - | ||||
| 83 | if err := sendCommand(ctx, stdio, "new-tab"); err != nil { | 93 | if err := sendCommand(ctx, stdio, "new-tab"); err != nil { | ||
| 84 | - | _ = stdio.Close() | |||
| 85 | return fmt.Errorf("create second tab: %w", err) | 94 | return fmt.Errorf("create second tab: %w", err) | ||
| 86 | } | 95 | } | ||
| 87 | liveWithTabs, err := waitForSessionWithTabCount(ctx, catalog, target, sessionName, "running", 2) | 96 | liveWithTabs, err := waitForSessionWithTabCount(ctx, catalog, target, sessionName, "running", 2) | ||
| 88 | if err != nil { | 97 | if err != nil { | ||
| 89 | - | _ = stdio.Close() | |||
| 90 | return fmt.Errorf("multi-tab catalog: %w", err) | 98 | return fmt.Errorf("multi-tab catalog: %w", err) | ||
| 91 | } | 99 | } | ||
| 92 | - | removedTargets := make([]domain.RemoteSessionTarget, 0, len(ports.CatalogTabs(liveWithTabs))) | 100 | + | beforeTabs := ports.CatalogTabs(liveWithTabs) |
| 93 | - | for _, tab := range ports.CatalogTabs(liveWithTabs) { | |||
| 94 | - | candidate := liveWithTabs | |||
| 95 | - | candidate.ActiveTabID = tab.ID | |||
| 96 | - | removed, targetErr := targetForSession(target, candidate, false) | |||
| 97 | - | if targetErr != nil { | |||
| 98 | - | _ = stdio.Close() | |||
| 99 | - | return fmt.Errorf("removed-tab target: %w", targetErr) | |||
| 100 | - | } | |||
| 101 | - | removedTargets = append(removedTargets, removed) | |||
| 102 | - | } | |||
| 103 | if err := sendCommand(ctx, stdio, "close-tab"); err != nil { | 101 | if err := sendCommand(ctx, stdio, "close-tab"); err != nil { | ||
| 104 | - | _ = stdio.Close() | |||
| 105 | return fmt.Errorf("remove tab: %w", err) | 102 | return fmt.Errorf("remove tab: %w", err) | ||
| 106 | } | 103 | } | ||
| 107 | - | if _, err := waitForSessionWithTabCount(ctx, catalog, target, sessionName, "running", 1); err != nil { | 104 | + | after, err := waitForSessionWithTabCount(ctx, catalog, target, sessionName, "running", 1) |
| 108 | - | _ = stdio.Close() | 105 | + | if err != nil { |
| 109 | return fmt.Errorf("removed-tab catalog: %w", err) | 106 | return fmt.Errorf("removed-tab catalog: %w", err) | ||
| 110 | } | 107 | } | ||
| 111 | - | removed := false | 108 | + | removedID, ok := removedCatalogTabID(beforeTabs, ports.CatalogTabs(after)) |
| 112 | - | for _, removedTarget := range removedTargets { | 109 | + | if !ok { |
| 113 | - | if _, previewErr := preview.Preview(ctx, removedTarget, 40, 8); previewErr != nil { | 110 | + | return errors.New("removed-tab catalog did not identify exactly one removed tab") |
| 114 | - | removed = true | 111 | + | } |
| 115 | - | break | 112 | + | removedSession := liveWithTabs |
| 116 | - | } | 113 | + | removedSession.ActiveTabID = removedID |
| 114 | + | removedTarget, err := targetForSession(target, removedSession, false) | |||
| 115 | + | if err != nil { | |||
| 116 | + | return fmt.Errorf("removed-tab target: %w", err) | |||
| 117 | } | 117 | } | ||
| 118 | - | if !removed { | 118 | + | if _, err := preview.Preview(ctx, removedTarget, 40, 8); err == nil { |
| 119 | - | _ = stdio.Close() | |||
| 120 | return errors.New("removed tab unexpectedly produced a preview") | 119 | return errors.New("removed tab unexpectedly produced a preview") | ||
| 121 | } | 120 | } | ||
| 122 | - | 121 | + | survivingTarget, err := targetForSession(target, after, false) | |
| 122 | + | if err != nil { | |||
| 123 | + | return fmt.Errorf("surviving-tab target: %w", err) | |||
| 124 | + | } | |||
| 125 | + | if _, err := preview.Preview(ctx, survivingTarget, 40, 8); err != nil { | |||
| 126 | + | return fmt.Errorf("surviving tab preview: %w", err) | |||
| 127 | + | } | |||
| 123 | if err := detach(ctx, stdio); err != nil { | 128 | if err := detach(ctx, stdio); err != nil { | ||
| 124 | - | _ = stdio.Close() | |||
| 125 | return fmt.Errorf("stop session: %w", err) | 129 | return fmt.Errorf("stop session: %w", err) | ||
| 126 | } | 130 | } | ||
| 127 | - | _ = stdio.Close() | 131 | + | return nil |
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | func runRestartResumePhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, preview ports.RemotePreviewClient) (domain.RemoteSessionTarget, error) { | |||
| 128 | if err := runRemoteCommand(ctx, target, "vev", "kill", "--daemon"); err != nil { | 135 | if err := runRemoteCommand(ctx, target, "vev", "kill", "--daemon"); err != nil { | ||
| 129 | - | return fmt.Errorf("stop remote daemon: %w", err) | 136 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("stop remote daemon: %w", err) |
| 130 | } | 137 | } | ||
| 131 | if err := runRemoteCommand(ctx, target, "vev", "--daemon-launcher"); err != nil { | 138 | if err := runRemoteCommand(ctx, target, "vev", "--daemon-launcher"); err != nil { | ||
| 132 | - | return fmt.Errorf("restart remote daemon: %w", err) | 139 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("restart remote daemon: %w", err) |
| 133 | } | 140 | } | ||
| 134 | - | ||||
| 135 | resumedCatalog, err := waitForSession(ctx, catalog, target, sessionName, "running") | 141 | resumedCatalog, err := waitForSession(ctx, catalog, target, sessionName, "running") | ||
| 136 | if err != nil { | 142 | if err != nil { | ||
| 137 | - | return fmt.Errorf("resumed catalog: %w", err) | 143 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("resumed catalog: %w", err) |
| 138 | } | 144 | } | ||
| 139 | resumedTarget, err := targetForSession(target, resumedCatalog, false) | 145 | resumedTarget, err := targetForSession(target, resumedCatalog, false) | ||
| 140 | if err != nil { | 146 | if err != nil { | ||
| 141 | - | return fmt.Errorf("resumed target: %w", err) | 147 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("resumed target: %w", err) |
| 142 | } | 148 | } | ||
| 143 | resumed, err := openTransport(ctx, ports.RemoteTransportStdio, target) | 149 | resumed, err := openTransport(ctx, ports.RemoteTransportStdio, target) | ||
| 144 | if err != nil { | 150 | if err != nil { | ||
| 145 | - | return fmt.Errorf("open restart stdio transport: %w", err) | 151 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("open restart stdio transport: %w", err) |
| 146 | } | 152 | } | ||
| 153 | + | defer func() { _ = resumed.Close() }() | |||
| 147 | if err := attachAndCheck(ctx, resumed, ports.Hello{ | 154 | if err := attachAndCheck(ctx, resumed, ports.Hello{ | ||
| 148 | - | Intent: ports.IntentResume, | 155 | + | Intent: ports.IntentResume, Name: sessionName, Env: localEnvironment(), Cwd: "/tmp/local-cwd", |
| 149 | - | Name: sessionName, | 156 | + | TermEnv: "xterm-256color", TrueColor: true, RemoteTarget: &resumedTarget, |
| 150 | - | Env: localEnvironment(), | |||
| 151 | - | Cwd: "/tmp/local-cwd", | |||
| 152 | - | TermEnv: "xterm-256color", | |||
| 153 | - | TrueColor: true, | |||
| 154 | - | RemoteTarget: &resumedTarget, | |||
| 155 | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | 157 | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | ||
| 156 | }); err != nil { | 158 | }); err != nil { | ||
| 157 | - | _ = resumed.Close() | 159 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("exact restart resume: %w", err) |
| 158 | - | return fmt.Errorf("exact restart resume: %w", err) | |||
| 159 | } | 160 | } | ||
| 160 | if _, err := preview.Preview(ctx, resumedTarget, 40, 8); err != nil { | 161 | if _, err := preview.Preview(ctx, resumedTarget, 40, 8); err != nil { | ||
| 161 | - | _ = resumed.Close() | 162 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("restart preview: %w", err) |
| 162 | - | return fmt.Errorf("restart preview: %w", err) | |||
| 163 | } | 163 | } | ||
| 164 | if err := touchRemoteFlag(ctx, target, "/tmp/vev-slow-preview"); err != nil { | 164 | if err := touchRemoteFlag(ctx, target, "/tmp/vev-slow-preview"); err != nil { | ||
| 165 | - | _ = resumed.Close() | 165 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("enable slow preview: %w", err) |
| 166 | - | return fmt.Errorf("enable slow preview: %w", err) | |||
| 167 | } | 166 | } | ||
| 168 | slowCtx, slowCancel := context.WithTimeout(ctx, 150*time.Millisecond) | 167 | slowCtx, slowCancel := context.WithTimeout(ctx, 150*time.Millisecond) | ||
| 169 | _, slowErr := preview.Preview(slowCtx, resumedTarget, 40, 8) | 168 | _, slowErr := preview.Preview(slowCtx, resumedTarget, 40, 8) | ||
| 170 | slowCancel() | 169 | slowCancel() | ||
| 170 | + | if err := touchRemoteFlag(ctx, target, "/tmp/vev-slow-preview-off"); err != nil { | |||
| 171 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("disable slow preview: %w", err) | |||
| 172 | + | } | |||
| 171 | if slowErr == nil { | 173 | if slowErr == nil { | ||
| 172 | - | _ = resumed.Close() | 174 | + | return domain.RemoteSessionTarget{}, errors.New("slow preview did not honor cancellation") |
| 173 | - | return errors.New("slow preview did not honor cancellation") | |||
| 174 | } | 175 | } | ||
| 175 | - | _ = touchRemoteFlag(ctx, target, "/tmp/vev-slow-preview-off") | |||
| 176 | - | ||||
| 177 | if err := detach(ctx, resumed); err != nil { | 176 | if err := detach(ctx, resumed); err != nil { | ||
| 178 | - | _ = resumed.Close() | 177 | + | return domain.RemoteSessionTarget{}, fmt.Errorf("stop resumed session: %w", err) |
| 179 | - | return fmt.Errorf("stop resumed session: %w", err) | |||
| 180 | } | 178 | } | ||
| 181 | - | _ = resumed.Close() | 179 | + | return resumedTarget, nil |
| 180 | + | } | |||
| 182 | 181 | ||||
| 182 | + | func runUDPPhase(ctx context.Context, target string) error { | |||
| 183 | udp, err := openTransport(ctx, ports.RemoteTransportUDP, target) | 183 | udp, err := openTransport(ctx, ports.RemoteTransportUDP, target) | ||
| 184 | if err != nil { | 184 | if err != nil { | ||
| 185 | return fmt.Errorf("open UDP transport: %w", err) | 185 | return fmt.Errorf("open UDP transport: %w", err) | ||
| 186 | } | 186 | } | ||
| 187 | + | defer func() { _ = udp.Close() }() | |||
| 187 | if err := attachAndCheck(ctx, udp, ports.Hello{ | 188 | if err := attachAndCheck(ctx, udp, ports.Hello{ | ||
| 188 | - | Intent: ports.IntentNew, | 189 | + | Intent: ports.IntentNew, Name: "udp-harness", Env: localEnvironment(), Cwd: "/tmp/local-cwd", |
| 189 | - | Name: "udp-harness", | 190 | + | TermEnv: "xterm-256color", TrueColor: true, |
| 190 | - | Env: localEnvironment(), | |||
| 191 | - | Cwd: "/tmp/local-cwd", | |||
| 192 | - | TermEnv: "xterm-256color", | |||
| 193 | - | TrueColor: true, | |||
| 194 | }); err != nil { | 191 | }); err != nil { | ||
| 195 | - | _ = udp.Close() | |||
| 196 | return fmt.Errorf("UDP attach: %w", err) | 192 | return fmt.Errorf("UDP attach: %w", err) | ||
| 197 | } | 193 | } | ||
| 198 | if err := detach(ctx, udp); err != nil { | 194 | if err := detach(ctx, udp); err != nil { | ||
| 199 | - | _ = udp.Close() | |||
| 200 | return fmt.Errorf("UDP detach: %w", err) | 195 | return fmt.Errorf("UDP detach: %w", err) | ||
| 201 | } | 196 | } | ||
| 202 | - | _ = udp.Close() | 197 | + | return nil |
| 198 | + | } | |||
| 203 | 199 | ||||
| 200 | + | func runCatalogFailurePhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient) error { | |||
| 204 | if err := touchRemoteFlag(ctx, target, "/tmp/vev-bad-catalog"); err != nil { | 201 | if err := touchRemoteFlag(ctx, target, "/tmp/vev-bad-catalog"); err != nil { | ||
| 205 | return fmt.Errorf("enable mismatched catalog: %w", err) | 202 | return fmt.Errorf("enable mismatched catalog: %w", err) | ||
| 206 | } | 203 | } | ||
| 207 | if _, err := catalog.List(ctx, target); err == nil { | 204 | if _, err := catalog.List(ctx, target); err == nil { | ||
| 208 | return errors.New("version-mismatched catalog unexpectedly decoded") | 205 | return errors.New("version-mismatched catalog unexpectedly decoded") | ||
| 209 | } | 206 | } | ||
| 210 | - | _ = touchRemoteFlag(ctx, target, "/tmp/vev-bad-catalog-off") | 207 | + | if err := touchRemoteFlag(ctx, target, "/tmp/vev-bad-catalog-off"); err != nil { |
| 211 | - | 208 | + | return fmt.Errorf("disable mismatched catalog: %w", err) | |
| 212 | - | if _, err := catalog.List(ctx, "test@unreachable.invalid"); err == nil { | |||
| 213 | - | return errors.New("unreachable catalog unexpectedly succeeded") | |||
| 214 | } | 209 | } | ||
| 210 | + | return nil | |||
| 211 | + | } | |||
| 215 | 212 | ||||
| 213 | + | func runStaleTargetPhase(ctx context.Context, target string, catalog ports.RemoteCatalogClient, staleTarget domain.RemoteSessionTarget) error { | |||
| 216 | if err := runRemoteCommand(ctx, target, "vev", "kill", sessionName); err != nil { | 214 | if err := runRemoteCommand(ctx, target, "vev", "kill", sessionName); err != nil { | ||
| 217 | return fmt.Errorf("destroy session for stale-target test: %w", err) | 215 | return fmt.Errorf("destroy session for stale-target test: %w", err) | ||
| 218 | } | 216 | } | ||
| @@ -223,15 +221,31 @@ func run() error { | |||||
| 223 | if err != nil { | 221 | if err != nil { | ||
| 224 | return fmt.Errorf("open stale-target transport: %w", err) | 222 | return fmt.Errorf("open stale-target transport: %w", err) | ||
| 225 | } | 223 | } | ||
| 226 | - | if err := expectNoSuchTarget(ctx, stale, resumedTarget); err != nil { | 224 | + | defer func() { _ = stale.Close() }() |
| 227 | - | _ = stale.Close() | 225 | + | if err := expectNoSuchTarget(ctx, stale, staleTarget); err != nil { |
| 228 | return fmt.Errorf("stale target fence: %w", err) | 226 | return fmt.Errorf("stale target fence: %w", err) | ||
| 229 | } | 227 | } | ||
| 230 | - | _ = stale.Close() | |||
| 231 | - | ||||
| 232 | return nil | 228 | return nil | ||
| 233 | } | 229 | } | ||
| 234 | 230 | ||||
| 231 | + | func removedCatalogTabID(before, after []ports.RemoteCatalogTab) (string, bool) { | |||
| 232 | + | remaining := make(map[string]struct{}, len(after)) | |||
| 233 | + | for _, tab := range after { | |||
| 234 | + | remaining[tab.ID] = struct{}{} | |||
| 235 | + | } | |||
| 236 | + | var removed string | |||
| 237 | + | for _, tab := range before { | |||
| 238 | + | if _, exists := remaining[tab.ID]; exists { | |||
| 239 | + | continue | |||
| 240 | + | } | |||
| 241 | + | if removed != "" { | |||
| 242 | + | return "", false | |||
| 243 | + | } | |||
| 244 | + | removed = tab.ID | |||
| 245 | + | } | |||
| 246 | + | return removed, removed != "" | |||
| 247 | + | } | |||
| 248 | + | ||||
| 235 | func localEnvironment() []string { | 249 | func localEnvironment() []string { | ||
| 236 | return []string{ | 250 | return []string{ | ||
| 237 | "VEV_TEST_ENV=local", | 251 | "VEV_TEST_ENV=local", | ||
| @@ -243,21 +257,68 @@ func localEnvironment() []string { | |||||
| 243 | } | 257 | } | ||
| 244 | } | 258 | } | ||
| 245 | 259 | ||||
| 246 | - | func openTransport(ctx context.Context, mode ports.RemoteTransportMode, target string) (ports.Transport, error) { | 260 | + | type harnessFrame struct { |
| 261 | + | frame ports.Frame | |||
| 262 | + | err error | |||
| 263 | + | } | |||
| 264 | + | ||||
| 265 | + | type harnessTransport struct { | |||
| 266 | + | ports.Transport | |||
| 267 | + | frames chan harnessFrame | |||
| 268 | + | stop chan struct{} | |||
| 269 | + | closeOnce sync.Once | |||
| 270 | + | closeErr error | |||
| 271 | + | } | |||
| 272 | + | ||||
| 273 | + | func newHarnessTransport(transport ports.Transport) *harnessTransport { | |||
| 274 | + | h := &harnessTransport{Transport: transport, frames: make(chan harnessFrame, 16), stop: make(chan struct{})} | |||
| 275 | + | go h.receiveLoop() | |||
| 276 | + | return h | |||
| 277 | + | } | |||
| 278 | + | ||||
| 279 | + | func (h *harnessTransport) receiveLoop() { | |||
| 280 | + | for { | |||
| 281 | + | frame, err := h.Transport.Recv() | |||
| 282 | + | select { | |||
| 283 | + | case h.frames <- harnessFrame{frame: frame, err: err}: | |||
| 284 | + | case <-h.stop: | |||
| 285 | + | return | |||
| 286 | + | } | |||
| 287 | + | if err != nil { | |||
| 288 | + | return | |||
| 289 | + | } | |||
| 290 | + | } | |||
| 291 | + | } | |||
| 292 | + | ||||
| 293 | + | func (h *harnessTransport) Close() error { | |||
| 294 | + | h.closeOnce.Do(func() { | |||
| 295 | + | close(h.stop) | |||
| 296 | + | h.closeErr = h.Transport.Close() | |||
| 297 | + | }) | |||
| 298 | + | return h.closeErr | |||
| 299 | + | } | |||
| 300 | + | ||||
| 301 | + | func openTransport(ctx context.Context, mode ports.RemoteTransportMode, target string) (*harnessTransport, error) { | |||
| 302 | + | var transport ports.Transport | |||
| 303 | + | var err error | |||
| 247 | switch mode { | 304 | switch mode { | ||
| 248 | case ports.RemoteTransportStdio: | 305 | case ports.RemoteTransportStdio: | ||
| 249 | - | return sshstdio.DialContext(ctx, target, "", slog.New(slog.DiscardHandler)) | 306 | + | transport, err = sshstdio.DialContext(ctx, target, "", slog.New(slog.DiscardHandler)) |
| 250 | case ports.RemoteTransportUDP: | 307 | case ports.RemoteTransportUDP: | ||
| 251 | dialer := dgram.NewRemoteDialerWithLogger(target, "", slog.New(slog.DiscardHandler)) | 308 | dialer := dgram.NewRemoteDialerWithLogger(target, "", slog.New(slog.DiscardHandler)) | ||
| 252 | dialer.BootstrapTimeout = 8 * time.Second | 309 | dialer.BootstrapTimeout = 8 * time.Second | ||
| 253 | dialer.ProbeTimeout = 3 * time.Second | 310 | dialer.ProbeTimeout = 3 * time.Second | ||
| 254 | - | return dialer.Dial(ctx) | 311 | + | transport, err = dialer.Dial(ctx) |
| 255 | default: | 312 | default: | ||
| 256 | return nil, fmt.Errorf("unsupported transport %q", mode) | 313 | return nil, fmt.Errorf("unsupported transport %q", mode) | ||
| 257 | } | 314 | } | ||
| 315 | + | if err != nil { | |||
| 316 | + | return nil, err | |||
| 317 | + | } | |||
| 318 | + | return newHarnessTransport(transport), nil | |||
| 258 | } | 319 | } | ||
| 259 | 320 | ||||
| 260 | - | func attachAndCheck(ctx context.Context, tr ports.Transport, hello ports.Hello) error { | 321 | + | func attachAndCheck(ctx context.Context, tr *harnessTransport, hello ports.Hello) error { |
| 261 | hello.Version = ports.ProtocolVersion | 322 | hello.Version = ports.ProtocolVersion | ||
| 262 | hello.Size = domain.Size{Cols: 80, Rows: 24} | 323 | hello.Size = domain.Size{Cols: 80, Rows: 24} | ||
| 263 | if hello.EnvironmentPolicy == 0 { | 324 | if hello.EnvironmentPolicy == 0 { | ||
| @@ -287,7 +348,7 @@ func attachAndCheck(ctx context.Context, tr ports.Transport, hello ports.Hello) | |||||
| 287 | return nil | 348 | return nil | ||
| 288 | } | 349 | } | ||
| 289 | 350 | ||||
| 290 | - | func sendInputAndAwait(ctx context.Context, tr ports.Transport, command, want string) error { | 351 | + | func sendInputAndAwait(ctx context.Context, tr *harnessTransport, command, want string) error { |
| 291 | if err := tr.Send(ports.Frame{Type: ports.MsgInput, Payload: ports.MarshalInput(ports.Input{Data: []byte(command + "\n")})}); err != nil { | 352 | if err := tr.Send(ports.Frame{Type: ports.MsgInput, Payload: ports.MarshalInput(ports.Input{Data: []byte(command + "\n")})}); err != nil { | ||
| 292 | return err | 353 | return err | ||
| 293 | } | 354 | } | ||
| @@ -331,7 +392,7 @@ func sendInputAndAwait(ctx context.Context, tr ports.Transport, command, want st | |||||
| 331 | } | 392 | } | ||
| 332 | } | 393 | } | ||
| 333 | 394 | ||||
| 334 | - | func sendCommand(ctx context.Context, tr ports.Transport, slug string) error { | 395 | + | func sendCommand(ctx context.Context, tr *harnessTransport, slug string) error { |
| 335 | id := requestID.Add(1) | 396 | id := requestID.Add(1) | ||
| 336 | request := ports.CommandRequest{ | 397 | request := ports.CommandRequest{ | ||
| 337 | Version: ports.ProtocolVersion, | 398 | Version: ports.ProtocolVersion, | ||
| @@ -367,7 +428,7 @@ func sendCommand(ctx context.Context, tr ports.Transport, slug string) error { | |||||
| 367 | } | 428 | } | ||
| 368 | } | 429 | } | ||
| 369 | 430 | ||||
| 370 | - | func detach(ctx context.Context, tr ports.Transport) error { | 431 | + | func detach(ctx context.Context, tr *harnessTransport) error { |
| 371 | if err := tr.Send(ports.Frame{Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}); err != nil { | 432 | if err := tr.Send(ports.Frame{Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}); err != nil { | ||
| 372 | return err | 433 | return err | ||
| 373 | } | 434 | } | ||
| @@ -384,7 +445,7 @@ func detach(ctx context.Context, tr ports.Transport) error { | |||||
| 384 | } | 445 | } | ||
| 385 | } | 446 | } | ||
| 386 | 447 | ||||
| 387 | - | func expectNoSuchTarget(ctx context.Context, tr ports.Transport, target domain.RemoteSessionTarget) error { | 448 | + | func expectNoSuchTarget(ctx context.Context, tr *harnessTransport, target domain.RemoteSessionTarget) error { |
| 388 | target.Endpoint = targetEndpoint(target.Endpoint) | 449 | target.Endpoint = targetEndpoint(target.Endpoint) | ||
| 389 | target.DisplayOrigin = "remote" | 450 | target.DisplayOrigin = "remote" | ||
| 390 | target.SessionName = sessionName | 451 | target.SessionName = sessionName | ||
| @@ -528,22 +589,11 @@ func runRemoteCommand(ctx context.Context, target string, command ...string) err | |||||
| 528 | return cmd.Run() | 589 | return cmd.Run() | ||
| 529 | } | 590 | } | ||
| 530 | 591 | ||||
| 531 | - | func receiveWithTimeout(ctx context.Context, tr ports.Transport) (ports.Frame, error) { | 592 | + | func receiveWithTimeout(ctx context.Context, tr *harnessTransport) (ports.Frame, error) { |
| 532 | - | result := make(chan struct { | |||
| 533 | - | frame ports.Frame | |||
| 534 | - | err error | |||
| 535 | - | }, 1) | |||
| 536 | - | go func() { | |||
| 537 | - | frame, err := tr.Recv() | |||
| 538 | - | result <- struct { | |||
| 539 | - | frame ports.Frame | |||
| 540 | - | err error | |||
| 541 | - | }{frame: frame, err: err} | |||
| 542 | - | }() | |||
| 543 | select { | 593 | select { | ||
| 544 | case <-ctx.Done(): | 594 | case <-ctx.Done(): | ||
| 545 | return ports.Frame{}, ctx.Err() | 595 | return ports.Frame{}, ctx.Err() | ||
| 546 | - | case outcome := <-result: | 596 | + | case outcome := <-tr.frames: |
| 547 | return outcome.frame, outcome.err | 597 | return outcome.frame, outcome.err | ||
| 548 | } | 598 | } | ||
| 549 | } | 599 | } | ||
M scripts/remote-picker-harness/run.sh +14 -2
| @@ -40,11 +40,20 @@ remote_container="${network}-remote" | |||
| 40 | 40 | context_dir="$(mktemp -d "${TMPDIR:-/tmp}/vev-remote-picker-harness.XXXXXX")" | |
| 41 | 41 | ||
| 42 | 42 | cleanup() { | |
| 43 | + | status=$? | |
| 44 | + | trap - EXIT INT TERM | |
| 43 | 45 | set +e | |
| 46 | + | if [ "$status" -ne 0 ]; then | |
| 47 | + | printf 'remote picker harness: local container logs:\n' >&2 | |
| 48 | + | docker logs "$local_container" >&2 | |
| 49 | + | printf 'remote picker harness: remote container logs:\n' >&2 | |
| 50 | + | docker logs "$remote_container" >&2 | |
| 51 | + | fi | |
| 44 | 52 | docker rm -f "$local_container" "$remote_container" >/dev/null 2>&1 | |
| 45 | 53 | docker network rm "$network" >/dev/null 2>&1 | |
| 46 | 54 | docker image rm "$image" >/dev/null 2>&1 | |
| 47 | 55 | rm -rf "$context_dir" | |
| 56 | + | exit "$status" | |
| 48 | 57 | } | |
| 49 | 58 | trap cleanup EXIT INT TERM | |
| 50 | 59 | ||
| @@ -57,11 +66,14 @@ if ! docker info >/dev/null 2>&1; then | |||
| 57 | 66 | fi | |
| 58 | 67 | ||
| 59 | 68 | printf 'remote picker harness: building %s\n' "$image" | |
| 60 | - | go build -o "$context_dir/vev" "$repo_root" | |
| 61 | - | go build -o "$context_dir/harness" "$repo_root/scripts/remote-picker-harness" | |
| 69 | + | docker pull "$base_image" >/dev/null | |
| 70 | + | image_arch="$(docker image inspect "$base_image" --format '{{.Architecture}}')" | |
| 71 | + | GOOS=linux GOARCH="$image_arch" CGO_ENABLED=0 go build -o "$context_dir/vev" "$repo_root" | |
| 72 | + | GOOS=linux GOARCH="$image_arch" CGO_ENABLED=0 go build -o "$context_dir/harness" "$repo_root/scripts/remote-picker-harness" | |
| 62 | 73 | ssh-keygen -q -t ed25519 -N '' -f "$context_dir/id_ed25519" | |
| 63 | 74 | cp "$repo_root/scripts/remote-picker-harness/Dockerfile" "$context_dir/Dockerfile" | |
| 64 | 75 | cp "$repo_root/scripts/remote-picker-harness/vev-wrapper" "$context_dir/vev-wrapper" | |
| 76 | + | printf '%s\n' 'id_ed25519' > "$context_dir/.dockerignore" | |
| 65 | 77 | chmod 0600 "$context_dir/id_ed25519" | |
| 66 | 78 | ||
| 67 | 79 | docker build --build-arg "BASE_IMAGE=$base_image" -t "$image" "$context_dir" >/dev/null | |
| @@ -40,11 +40,20 @@ remote_container="${network}-remote" | |||||
| 40 | context_dir="$(mktemp -d "${TMPDIR:-/tmp}/vev-remote-picker-harness.XXXXXX")" | 40 | context_dir="$(mktemp -d "${TMPDIR:-/tmp}/vev-remote-picker-harness.XXXXXX")" | ||
| 41 | 41 | ||||
| 42 | cleanup() { | 42 | cleanup() { | ||
| 43 | + | status=$? | |||
| 44 | + | trap - EXIT INT TERM | |||
| 43 | set +e | 45 | set +e | ||
| 46 | + | if [ "$status" -ne 0 ]; then | |||
| 47 | + | printf 'remote picker harness: local container logs:\n' >&2 | |||
| 48 | + | docker logs "$local_container" >&2 | |||
| 49 | + | printf 'remote picker harness: remote container logs:\n' >&2 | |||
| 50 | + | docker logs "$remote_container" >&2 | |||
| 51 | + | fi | |||
| 44 | docker rm -f "$local_container" "$remote_container" >/dev/null 2>&1 | 52 | docker rm -f "$local_container" "$remote_container" >/dev/null 2>&1 | ||
| 45 | docker network rm "$network" >/dev/null 2>&1 | 53 | docker network rm "$network" >/dev/null 2>&1 | ||
| 46 | docker image rm "$image" >/dev/null 2>&1 | 54 | docker image rm "$image" >/dev/null 2>&1 | ||
| 47 | rm -rf "$context_dir" | 55 | rm -rf "$context_dir" | ||
| 56 | + | exit "$status" | |||
| 48 | } | 57 | } | ||
| 49 | trap cleanup EXIT INT TERM | 58 | trap cleanup EXIT INT TERM | ||
| 50 | 59 | ||||
| @@ -57,11 +66,14 @@ if ! docker info >/dev/null 2>&1; then | |||||
| 57 | fi | 66 | fi | ||
| 58 | 67 | ||||
| 59 | printf 'remote picker harness: building %s\n' "$image" | 68 | printf 'remote picker harness: building %s\n' "$image" | ||
| 60 | - | go build -o "$context_dir/vev" "$repo_root" | 69 | + | docker pull "$base_image" >/dev/null |
| 61 | - | go build -o "$context_dir/harness" "$repo_root/scripts/remote-picker-harness" | 70 | + | image_arch="$(docker image inspect "$base_image" --format '{{.Architecture}}')" |
| 71 | + | GOOS=linux GOARCH="$image_arch" CGO_ENABLED=0 go build -o "$context_dir/vev" "$repo_root" | |||
| 72 | + | GOOS=linux GOARCH="$image_arch" CGO_ENABLED=0 go build -o "$context_dir/harness" "$repo_root/scripts/remote-picker-harness" | |||
| 62 | ssh-keygen -q -t ed25519 -N '' -f "$context_dir/id_ed25519" | 73 | ssh-keygen -q -t ed25519 -N '' -f "$context_dir/id_ed25519" | ||
| 63 | cp "$repo_root/scripts/remote-picker-harness/Dockerfile" "$context_dir/Dockerfile" | 74 | cp "$repo_root/scripts/remote-picker-harness/Dockerfile" "$context_dir/Dockerfile" | ||
| 64 | cp "$repo_root/scripts/remote-picker-harness/vev-wrapper" "$context_dir/vev-wrapper" | 75 | cp "$repo_root/scripts/remote-picker-harness/vev-wrapper" "$context_dir/vev-wrapper" | ||
| 76 | + | printf '%s\n' 'id_ed25519' > "$context_dir/.dockerignore" | |||
| 65 | chmod 0600 "$context_dir/id_ed25519" | 77 | chmod 0600 "$context_dir/id_ed25519" | ||
| 66 | 78 | ||||
| 67 | docker build --build-arg "BASE_IMAGE=$base_image" -t "$image" "$context_dir" >/dev/null | 79 | docker build --build-arg "BASE_IMAGE=$base_image" -t "$image" "$context_dir" >/dev/null | ||
M scripts/remote-picker-harness/vev-wrapper +2 -1
| @@ -12,7 +12,8 @@ export COLORTERM=truecolor | |||
| 12 | 12 | case "$*" in | |
| 13 | 13 | *remote-catalog*) | |
| 14 | 14 | if [ -e /tmp/vev-bad-catalog ]; then | |
| 15 | - | printf '%s\n' '{"protocol_version":23,"schema_version":1,"sessions":[]}' | |
| 15 | + | # Keep zero invalid so this fixture cannot become a valid protocol version. | |
| 16 | + | printf '%s\n' '{"protocol_version":0,"schema_version":1,"sessions":[]}' | |
| 16 | 17 | exit 0 | |
| 17 | 18 | fi | |
| 18 | 19 | ;; | |
| @@ -12,7 +12,8 @@ export COLORTERM=truecolor | |||||
| 12 | case "$*" in | 12 | case "$*" in | ||
| 13 | *remote-catalog*) | 13 | *remote-catalog*) | ||
| 14 | if [ -e /tmp/vev-bad-catalog ]; then | 14 | if [ -e /tmp/vev-bad-catalog ]; then | ||
| 15 | - | printf '%s\n' '{"protocol_version":23,"schema_version":1,"sessions":[]}' | 15 | + | # Keep zero invalid so this fixture cannot become a valid protocol version. |
| 16 | + | printf '%s\n' '{"protocol_version":0,"schema_version":1,"sessions":[]}' | |||
| 16 | exit 0 | 17 | exit 0 | ||
| 17 | fi | 18 | fi | ||
| 18 | ;; | 19 | ;; | ||