fix(broker): narrow SSH auth classification, keep dial causes, and document visited routes
- commit
- 7eed0073edbcc29d4cfc106be1502acc34c17836
- author
- brice · · 2026-09-23 20:24 UTC
- parent
- f06fef6
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
13 files · +131 -51
M docs/architecture.md +2 -2
| @@ -1,6 +1,6 @@ | |||
| 1 | 1 | # Architecture | |
| 2 | 2 | ||
| 3 | - | vev uses a hexagonal core with typed session messages at the client and daemon boundary. Binary framing and carriage selection stay outside the use cases. The current strict protocol version is 58. | |
| 3 | + | vev uses a hexagonal core with typed session messages at the client and daemon boundary. Binary framing and carriage selection stay outside the use cases. The current strict protocol version is 59. | |
| 4 | 4 | ||
| 5 | 5 | `CommandResult` has a closed explicit outcome contract: | |
| 6 | 6 | ||
| @@ -151,7 +151,7 @@ Use cases exchange `protocol.ClientMessage` and `protocol.ServerMessage` values. | |||
| 151 | 151 | `go tool buf generate`, and handle the new variant in `sessionwire`. | |
| 152 | 152 | Never edit generated `*.pb.go`; never add manual IDs or dispatch tables. | |
| 153 | 153 | - Implement I/O, queues, workers, environment integration, or technology selection in an adapter or `internal/app`. | |
| 154 | - | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `58`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. | |
| 154 | + | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `59`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. | |
| 155 | 155 | ||
| 156 | 156 | ## Broker wire core (Plan 001 P3.1, not activated) | |
| 157 | 157 | ||
| @@ -1,6 +1,6 @@ | |||||
| 1 | # Architecture | 1 | # Architecture | ||
| 2 | 2 | ||||
| 3 | - | vev uses a hexagonal core with typed session messages at the client and daemon boundary. Binary framing and carriage selection stay outside the use cases. The current strict protocol version is 58. | 3 | + | vev uses a hexagonal core with typed session messages at the client and daemon boundary. Binary framing and carriage selection stay outside the use cases. The current strict protocol version is 59. |
| 4 | 4 | ||||
| 5 | `CommandResult` has a closed explicit outcome contract: | 5 | `CommandResult` has a closed explicit outcome contract: | ||
| 6 | 6 | ||||
| @@ -151,7 +151,7 @@ Use cases exchange `protocol.ClientMessage` and `protocol.ServerMessage` values. | |||||
| 151 | `go tool buf generate`, and handle the new variant in `sessionwire`. | 151 | `go tool buf generate`, and handle the new variant in `sessionwire`. | ||
| 152 | Never edit generated `*.pb.go`; never add manual IDs or dispatch tables. | 152 | Never edit generated `*.pb.go`; never add manual IDs or dispatch tables. | ||
| 153 | - Implement I/O, queues, workers, environment integration, or technology selection in an adapter or `internal/app`. | 153 | - Implement I/O, queues, workers, environment integration, or technology selection in an adapter or `internal/app`. | ||
| 154 | - | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `58`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. | 154 | + | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `59`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. |
| 155 | 155 | ||||
| 156 | ## Broker wire core (Plan 001 P3.1, not activated) | 156 | ## Broker wire core (Plan 001 P3.1, not activated) | ||
| 157 | 157 | ||||
M docs/remote-resilience.md +3 -1
| @@ -42,7 +42,9 @@ exactly. The daemon renders the latest attachment snapshot; active routes are | |||
| 42 | 42 | metadata-only. `JRS` and cross-daemon `BCK` transitions send typed | |
| 43 | 43 | key/generation actions back to the client, while `BCK` to a live session on the | |
| 44 | 44 | current authenticated daemon uses an in-band switch. Route history is | |
| 45 | - | process-local and is not persisted or shared between clients. | |
| 45 | + | process-local and is not persisted or shared between clients. The status-bar | |
| 46 | + | history and `JRS` ranks list only routes this client has attached to, so a new | |
| 47 | + | client starts with an empty history; the palette lists every known route. | |
| 46 | 48 | ||
| 47 | 49 | A picker-selected remote target is a direct handoff with one input pump. Exact | |
| 48 | 50 | lifecycle/name identities are carried separately from display labels, and a | |
| @@ -42,7 +42,9 @@ exactly. The daemon renders the latest attachment snapshot; active routes are | |||||
| 42 | metadata-only. `JRS` and cross-daemon `BCK` transitions send typed | 42 | metadata-only. `JRS` and cross-daemon `BCK` transitions send typed | ||
| 43 | key/generation actions back to the client, while `BCK` to a live session on the | 43 | key/generation actions back to the client, while `BCK` to a live session on the | ||
| 44 | current authenticated daemon uses an in-band switch. Route history is | 44 | current authenticated daemon uses an in-band switch. Route history is | ||
| 45 | - | process-local and is not persisted or shared between clients. | 45 | + | process-local and is not persisted or shared between clients. The status-bar |
| 46 | + | history and `JRS` ranks list only routes this client has attached to, so a new | |||
| 47 | + | client starts with an empty history; the palette lists every known route. | |||
| 46 | 48 | ||||
| 47 | A picker-selected remote target is a direct handoff with one input pump. Exact | 49 | A picker-selected remote target is a direct handoff with one input pump. Exact | ||
| 48 | lifecycle/name identities are carried separately from display labels, and a | 50 | lifecycle/name identities are carried separately from display labels, and a | ||
M internal/adapters/sessionwire/proto_convert_route_hosts_test.go +4 -1
| @@ -17,7 +17,7 @@ func routeHostsTestSnapshot() protocol.RecentRouteSnapshot { | |||
| 17 | 17 | Generation: 3, | |
| 18 | 18 | Entries: []protocol.RecentRouteEntry{{ | |
| 19 | 19 | Key: 1, Generation: 1, Target: target, Name: "work", HostLabel: "box", | |
| 20 | - | Kind: protocol.RouteKindRemote, Attention: true, AttentionSeq: 5, | |
| 20 | + | Kind: protocol.RouteKindRemote, Attention: true, AttentionSeq: 5, Visited: true, | |
| 21 | 21 | }}, | |
| 22 | 22 | Hosts: []protocol.RouteHost{ | |
| 23 | 23 | {Key: 2, Generation: 1, Label: "a", Kind: protocol.RouteKindRemote}, | |
| @@ -35,6 +35,9 @@ func TestProtoRouteHostsWire(t *testing.T) { | |||
| 35 | 35 | seqBytes, err := proto.Marshal(&wire.RecentRouteEntry{AttentionSeq: 5}) | |
| 36 | 36 | require.NoError(t, err) | |
| 37 | 37 | require.Equal(t, []byte{0x50, 0x05}, seqBytes) | |
| 38 | + | visitedBytes, err := proto.Marshal(&wire.RecentRouteEntry{Visited: true}) | |
| 39 | + | require.NoError(t, err) | |
| 40 | + | require.Equal(t, []byte{0x58, 0x01}, visitedBytes) | |
| 38 | 41 | ||
| 39 | 42 | snapshot := routeHostsTestSnapshot() | |
| 40 | 43 | envelope, err := encodeProtoClient(snapshot) | |
| @@ -17,7 +17,7 @@ func routeHostsTestSnapshot() protocol.RecentRouteSnapshot { | |||||
| 17 | Generation: 3, | 17 | Generation: 3, | ||
| 18 | Entries: []protocol.RecentRouteEntry{{ | 18 | Entries: []protocol.RecentRouteEntry{{ | ||
| 19 | Key: 1, Generation: 1, Target: target, Name: "work", HostLabel: "box", | 19 | Key: 1, Generation: 1, Target: target, Name: "work", HostLabel: "box", | ||
| 20 | - | Kind: protocol.RouteKindRemote, Attention: true, AttentionSeq: 5, | 20 | + | Kind: protocol.RouteKindRemote, Attention: true, AttentionSeq: 5, Visited: true, |
| 21 | }}, | 21 | }}, | ||
| 22 | Hosts: []protocol.RouteHost{ | 22 | Hosts: []protocol.RouteHost{ | ||
| 23 | {Key: 2, Generation: 1, Label: "a", Kind: protocol.RouteKindRemote}, | 23 | {Key: 2, Generation: 1, Label: "a", Kind: protocol.RouteKindRemote}, | ||
| @@ -35,6 +35,9 @@ func TestProtoRouteHostsWire(t *testing.T) { | |||||
| 35 | seqBytes, err := proto.Marshal(&wire.RecentRouteEntry{AttentionSeq: 5}) | 35 | seqBytes, err := proto.Marshal(&wire.RecentRouteEntry{AttentionSeq: 5}) | ||
| 36 | require.NoError(t, err) | 36 | require.NoError(t, err) | ||
| 37 | require.Equal(t, []byte{0x50, 0x05}, seqBytes) | 37 | require.Equal(t, []byte{0x50, 0x05}, seqBytes) | ||
| 38 | + | visitedBytes, err := proto.Marshal(&wire.RecentRouteEntry{Visited: true}) | |||
| 39 | + | require.NoError(t, err) | |||
| 40 | + | require.Equal(t, []byte{0x58, 0x01}, visitedBytes) | |||
| 38 | 41 | ||||
| 39 | snapshot := routeHostsTestSnapshot() | 42 | snapshot := routeHostsTestSnapshot() | ||
| 40 | envelope, err := encodeProtoClient(snapshot) | 43 | envelope, err := encodeProtoClient(snapshot) | ||
M internal/adapters/sshstdio/classify.go +1 -1
| @@ -16,7 +16,7 @@ func ClassifyStderr(stderr string) domain.RemoteFailureKind { | |||
| 16 | 16 | case strings.Contains(text, "host key verification failed"), | |
| 17 | 17 | strings.Contains(text, "remote host identification has changed"): | |
| 18 | 18 | return domain.RemoteFailureTrust | |
| 19 | - | case strings.Contains(text, "permission denied"), | |
| 19 | + | case strings.Contains(text, "permission denied ("), | |
| 20 | 20 | strings.Contains(text, "unable to authenticate"), | |
| 21 | 21 | strings.Contains(text, "authentication required"), | |
| 22 | 22 | strings.Contains(text, "too many authentication failures"): | |
| @@ -16,7 +16,7 @@ func ClassifyStderr(stderr string) domain.RemoteFailureKind { | |||||
| 16 | case strings.Contains(text, "host key verification failed"), | 16 | case strings.Contains(text, "host key verification failed"), | ||
| 17 | strings.Contains(text, "remote host identification has changed"): | 17 | strings.Contains(text, "remote host identification has changed"): | ||
| 18 | return domain.RemoteFailureTrust | 18 | return domain.RemoteFailureTrust | ||
| 19 | - | case strings.Contains(text, "permission denied"), | 19 | + | case strings.Contains(text, "permission denied ("), |
| 20 | strings.Contains(text, "unable to authenticate"), | 20 | strings.Contains(text, "unable to authenticate"), | ||
| 21 | strings.Contains(text, "authentication required"), | 21 | strings.Contains(text, "authentication required"), | ||
| 22 | strings.Contains(text, "too many authentication failures"): | 22 | strings.Contains(text, "too many authentication failures"): | ||
M internal/adapters/sshstdio/classify_test.go +1 -0
| @@ -20,6 +20,7 @@ func TestClassifyStderr(t *testing.T) { | |||
| 20 | 20 | {name: "unknown host key", stderr: "Host key verification failed.", want: domain.RemoteFailureTrust}, | |
| 21 | 21 | {name: "changed host key", stderr: "WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED!", want: domain.RemoteFailureTrust}, | |
| 22 | 22 | {name: "timeout", stderr: "ssh: connect to host example.test port 22: Connection timed out", want: domain.RemoteFailureTimeout}, | |
| 23 | + | {name: "remote shell permission stays generic", stderr: "bash: /usr/local/bin/vev: Permission denied", want: domain.RemoteFailureNone}, | |
| 23 | 24 | {name: "refused stays generic", stderr: "connect: connection refused", want: domain.RemoteFailureNone}, | |
| 24 | 25 | {name: "empty", stderr: "", want: domain.RemoteFailureNone}, | |
| 25 | 26 | } | |
| @@ -20,6 +20,7 @@ func TestClassifyStderr(t *testing.T) { | |||||
| 20 | {name: "unknown host key", stderr: "Host key verification failed.", want: domain.RemoteFailureTrust}, | 20 | {name: "unknown host key", stderr: "Host key verification failed.", want: domain.RemoteFailureTrust}, | ||
| 21 | {name: "changed host key", stderr: "WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED!", want: domain.RemoteFailureTrust}, | 21 | {name: "changed host key", stderr: "WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED!", want: domain.RemoteFailureTrust}, | ||
| 22 | {name: "timeout", stderr: "ssh: connect to host example.test port 22: Connection timed out", want: domain.RemoteFailureTimeout}, | 22 | {name: "timeout", stderr: "ssh: connect to host example.test port 22: Connection timed out", want: domain.RemoteFailureTimeout}, | ||
| 23 | + | {name: "remote shell permission stays generic", stderr: "bash: /usr/local/bin/vev: Permission denied", want: domain.RemoteFailureNone}, | |||
| 23 | {name: "refused stays generic", stderr: "connect: connection refused", want: domain.RemoteFailureNone}, | 24 | {name: "refused stays generic", stderr: "connect: connection refused", want: domain.RemoteFailureNone}, | ||
| 24 | {name: "empty", stderr: "", want: domain.RemoteFailureNone}, | 25 | {name: "empty", stderr: "", want: domain.RemoteFailureNone}, | ||
| 25 | } | 26 | } | ||
A internal/app/broker_bootstrap_classify_test.go +48 -0
| @@ -0,0 +1,48 @@ | |||
| 1 | + | package app | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "errors" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/require" | |
| 8 | + | ||
| 9 | + | "github.com/bnema/vev/internal/adapters/sshstdio" | |
| 10 | + | "github.com/bnema/vev/internal/domain" | |
| 11 | + | "github.com/bnema/vev/internal/ports" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | func TestClassifyBootstrapFailureKeepsKindThroughTheDialChain(t *testing.T) { | |
| 15 | + | cause := errors.New("exit status 255") | |
| 16 | + | tests := []struct { | |
| 17 | + | name string | |
| 18 | + | stderr string | |
| 19 | + | nilErr bool | |
| 20 | + | want domain.RemoteFailureKind | |
| 21 | + | }{ | |
| 22 | + | {name: "auth failure is typed", stderr: "user@example.test: Permission denied (publickey).", want: domain.RemoteFailureAuthentication}, | |
| 23 | + | {name: "unclassified stderr stays untyped", stderr: "connect: connection refused", want: domain.RemoteFailureNone}, | |
| 24 | + | {name: "nil sink stays untyped", nilErr: true, want: domain.RemoteFailureNone}, | |
| 25 | + | } | |
| 26 | + | for _, tt := range tests { | |
| 27 | + | t.Run(tt.name, func(t *testing.T) { | |
| 28 | + | var sink *sshstdio.DiagnosticSink | |
| 29 | + | if !tt.nilErr { | |
| 30 | + | sink = sshstdio.NewDiagnosticSink(0) | |
| 31 | + | _, _ = sink.Write([]byte(tt.stderr)) | |
| 32 | + | } | |
| 33 | + | err := classifyBootstrapFailure(sink, cause) | |
| 34 | + | // The pool wraps dial failures as a BrokerError cause. | |
| 35 | + | wrapped := ports.BrokerError{Code: ports.BrokerErrorUnavailable, Cause: brokerQUICUnavailable("read bootstrap readiness", err)} | |
| 36 | + | ||
| 37 | + | require.ErrorIs(t, wrapped, cause, "the original cause stays in the chain") | |
| 38 | + | var typed domain.RemoteFailure | |
| 39 | + | if tt.want == domain.RemoteFailureNone { | |
| 40 | + | require.False(t, errors.As(wrapped, &typed)) | |
| 41 | + | return | |
| 42 | + | } | |
| 43 | + | require.True(t, errors.As(wrapped, &typed)) | |
| 44 | + | require.Equal(t, tt.want, typed.Kind) | |
| 45 | + | require.Contains(t, err.Error(), cause.Error(), "the error text keeps the cause") | |
| 46 | + | }) | |
| 47 | + | } | |
| 48 | + | } | |
| @@ -0,0 +1,48 @@ | |||||
| 1 | + | package app | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "errors" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/require" | |||
| 8 | + | ||||
| 9 | + | "github.com/bnema/vev/internal/adapters/sshstdio" | |||
| 10 | + | "github.com/bnema/vev/internal/domain" | |||
| 11 | + | "github.com/bnema/vev/internal/ports" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | func TestClassifyBootstrapFailureKeepsKindThroughTheDialChain(t *testing.T) { | |||
| 15 | + | cause := errors.New("exit status 255") | |||
| 16 | + | tests := []struct { | |||
| 17 | + | name string | |||
| 18 | + | stderr string | |||
| 19 | + | nilErr bool | |||
| 20 | + | want domain.RemoteFailureKind | |||
| 21 | + | }{ | |||
| 22 | + | {name: "auth failure is typed", stderr: "user@example.test: Permission denied (publickey).", want: domain.RemoteFailureAuthentication}, | |||
| 23 | + | {name: "unclassified stderr stays untyped", stderr: "connect: connection refused", want: domain.RemoteFailureNone}, | |||
| 24 | + | {name: "nil sink stays untyped", nilErr: true, want: domain.RemoteFailureNone}, | |||
| 25 | + | } | |||
| 26 | + | for _, tt := range tests { | |||
| 27 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 28 | + | var sink *sshstdio.DiagnosticSink | |||
| 29 | + | if !tt.nilErr { | |||
| 30 | + | sink = sshstdio.NewDiagnosticSink(0) | |||
| 31 | + | _, _ = sink.Write([]byte(tt.stderr)) | |||
| 32 | + | } | |||
| 33 | + | err := classifyBootstrapFailure(sink, cause) | |||
| 34 | + | // The pool wraps dial failures as a BrokerError cause. | |||
| 35 | + | wrapped := ports.BrokerError{Code: ports.BrokerErrorUnavailable, Cause: brokerQUICUnavailable("read bootstrap readiness", err)} | |||
| 36 | + | ||||
| 37 | + | require.ErrorIs(t, wrapped, cause, "the original cause stays in the chain") | |||
| 38 | + | var typed domain.RemoteFailure | |||
| 39 | + | if tt.want == domain.RemoteFailureNone { | |||
| 40 | + | require.False(t, errors.As(wrapped, &typed)) | |||
| 41 | + | return | |||
| 42 | + | } | |||
| 43 | + | require.True(t, errors.As(wrapped, &typed)) | |||
| 44 | + | require.Equal(t, tt.want, typed.Kind) | |||
| 45 | + | require.Contains(t, err.Error(), cause.Error(), "the error text keeps the cause") | |||
| 46 | + | }) | |||
| 47 | + | } | |||
| 48 | + | } | |||
M internal/app/broker_offline_mux.go +1 -1
| @@ -430,7 +430,7 @@ func classifyBootstrapFailure(sink *sshstdio.DiagnosticSink, err error) error { | |||
| 430 | 430 | return err | |
| 431 | 431 | } | |
| 432 | 432 | if kind := sshstdio.ClassifyStderr(sink.String()); kind != domain.RemoteFailureNone { | |
| 433 | - | return domain.RemoteFailure{Kind: kind, Err: err} | |
| 433 | + | return fmt.Errorf("%w: %w", domain.RemoteFailure{Kind: kind}, err) | |
| 434 | 434 | } | |
| 435 | 435 | return err | |
| 436 | 436 | } | |
| @@ -430,7 +430,7 @@ func classifyBootstrapFailure(sink *sshstdio.DiagnosticSink, err error) error { | |||||
| 430 | return err | 430 | return err | ||
| 431 | } | 431 | } | ||
| 432 | if kind := sshstdio.ClassifyStderr(sink.String()); kind != domain.RemoteFailureNone { | 432 | if kind := sshstdio.ClassifyStderr(sink.String()); kind != domain.RemoteFailureNone { | ||
| 433 | - | return domain.RemoteFailure{Kind: kind, Err: err} | 433 | + | return fmt.Errorf("%w: %w", domain.RemoteFailure{Kind: kind}, err) |
| 434 | } | 434 | } | ||
| 435 | return err | 435 | return err | ||
| 436 | } | 436 | } | ||
M internal/app/spawn.go +6 -9
| @@ -26,10 +26,9 @@ import ( | |||
| 26 | 26 | var ErrDaemonUnreachable = errors.New("vev: daemon did not become reachable") | |
| 27 | 27 | ||
| 28 | 28 | // ErrLifecycleHeldByOther reports that another process kept the daemon | |
| 29 | - | // lifecycle lock for the whole start budget without publishing the carriage: | |
| 30 | - | // typically an older vev daemon that predates the broker. Spawning cannot | |
| 31 | - | // recover this; the user has to stop that process. | |
| 32 | - | var ErrLifecycleHeldByOther = errors.New("vev: another vev daemon holds the lifecycle lock but does not answer; stop the old daemon (for example `pkill -f -- '--daemon'`) and retry") | |
| 29 | + | // lifecycle lock for the whole start budget without publishing the carriage, | |
| 30 | + | // for example a vev daemon from an older build. Spawning cannot recover this. | |
| 31 | + | var ErrLifecycleHeldByOther = errors.New("vev: a vev daemon holds the lifecycle lock but did not answer within the start budget; if it is from an older build, stop it (for example `pkill -f 'vev --daemon'`) and retry") | |
| 33 | 32 | ||
| 34 | 33 | // spawnLockName is the mkdir-based lock directory guarding daemon spawn, so | |
| 35 | 34 | // concurrent first-clients elect a single spawner instead of racing to | |
| @@ -122,7 +121,6 @@ func waitForTargetOrLifecycle[T any](ctx context.Context, lockDir string, dial f | |||
| 122 | 121 | target T | |
| 123 | 122 | owner lifecycleOwnership | |
| 124 | 123 | } | |
| 125 | - | busy := false | |
| 126 | 124 | result, err := retryAttempts(ctx, cfg, func() (targetOrLifecycle, bool, error) { | |
| 127 | 125 | if target, err := dial(ctx); err == nil { | |
| 128 | 126 | return targetOrLifecycle{target: target}, true, nil | |
| @@ -134,12 +132,11 @@ func waitForTargetOrLifecycle[T any](ctx context.Context, lockDir string, dial f | |||
| 134 | 132 | if !errors.Is(err, lifecycle.ErrBusy) { | |
| 135 | 133 | return targetOrLifecycle{}, false, err | |
| 136 | 134 | } | |
| 137 | - | busy = true | |
| 138 | 135 | return targetOrLifecycle{}, false, nil | |
| 139 | 136 | }) | |
| 140 | - | if busy && errors.Is(err, ErrDaemonUnreachable) { | |
| 141 | - | // The lock stayed held and the target never answered for the whole | |
| 142 | - | // budget: a live owner that is not the daemon this binary expects. | |
| 137 | + | if errors.Is(err, ErrDaemonUnreachable) { | |
| 138 | + | // retryAttempts only runs out of budget after busy-lock attempts: the | |
| 139 | + | // lock stayed held and the target never answered for the whole budget. | |
| 143 | 140 | err = fmt.Errorf("%w (%w)", ErrLifecycleHeldByOther, err) | |
| 144 | 141 | } | |
| 145 | 142 | return result.target, result.owner, err | |
| @@ -26,10 +26,9 @@ import ( | |||||
| 26 | var ErrDaemonUnreachable = errors.New("vev: daemon did not become reachable") | 26 | var ErrDaemonUnreachable = errors.New("vev: daemon did not become reachable") | ||
| 27 | 27 | ||||
| 28 | // ErrLifecycleHeldByOther reports that another process kept the daemon | 28 | // ErrLifecycleHeldByOther reports that another process kept the daemon | ||
| 29 | - | // lifecycle lock for the whole start budget without publishing the carriage: | 29 | + | // lifecycle lock for the whole start budget without publishing the carriage, |
| 30 | - | // typically an older vev daemon that predates the broker. Spawning cannot | 30 | + | // for example a vev daemon from an older build. Spawning cannot recover this. |
| 31 | - | // recover this; the user has to stop that process. | 31 | + | var ErrLifecycleHeldByOther = errors.New("vev: a vev daemon holds the lifecycle lock but did not answer within the start budget; if it is from an older build, stop it (for example `pkill -f 'vev --daemon'`) and retry") |
| 32 | - | var ErrLifecycleHeldByOther = errors.New("vev: another vev daemon holds the lifecycle lock but does not answer; stop the old daemon (for example `pkill -f -- '--daemon'`) and retry") | |||
| 33 | 32 | ||||
| 34 | // spawnLockName is the mkdir-based lock directory guarding daemon spawn, so | 33 | // spawnLockName is the mkdir-based lock directory guarding daemon spawn, so | ||
| 35 | // concurrent first-clients elect a single spawner instead of racing to | 34 | // concurrent first-clients elect a single spawner instead of racing to | ||
| @@ -122,7 +121,6 @@ func waitForTargetOrLifecycle[T any](ctx context.Context, lockDir string, dial f | |||||
| 122 | target T | 121 | target T | ||
| 123 | owner lifecycleOwnership | 122 | owner lifecycleOwnership | ||
| 124 | } | 123 | } | ||
| 125 | - | busy := false | |||
| 126 | result, err := retryAttempts(ctx, cfg, func() (targetOrLifecycle, bool, error) { | 124 | result, err := retryAttempts(ctx, cfg, func() (targetOrLifecycle, bool, error) { | ||
| 127 | if target, err := dial(ctx); err == nil { | 125 | if target, err := dial(ctx); err == nil { | ||
| 128 | return targetOrLifecycle{target: target}, true, nil | 126 | return targetOrLifecycle{target: target}, true, nil | ||
| @@ -134,12 +132,11 @@ func waitForTargetOrLifecycle[T any](ctx context.Context, lockDir string, dial f | |||||
| 134 | if !errors.Is(err, lifecycle.ErrBusy) { | 132 | if !errors.Is(err, lifecycle.ErrBusy) { | ||
| 135 | return targetOrLifecycle{}, false, err | 133 | return targetOrLifecycle{}, false, err | ||
| 136 | } | 134 | } | ||
| 137 | - | busy = true | |||
| 138 | return targetOrLifecycle{}, false, nil | 135 | return targetOrLifecycle{}, false, nil | ||
| 139 | }) | 136 | }) | ||
| 140 | - | if busy && errors.Is(err, ErrDaemonUnreachable) { | 137 | + | if errors.Is(err, ErrDaemonUnreachable) { |
| 141 | - | // The lock stayed held and the target never answered for the whole | 138 | + | // retryAttempts only runs out of budget after busy-lock attempts: the |
| 142 | - | // budget: a live owner that is not the daemon this binary expects. | 139 | + | // lock stayed held and the target never answered for the whole budget. |
| 143 | err = fmt.Errorf("%w (%w)", ErrLifecycleHeldByOther, err) | 140 | err = fmt.Errorf("%w (%w)", ErrLifecycleHeldByOther, err) | ||
| 144 | } | 141 | } | ||
| 145 | return result.target, result.owner, err | 142 | return result.target, result.owner, err | ||
M internal/app/spawn_lifecycle_held_test.go +8 -2
| @@ -13,15 +13,17 @@ import ( | |||
| 13 | 13 | ) | |
| 14 | 14 | ||
| 15 | 15 | func TestWaitForTargetOrLifecycleNamesAHeldLock(t *testing.T) { | |
| 16 | - | cfg := backoffConfig{initial: time.Millisecond, max: 2 * time.Millisecond, total: 20 * time.Millisecond} | |
| 16 | + | cfg := backoffConfig{initial: time.Millisecond, max: 2 * time.Millisecond, total: 200 * time.Millisecond} | |
| 17 | 17 | refuse := func(context.Context) (struct{}, error) { return struct{}{}, errors.New("no carriage") } | |
| 18 | 18 | tests := []struct { | |
| 19 | 19 | name string | |
| 20 | 20 | held bool | |
| 21 | + | release bool | |
| 21 | 22 | wantHeld bool | |
| 22 | 23 | }{ | |
| 23 | 24 | {name: "lock held by a silent owner", held: true, wantHeld: true}, | |
| 24 | 25 | {name: "free lock is acquired", held: false, wantHeld: false}, | |
| 26 | + | {name: "lock released during the budget is acquired", held: true, release: true, wantHeld: false}, | |
| 25 | 27 | } | |
| 26 | 28 | for _, tt := range tests { | |
| 27 | 29 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -30,7 +32,11 @@ func TestWaitForTargetOrLifecycleNamesAHeldLock(t *testing.T) { | |||
| 30 | 32 | if tt.held { | |
| 31 | 33 | owner, err := lifecycle.TryAcquire(dir) | |
| 32 | 34 | require.NoError(t, err) | |
| 33 | - | t.Cleanup(func() { _ = owner.Release() }) | |
| 35 | + | if tt.release { | |
| 36 | + | time.AfterFunc(5*time.Millisecond, func() { _ = owner.Release() }) | |
| 37 | + | } else { | |
| 38 | + | t.Cleanup(func() { _ = owner.Release() }) | |
| 39 | + | } | |
| 34 | 40 | } | |
| 35 | 41 | ||
| 36 | 42 | _, owner, err := waitForTargetOrLifecycle(context.Background(), dir, refuse, cfg) | |
| @@ -13,15 +13,17 @@ import ( | |||||
| 13 | ) | 13 | ) | ||
| 14 | 14 | ||||
| 15 | func TestWaitForTargetOrLifecycleNamesAHeldLock(t *testing.T) { | 15 | func TestWaitForTargetOrLifecycleNamesAHeldLock(t *testing.T) { | ||
| 16 | - | cfg := backoffConfig{initial: time.Millisecond, max: 2 * time.Millisecond, total: 20 * time.Millisecond} | 16 | + | cfg := backoffConfig{initial: time.Millisecond, max: 2 * time.Millisecond, total: 200 * time.Millisecond} |
| 17 | refuse := func(context.Context) (struct{}, error) { return struct{}{}, errors.New("no carriage") } | 17 | refuse := func(context.Context) (struct{}, error) { return struct{}{}, errors.New("no carriage") } | ||
| 18 | tests := []struct { | 18 | tests := []struct { | ||
| 19 | name string | 19 | name string | ||
| 20 | held bool | 20 | held bool | ||
| 21 | + | release bool | |||
| 21 | wantHeld bool | 22 | wantHeld bool | ||
| 22 | }{ | 23 | }{ | ||
| 23 | {name: "lock held by a silent owner", held: true, wantHeld: true}, | 24 | {name: "lock held by a silent owner", held: true, wantHeld: true}, | ||
| 24 | {name: "free lock is acquired", held: false, wantHeld: false}, | 25 | {name: "free lock is acquired", held: false, wantHeld: false}, | ||
| 26 | + | {name: "lock released during the budget is acquired", held: true, release: true, wantHeld: false}, | |||
| 25 | } | 27 | } | ||
| 26 | for _, tt := range tests { | 28 | for _, tt := range tests { | ||
| 27 | t.Run(tt.name, func(t *testing.T) { | 29 | t.Run(tt.name, func(t *testing.T) { | ||
| @@ -30,7 +32,11 @@ func TestWaitForTargetOrLifecycleNamesAHeldLock(t *testing.T) { | |||||
| 30 | if tt.held { | 32 | if tt.held { | ||
| 31 | owner, err := lifecycle.TryAcquire(dir) | 33 | owner, err := lifecycle.TryAcquire(dir) | ||
| 32 | require.NoError(t, err) | 34 | require.NoError(t, err) | ||
| 33 | - | t.Cleanup(func() { _ = owner.Release() }) | 35 | + | if tt.release { |
| 36 | + | time.AfterFunc(5*time.Millisecond, func() { _ = owner.Release() }) | |||
| 37 | + | } else { | |||
| 38 | + | t.Cleanup(func() { _ = owner.Release() }) | |||
| 39 | + | } | |||
| 34 | } | 40 | } | ||
| 35 | 41 | ||||
| 36 | _, owner, err := waitForTargetOrLifecycle(context.Background(), dir, refuse, cfg) | 42 | _, owner, err := waitForTargetOrLifecycle(context.Background(), dir, refuse, cfg) | ||
M internal/usecase/broker/registry.go +3 -1
| @@ -89,6 +89,8 @@ type RegistryConfig struct { | |||
| 89 | 89 | // at least one live client subscription. A zero value falls back to | |
| 90 | 90 | // defaultDemandFreshForLocal (about 1s) or defaultDemandFreshForRemote | |
| 91 | 91 | // (about 2s). With no live subscription, no scheduled probe runs. | |
| 92 | + | // DemandFreshForRemote is also the first delay of a watched host's | |
| 93 | + | // failure backoff, which doubles up to max(DemandFreshForRemote, 30s). | |
| 92 | 94 | DemandFreshForLocal time.Duration | |
| 93 | 95 | DemandFreshForRemote time.Duration | |
| 94 | 96 | // ProbeTimeout bounds every local and remote observation attempt on the | |
| @@ -1175,7 +1177,7 @@ func demandRetryDelay(base time.Duration, failures uint) time.Duration { | |||
| 1175 | 1177 | for i := uint(1); i < failures && delay < demandRetryMax; i++ { | |
| 1176 | 1178 | delay *= 2 | |
| 1177 | 1179 | } | |
| 1178 | - | return min(delay, demandRetryMax) | |
| 1180 | + | return min(delay, max(base, demandRetryMax)) | |
| 1179 | 1181 | } | |
| 1180 | 1182 | ||
| 1181 | 1183 | // retryDelay is the capped exponential retry cadence for a failure streak. | |
| @@ -89,6 +89,8 @@ type RegistryConfig struct { | |||||
| 89 | // at least one live client subscription. A zero value falls back to | 89 | // at least one live client subscription. A zero value falls back to | ||
| 90 | // defaultDemandFreshForLocal (about 1s) or defaultDemandFreshForRemote | 90 | // defaultDemandFreshForLocal (about 1s) or defaultDemandFreshForRemote | ||
| 91 | // (about 2s). With no live subscription, no scheduled probe runs. | 91 | // (about 2s). With no live subscription, no scheduled probe runs. | ||
| 92 | + | // DemandFreshForRemote is also the first delay of a watched host's | |||
| 93 | + | // failure backoff, which doubles up to max(DemandFreshForRemote, 30s). | |||
| 92 | DemandFreshForLocal time.Duration | 94 | DemandFreshForLocal time.Duration | ||
| 93 | DemandFreshForRemote time.Duration | 95 | DemandFreshForRemote time.Duration | ||
| 94 | // ProbeTimeout bounds every local and remote observation attempt on the | 96 | // ProbeTimeout bounds every local and remote observation attempt on the | ||
| @@ -1175,7 +1177,7 @@ func demandRetryDelay(base time.Duration, failures uint) time.Duration { | |||||
| 1175 | for i := uint(1); i < failures && delay < demandRetryMax; i++ { | 1177 | for i := uint(1); i < failures && delay < demandRetryMax; i++ { | ||
| 1176 | delay *= 2 | 1178 | delay *= 2 | ||
| 1177 | } | 1179 | } | ||
| 1178 | - | return min(delay, demandRetryMax) | 1180 | + | return min(delay, max(base, demandRetryMax)) |
| 1179 | } | 1181 | } | ||
| 1180 | 1182 | ||||
| 1181 | // retryDelay is the capped exponential retry cadence for a failure streak. | 1183 | // retryDelay is the capped exponential retry cadence for a failure streak. | ||
M internal/usecase/client/picker_controller.go +1 -1
| @@ -495,7 +495,7 @@ func (p *pickerController) RenderNotice(size domain.Size) []byte { | |||
| 495 | 495 | return nil | |
| 496 | 496 | } | |
| 497 | 497 | var buffer bytes.Buffer | |
| 498 | - | if _, err := drawClientToast(&buffer, size, newest.Message, domain.AnchorTopRight); err != nil { | |
| 498 | + | if _, err := drawClientToast(&buffer, size, newest.Message, newest.Anchor); err != nil { | |
| 499 | 499 | return nil | |
| 500 | 500 | } | |
| 501 | 501 | return buffer.Bytes() | |
| @@ -495,7 +495,7 @@ func (p *pickerController) RenderNotice(size domain.Size) []byte { | |||||
| 495 | return nil | 495 | return nil | ||
| 496 | } | 496 | } | ||
| 497 | var buffer bytes.Buffer | 497 | var buffer bytes.Buffer | ||
| 498 | - | if _, err := drawClientToast(&buffer, size, newest.Message, domain.AnchorTopRight); err != nil { | 498 | + | if _, err := drawClientToast(&buffer, size, newest.Message, newest.Anchor); err != nil { |
| 499 | return nil | 499 | return nil | ||
| 500 | } | 500 | } | ||
| 501 | return buffer.Bytes() | 501 | return buffer.Bytes() | ||
M internal/usecase/client/route_ledger.go +4 -0
| @@ -29,6 +29,10 @@ import ( | |||
| 29 | 29 | // - Entries are ordered by this client's attach recency, then live local | |
| 30 | 30 | // sessions by catalogue LastUsedSeq, then live remote sessions in | |
| 31 | 31 | // catalogue order, then stopped sessions. | |
| 32 | + | // - Visited marks entries this client attached to. Attach recency sorts | |
| 33 | + | // first, so visited entries are always a prefix of Entries and Previous | |
| 34 | + | // is the first of them; the daemon's status history and jump-recent | |
| 35 | + | // ranks show only that prefix, while the palette shows every entry. | |
| 32 | 36 | // - The attached lifecycle is metadata only (ActiveEntry), never an entry. | |
| 33 | 37 | // - Attention is any(tab.Attention). AttentionSeq orders onsets as the | |
| 34 | 38 | // client observed them, across daemons, so the serving daemon can jump to | |
| @@ -29,6 +29,10 @@ import ( | |||||
| 29 | // - Entries are ordered by this client's attach recency, then live local | 29 | // - Entries are ordered by this client's attach recency, then live local | ||
| 30 | // sessions by catalogue LastUsedSeq, then live remote sessions in | 30 | // sessions by catalogue LastUsedSeq, then live remote sessions in | ||
| 31 | // catalogue order, then stopped sessions. | 31 | // catalogue order, then stopped sessions. | ||
| 32 | + | // - Visited marks entries this client attached to. Attach recency sorts | |||
| 33 | + | // first, so visited entries are always a prefix of Entries and Previous | |||
| 34 | + | // is the first of them; the daemon's status history and jump-recent | |||
| 35 | + | // ranks show only that prefix, while the palette shows every entry. | |||
| 32 | // - The attached lifecycle is metadata only (ActiveEntry), never an entry. | 36 | // - The attached lifecycle is metadata only (ActiveEntry), never an entry. | ||
| 33 | // - Attention is any(tab.Attention). AttentionSeq orders onsets as the | 37 | // - Attention is any(tab.Attention). AttentionSeq orders onsets as the | ||
| 34 | // client observed them, across daemons, so the serving daemon can jump to | 38 | // client observed them, across daemons, so the serving daemon can jump to | ||
M internal/usecase/daemon/recent_route_snapshot_test.go +49 -32
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "testing" | |
| 5 | 5 | ||
| 6 | 6 | "github.com/bnema/vev/internal/protocol" | |
| 7 | + | "github.com/bnema/vev/internal/usecase/command" | |
| 7 | 8 | "github.com/stretchr/testify/require" | |
| 8 | 9 | ) | |
| 9 | 10 | ||
| @@ -64,40 +65,56 @@ func TestAttachmentRouteSnapshotCopiesPublishedEntries(t *testing.T) { | |||
| 64 | 65 | require.Equal(t, "before", ac.routeSnapshotCopy().Entries[0].Name) | |
| 65 | 66 | } | |
| 66 | 67 | ||
| 67 | - | func TestPaletteRecentRouteSelectionSendsTypedClientAction(t *testing.T) { | |
| 68 | - | d, source, ac, _ := newManualSessionWithPTYs(t, nil) | |
| 69 | - | transport := &closeTrackingTransport{} | |
| 70 | - | ac.replaceTransport(transport) | |
| 71 | - | rc := d.attachCoordinator(source, nil, ac, true) | |
| 72 | - | token := source.captureAttachmentCapability(ac, transport) | |
| 73 | - | token.lease = rc.attachmentLease(ac) | |
| 74 | - | ac.installTestAttachmentCapability(token) | |
| 75 | - | effect, admitted := ac.beginAttachmentEffect(token) | |
| 76 | - | require.True(t, admitted) | |
| 77 | - | defer effect.End() | |
| 78 | - | ||
| 79 | - | exec := paletteExec{ | |
| 80 | - | d: d, sess: source, attachment: source, ac: ac, | |
| 81 | - | routeSnapshot: protocol.RecentRouteSnapshot{ | |
| 82 | - | Generation: 4, | |
| 83 | - | Entries: []protocol.RecentRouteEntry{testRouteEntry(2, 3, "remote", 2, protocol.RouteKindRemote)}, | |
| 84 | - | }, | |
| 85 | - | effect: effect, | |
| 68 | + | func TestPaletteJumpRecentSessionTargetsVisitedRank(t *testing.T) { | |
| 69 | + | unvisited := testRouteEntry(5, 1, "other", 5, protocol.RouteKindLocal) | |
| 70 | + | unvisited.Visited = false | |
| 71 | + | snapshot := protocol.RecentRouteSnapshot{ | |
| 72 | + | Generation: 4, | |
| 73 | + | Entries: []protocol.RecentRouteEntry{unvisited, testRouteEntry(2, 3, "remote", 2, protocol.RouteKindRemote)}, | |
| 86 | 74 | } | |
| 87 | - | require.NoError(t, exec.JumpRecentSession(1)) | |
| 88 | - | ||
| 89 | - | frames := transport.Sends() | |
| 90 | - | var action protocol.RouteNavigationAction | |
| 91 | - | found := false | |
| 92 | - | for _, frame := range frames { | |
| 93 | - | decoded, ok := decodeServerMessage(t, frame).(protocol.RouteNavigationAction) | |
| 94 | - | if !ok { | |
| 95 | - | continue | |
| 96 | - | } | |
| 97 | - | action, found = decoded, true | |
| 75 | + | tests := []struct { | |
| 76 | + | name string | |
| 77 | + | rank int | |
| 78 | + | wantKey uint64 | |
| 79 | + | wantErr error | |
| 80 | + | }{ | |
| 81 | + | {name: "rank 1 skips the unvisited entry", rank: 1, wantKey: 2}, | |
| 82 | + | {name: "rank beyond the visited prefix is refused", rank: 2, wantErr: command.ErrInvalidArguments}, | |
| 83 | + | } | |
| 84 | + | for _, tt := range tests { | |
| 85 | + | t.Run(tt.name, func(t *testing.T) { | |
| 86 | + | d, source, ac, _ := newManualSessionWithPTYs(t, nil) | |
| 87 | + | transport := &closeTrackingTransport{} | |
| 88 | + | ac.replaceTransport(transport) | |
| 89 | + | rc := d.attachCoordinator(source, nil, ac, true) | |
| 90 | + | token := source.captureAttachmentCapability(ac, transport) | |
| 91 | + | token.lease = rc.attachmentLease(ac) | |
| 92 | + | ac.installTestAttachmentCapability(token) | |
| 93 | + | effect, admitted := ac.beginAttachmentEffect(token) | |
| 94 | + | require.True(t, admitted) | |
| 95 | + | defer effect.End() | |
| 96 | + | ||
| 97 | + | exec := paletteExec{d: d, sess: source, attachment: source, ac: ac, routeSnapshot: snapshot, effect: effect} | |
| 98 | + | err := exec.JumpRecentSession(tt.rank) | |
| 99 | + | if tt.wantErr != nil { | |
| 100 | + | require.ErrorIs(t, err, tt.wantErr) | |
| 101 | + | return | |
| 102 | + | } | |
| 103 | + | require.NoError(t, err) | |
| 104 | + | ||
| 105 | + | var action protocol.RouteNavigationAction | |
| 106 | + | found := false | |
| 107 | + | for _, frame := range transport.Sends() { | |
| 108 | + | if decoded, ok := decodeServerMessage(t, frame).(protocol.RouteNavigationAction); ok { | |
| 109 | + | action, found = decoded, true | |
| 110 | + | } | |
| 111 | + | } | |
| 112 | + | require.True(t, found) | |
| 113 | + | require.Equal(t, protocol.RouteNavigationAction{SnapshotGeneration: 4, Key: tt.wantKey, Generation: 3}, action) | |
| 114 | + | hints := recentRouteHints(snapshot, nil) | |
| 115 | + | require.Equal(t, action.Key, hints.Recent[tt.rank-1].Key, "hint and jump share the rank mapping") | |
| 116 | + | }) | |
| 98 | 117 | } | |
| 99 | - | require.True(t, found) | |
| 100 | - | require.Equal(t, protocol.RouteNavigationAction{SnapshotGeneration: 4, Key: 2, Generation: 3}, action) | |
| 101 | 118 | } | |
| 102 | 119 | ||
| 103 | 120 | func TestRecentRouteHintsRetainSnapshotSelectionIdentity(t *testing.T) { | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "testing" | 4 | "testing" | ||
| 5 | 5 | ||||
| 6 | "github.com/bnema/vev/internal/protocol" | 6 | "github.com/bnema/vev/internal/protocol" | ||
| 7 | + | "github.com/bnema/vev/internal/usecase/command" | |||
| 7 | "github.com/stretchr/testify/require" | 8 | "github.com/stretchr/testify/require" | ||
| 8 | ) | 9 | ) | ||
| 9 | 10 | ||||
| @@ -64,40 +65,56 @@ func TestAttachmentRouteSnapshotCopiesPublishedEntries(t *testing.T) { | |||||
| 64 | require.Equal(t, "before", ac.routeSnapshotCopy().Entries[0].Name) | 65 | require.Equal(t, "before", ac.routeSnapshotCopy().Entries[0].Name) | ||
| 65 | } | 66 | } | ||
| 66 | 67 | ||||
| 67 | - | func TestPaletteRecentRouteSelectionSendsTypedClientAction(t *testing.T) { | 68 | + | func TestPaletteJumpRecentSessionTargetsVisitedRank(t *testing.T) { |
| 68 | - | d, source, ac, _ := newManualSessionWithPTYs(t, nil) | 69 | + | unvisited := testRouteEntry(5, 1, "other", 5, protocol.RouteKindLocal) |
| 69 | - | transport := &closeTrackingTransport{} | 70 | + | unvisited.Visited = false |
| 70 | - | ac.replaceTransport(transport) | 71 | + | snapshot := protocol.RecentRouteSnapshot{ |
| 71 | - | rc := d.attachCoordinator(source, nil, ac, true) | 72 | + | Generation: 4, |
| 72 | - | token := source.captureAttachmentCapability(ac, transport) | 73 | + | Entries: []protocol.RecentRouteEntry{unvisited, testRouteEntry(2, 3, "remote", 2, protocol.RouteKindRemote)}, |
| 73 | - | token.lease = rc.attachmentLease(ac) | |||
| 74 | - | ac.installTestAttachmentCapability(token) | |||
| 75 | - | effect, admitted := ac.beginAttachmentEffect(token) | |||
| 76 | - | require.True(t, admitted) | |||
| 77 | - | defer effect.End() | |||
| 78 | - | ||||
| 79 | - | exec := paletteExec{ | |||
| 80 | - | d: d, sess: source, attachment: source, ac: ac, | |||
| 81 | - | routeSnapshot: protocol.RecentRouteSnapshot{ | |||
| 82 | - | Generation: 4, | |||
| 83 | - | Entries: []protocol.RecentRouteEntry{testRouteEntry(2, 3, "remote", 2, protocol.RouteKindRemote)}, | |||
| 84 | - | }, | |||
| 85 | - | effect: effect, | |||
| 86 | } | 74 | } | ||
| 87 | - | require.NoError(t, exec.JumpRecentSession(1)) | 75 | + | tests := []struct { |
| 88 | - | 76 | + | name string | |
| 89 | - | frames := transport.Sends() | 77 | + | rank int |
| 90 | - | var action protocol.RouteNavigationAction | 78 | + | wantKey uint64 |
| 91 | - | found := false | 79 | + | wantErr error |
| 92 | - | for _, frame := range frames { | 80 | + | }{ |
| 93 | - | decoded, ok := decodeServerMessage(t, frame).(protocol.RouteNavigationAction) | 81 | + | {name: "rank 1 skips the unvisited entry", rank: 1, wantKey: 2}, |
| 94 | - | if !ok { | 82 | + | {name: "rank beyond the visited prefix is refused", rank: 2, wantErr: command.ErrInvalidArguments}, |
| 95 | - | continue | 83 | + | } |
| 96 | - | } | 84 | + | for _, tt := range tests { |
| 97 | - | action, found = decoded, true | 85 | + | t.Run(tt.name, func(t *testing.T) { |
| 86 | + | d, source, ac, _ := newManualSessionWithPTYs(t, nil) | |||
| 87 | + | transport := &closeTrackingTransport{} | |||
| 88 | + | ac.replaceTransport(transport) | |||
| 89 | + | rc := d.attachCoordinator(source, nil, ac, true) | |||
| 90 | + | token := source.captureAttachmentCapability(ac, transport) | |||
| 91 | + | token.lease = rc.attachmentLease(ac) | |||
| 92 | + | ac.installTestAttachmentCapability(token) | |||
| 93 | + | effect, admitted := ac.beginAttachmentEffect(token) | |||
| 94 | + | require.True(t, admitted) | |||
| 95 | + | defer effect.End() | |||
| 96 | + | ||||
| 97 | + | exec := paletteExec{d: d, sess: source, attachment: source, ac: ac, routeSnapshot: snapshot, effect: effect} | |||
| 98 | + | err := exec.JumpRecentSession(tt.rank) | |||
| 99 | + | if tt.wantErr != nil { | |||
| 100 | + | require.ErrorIs(t, err, tt.wantErr) | |||
| 101 | + | return | |||
| 102 | + | } | |||
| 103 | + | require.NoError(t, err) | |||
| 104 | + | ||||
| 105 | + | var action protocol.RouteNavigationAction | |||
| 106 | + | found := false | |||
| 107 | + | for _, frame := range transport.Sends() { | |||
| 108 | + | if decoded, ok := decodeServerMessage(t, frame).(protocol.RouteNavigationAction); ok { | |||
| 109 | + | action, found = decoded, true | |||
| 110 | + | } | |||
| 111 | + | } | |||
| 112 | + | require.True(t, found) | |||
| 113 | + | require.Equal(t, protocol.RouteNavigationAction{SnapshotGeneration: 4, Key: tt.wantKey, Generation: 3}, action) | |||
| 114 | + | hints := recentRouteHints(snapshot, nil) | |||
| 115 | + | require.Equal(t, action.Key, hints.Recent[tt.rank-1].Key, "hint and jump share the rank mapping") | |||
| 116 | + | }) | |||
| 98 | } | 117 | } | ||
| 99 | - | require.True(t, found) | |||
| 100 | - | require.Equal(t, protocol.RouteNavigationAction{SnapshotGeneration: 4, Key: 2, Generation: 3}, action) | |||
| 101 | } | 118 | } | ||
| 102 | 119 | ||||
| 103 | func TestRecentRouteHintsRetainSnapshotSelectionIdentity(t *testing.T) { | 120 | func TestRecentRouteHintsRetainSnapshotSelectionIdentity(t *testing.T) { | ||