feat(control): return explicit kill outcomes
- commit
- 69cc15b18414c6a1fb13ee8e8a9eb0ecf964d714
- author
- brice · · 2026-09-20 07:45 UTC
- parent
- 68c0c3b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
30 files · +1657 -247
M docs/architecture.md +1 -1
| @@ -197,7 +197,7 @@ Use cases exchange `protocol.ClientMessage` and `protocol.ServerMessage` values. | |||
| 197 | 197 | `go tool buf generate`, and handle the new variant in `sessionwire`. | |
| 198 | 198 | Never edit generated `*.pb.go`; never add manual IDs or dispatch tables. | |
| 199 | 199 | - Implement I/O, queues, workers, environment integration, or technology selection in an adapter or `internal/app`. | |
| 200 | - | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `56`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. | |
| 200 | + | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `57`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. | |
| 201 | 201 | ||
| 202 | 202 | ## Broker wire core (Plan 001 P3.1, not activated) | |
| 203 | 203 | ||
| @@ -197,7 +197,7 @@ Use cases exchange `protocol.ClientMessage` and `protocol.ServerMessage` values. | |||||
| 197 | `go tool buf generate`, and handle the new variant in `sessionwire`. | 197 | `go tool buf generate`, and handle the new variant in `sessionwire`. | ||
| 198 | Never edit generated `*.pb.go`; never add manual IDs or dispatch tables. | 198 | Never edit generated `*.pb.go`; never add manual IDs or dispatch tables. | ||
| 199 | - Implement I/O, queues, workers, environment integration, or technology selection in an adapter or `internal/app`. | 199 | - Implement I/O, queues, workers, environment integration, or technology selection in an adapter or `internal/app`. | ||
| 200 | - | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `56`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. | 200 | + | - Bump `internal/protocol.Version` for negotiated wire layout changes (currently `57`). The preamble epoch (`wire.ProtocolEpoch`, QUIC ALPN `vev/1`) bumps only for an intentional clean break. |
| 201 | 201 | ||||
| 202 | ## Broker wire core (Plan 001 P3.1, not activated) | 202 | ## Broker wire core (Plan 001 P3.1, not activated) | ||
| 203 | 203 | ||||
M docs/protocol.md +20 -10
| @@ -28,7 +28,7 @@ Server → client: `Welcome`, `ErrorMsg`, `Output`, `Detached`, `Pong`, | |||
| 28 | 28 | `NavigationInventoryResponse`, `NavigationInventoryDemand`, | |
| 29 | 29 | `NavigationInventorySelection`, `PickerOffer`, `PickerSnapshot`, | |
| 30 | 30 | `PickerClosed`, `PickerResult`, `PickerFailure`, `PickerPreview`, | |
| 31 | - | `PickerControlResponse`. | |
| 31 | + | `PickerControlResponse`, `KillResult`. | |
| 32 | 32 | ||
| 33 | 33 | `RouteNavigationFailure` is the one payload that appears in both unions. | |
| 34 | 34 | `sessionwire` conversion switches (`decodeProtoClient`/`decodeProtoServer`, | |
| @@ -78,7 +78,7 @@ mutation: | |||
| 78 | 78 | | `DecodeMessageHello`, version mismatch | `ErrorMsg{ErrVersionMismatch}` | no | | |
| 79 | 79 | | `DecodeMessageHello`, malformed | `ErrorMsg{ErrInternal}` | no | | |
| 80 | 80 | | `DecodeMessageCommand` | `CommandResult{Code, Text}` | yes, else silent close | | |
| 81 | - | | `DecodeMessageKill` | `ErrorMsg{ErrInternal}` | no | | |
| 81 | + | | `DecodeMessageKill` | `ErrorMsg{ErrInternal}` | no — a malformed kill has no reliable RequestID to correlate a `KillResult` to | | |
| 82 | 82 | | `DecodeMessageRemotePreview` | `RemotePreview{Status: Malformed}` | no | | |
| 83 | 83 | | `DecodeMessageNavigationInventory` | `NavigationInventoryResponse{Invalid / VersionMismatch}` | yes, else silent close | | |
| 84 | 84 | | other / unknown | `ErrorMsg{ErrInternal, "expected hello"}` | no | | |
| @@ -188,14 +188,24 @@ while `pickerOpen`; only typed picker messages act. | |||
| 188 | 188 | and carry no async byte-budget charge; async sends are charged on | |
| 189 | 189 | admission and released exactly once on dequeue. | |
| 190 | 190 | ||
| 191 | - | Current state: a one-shot control client uses the daemon closing the | |
| 192 | - | connection (EOF) as its success signal, so a control failure whose error | |
| 193 | - | response cannot be delivered would otherwise be indistinguishable from | |
| 194 | - | success; the daemon only logs such a failed send | |
| 195 | - | (`daemon.logControlSendFailure`) before closing. The wire-level decision for that | |
| 196 | - | residual belongs to the P7 explicit result protocol, which replaces close-as- | |
| 197 | - | success with an explicit per-request result; until that cutover the close-based | |
| 198 | - | convention is kept unchanged (GO-005, deferred to P7). | |
| 191 | + | Current state: the one-shot kill control paths have moved to the explicit | |
| 192 | + | result protocol. `Kill` carries a unique nonzero `RequestID`, and the daemon | |
| 193 | + | answers every normally decoded kill request with exactly one correlated | |
| 194 | + | `KillResult` before the control connection closes: `KillSucceeded`, | |
| 195 | + | `KillFailed` (with a bounded structured `Failures` list), or a definite | |
| 196 | + | `KillOutcomeUnknown`. The client consumes that result; a close without a | |
| 197 | + | result, a mismatched `RequestID`, or a wrong-typed reply is `outcome unknown` | |
| 198 | + | and is never replayed, so a control failure whose response cannot be delivered | |
| 199 | + | is never mistaken for success. The remaining one-shot controls (`List`, | |
| 200 | + | `CommandRequest`, remote preview, inventory, and picker control) have no | |
| 201 | + | explicit per-request result. They do not treat close as success: the daemon | |
| 202 | + | attempts one response and then closes, and a client that receives no valid | |
| 203 | + | response reports a receive error rather than a silent success. Because no | |
| 204 | + | typed result exists, an undeliverable response is logged daemon-side only where | |
| 205 | + | the handler has a send-failure path (`CommandRequest`, remote preview) and is | |
| 206 | + | otherwise unrecorded, so the client is left without any outcome at all; these | |
| 207 | + | paths stay weaker than the explicit `KillResult`, and replacing them with | |
| 208 | + | explicit per-request results belongs to the P7 cutover (GO-005). | |
| 199 | 209 | ||
| 200 | 210 | ## Limits | |
| 201 | 211 | ||
| @@ -28,7 +28,7 @@ Server → client: `Welcome`, `ErrorMsg`, `Output`, `Detached`, `Pong`, | |||||
| 28 | `NavigationInventoryResponse`, `NavigationInventoryDemand`, | 28 | `NavigationInventoryResponse`, `NavigationInventoryDemand`, | ||
| 29 | `NavigationInventorySelection`, `PickerOffer`, `PickerSnapshot`, | 29 | `NavigationInventorySelection`, `PickerOffer`, `PickerSnapshot`, | ||
| 30 | `PickerClosed`, `PickerResult`, `PickerFailure`, `PickerPreview`, | 30 | `PickerClosed`, `PickerResult`, `PickerFailure`, `PickerPreview`, | ||
| 31 | - | `PickerControlResponse`. | 31 | + | `PickerControlResponse`, `KillResult`. |
| 32 | 32 | ||||
| 33 | `RouteNavigationFailure` is the one payload that appears in both unions. | 33 | `RouteNavigationFailure` is the one payload that appears in both unions. | ||
| 34 | `sessionwire` conversion switches (`decodeProtoClient`/`decodeProtoServer`, | 34 | `sessionwire` conversion switches (`decodeProtoClient`/`decodeProtoServer`, | ||
| @@ -78,7 +78,7 @@ mutation: | |||||
| 78 | | `DecodeMessageHello`, version mismatch | `ErrorMsg{ErrVersionMismatch}` | no | | 78 | | `DecodeMessageHello`, version mismatch | `ErrorMsg{ErrVersionMismatch}` | no | | ||
| 79 | | `DecodeMessageHello`, malformed | `ErrorMsg{ErrInternal}` | no | | 79 | | `DecodeMessageHello`, malformed | `ErrorMsg{ErrInternal}` | no | | ||
| 80 | | `DecodeMessageCommand` | `CommandResult{Code, Text}` | yes, else silent close | | 80 | | `DecodeMessageCommand` | `CommandResult{Code, Text}` | yes, else silent close | | ||
| 81 | - | | `DecodeMessageKill` | `ErrorMsg{ErrInternal}` | no | | 81 | + | | `DecodeMessageKill` | `ErrorMsg{ErrInternal}` | no — a malformed kill has no reliable RequestID to correlate a `KillResult` to | |
| 82 | | `DecodeMessageRemotePreview` | `RemotePreview{Status: Malformed}` | no | | 82 | | `DecodeMessageRemotePreview` | `RemotePreview{Status: Malformed}` | no | | ||
| 83 | | `DecodeMessageNavigationInventory` | `NavigationInventoryResponse{Invalid / VersionMismatch}` | yes, else silent close | | 83 | | `DecodeMessageNavigationInventory` | `NavigationInventoryResponse{Invalid / VersionMismatch}` | yes, else silent close | | ||
| 84 | | other / unknown | `ErrorMsg{ErrInternal, "expected hello"}` | no | | 84 | | other / unknown | `ErrorMsg{ErrInternal, "expected hello"}` | no | | ||
| @@ -188,14 +188,24 @@ while `pickerOpen`; only typed picker messages act. | |||||
| 188 | and carry no async byte-budget charge; async sends are charged on | 188 | and carry no async byte-budget charge; async sends are charged on | ||
| 189 | admission and released exactly once on dequeue. | 189 | admission and released exactly once on dequeue. | ||
| 190 | 190 | ||||
| 191 | - | Current state: a one-shot control client uses the daemon closing the | 191 | + | Current state: the one-shot kill control paths have moved to the explicit |
| 192 | - | connection (EOF) as its success signal, so a control failure whose error | 192 | + | result protocol. `Kill` carries a unique nonzero `RequestID`, and the daemon |
| 193 | - | response cannot be delivered would otherwise be indistinguishable from | 193 | + | answers every normally decoded kill request with exactly one correlated |
| 194 | - | success; the daemon only logs such a failed send | 194 | + | `KillResult` before the control connection closes: `KillSucceeded`, |
| 195 | - | (`daemon.logControlSendFailure`) before closing. The wire-level decision for that | 195 | + | `KillFailed` (with a bounded structured `Failures` list), or a definite |
| 196 | - | residual belongs to the P7 explicit result protocol, which replaces close-as- | 196 | + | `KillOutcomeUnknown`. The client consumes that result; a close without a |
| 197 | - | success with an explicit per-request result; until that cutover the close-based | 197 | + | result, a mismatched `RequestID`, or a wrong-typed reply is `outcome unknown` |
| 198 | - | convention is kept unchanged (GO-005, deferred to P7). | 198 | + | and is never replayed, so a control failure whose response cannot be delivered |
| 199 | + | is never mistaken for success. The remaining one-shot controls (`List`, | |||
| 200 | + | `CommandRequest`, remote preview, inventory, and picker control) have no | |||
| 201 | + | explicit per-request result. They do not treat close as success: the daemon | |||
| 202 | + | attempts one response and then closes, and a client that receives no valid | |||
| 203 | + | response reports a receive error rather than a silent success. Because no | |||
| 204 | + | typed result exists, an undeliverable response is logged daemon-side only where | |||
| 205 | + | the handler has a send-failure path (`CommandRequest`, remote preview) and is | |||
| 206 | + | otherwise unrecorded, so the client is left without any outcome at all; these | |||
| 207 | + | paths stay weaker than the explicit `KillResult`, and replacing them with | |||
| 208 | + | explicit per-request results belongs to the P7 cutover (GO-005). | |||
| 199 | 209 | ||||
| 200 | ## Limits | 210 | ## Limits | ||
| 201 | 211 | ||||
M internal/adapters/sessionwire/client_test.go +3 -1
| @@ -25,7 +25,7 @@ func TestClientConnectionEncodesEveryClientMessage(t *testing.T) { | |||
| 25 | 25 | {name: "detach", message: protocol.Detach{}}, | |
| 26 | 26 | {name: "ping", message: protocol.Ping{}}, | |
| 27 | 27 | {name: "list", message: protocol.List{}}, | |
| 28 | - | {name: "kill", message: protocol.Kill{Name: "work"}}, | |
| 28 | + | {name: "kill", message: protocol.Kill{RequestID: 1, Name: "work"}}, | |
| 29 | 29 | {name: "theme", message: protocol.Theme{TrueColor: true}}, | |
| 30 | 30 | {name: "ack", message: protocol.Ack{Epoch: 1, State: 1}}, | |
| 31 | 31 | {name: "image", message: protocol.ImagePush{InputSeq: 1, Mime: "image/png", Data: []byte{1}}}, | |
| @@ -73,6 +73,8 @@ func TestClientConnectionDecodesEveryServerMessage(t *testing.T) { | |||
| 73 | 73 | {name: "pong", message: protocol.Pong{}}, | |
| 74 | 74 | {name: "sessions", message: protocol.Sessions{}}, | |
| 75 | 75 | {name: "command result", message: protocol.CommandResult{RequestID: 1, OK: true}}, | |
| 76 | + | {name: "kill result", message: protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded}}, | |
| 77 | + | {name: "kill result partial failures", message: protocol.KillResult{RequestID: 2, Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "partial", Failures: []protocol.KillFailure{{Class: "stopped", Name: "work", Text: "boom"}}}}, | |
| 76 | 78 | {name: "attach target", message: protocol.AttachTarget{Session: "work", Intent: protocol.IntentAttach}}, | |
| 77 | 79 | {name: "preview", message: protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewUnavailable}}, | |
| 78 | 80 | {name: "identity", message: protocol.CommittedRouteIdentity{Target: exact}}, | |
| @@ -25,7 +25,7 @@ func TestClientConnectionEncodesEveryClientMessage(t *testing.T) { | |||||
| 25 | {name: "detach", message: protocol.Detach{}}, | 25 | {name: "detach", message: protocol.Detach{}}, | ||
| 26 | {name: "ping", message: protocol.Ping{}}, | 26 | {name: "ping", message: protocol.Ping{}}, | ||
| 27 | {name: "list", message: protocol.List{}}, | 27 | {name: "list", message: protocol.List{}}, | ||
| 28 | - | {name: "kill", message: protocol.Kill{Name: "work"}}, | 28 | + | {name: "kill", message: protocol.Kill{RequestID: 1, Name: "work"}}, |
| 29 | {name: "theme", message: protocol.Theme{TrueColor: true}}, | 29 | {name: "theme", message: protocol.Theme{TrueColor: true}}, | ||
| 30 | {name: "ack", message: protocol.Ack{Epoch: 1, State: 1}}, | 30 | {name: "ack", message: protocol.Ack{Epoch: 1, State: 1}}, | ||
| 31 | {name: "image", message: protocol.ImagePush{InputSeq: 1, Mime: "image/png", Data: []byte{1}}}, | 31 | {name: "image", message: protocol.ImagePush{InputSeq: 1, Mime: "image/png", Data: []byte{1}}}, | ||
| @@ -73,6 +73,8 @@ func TestClientConnectionDecodesEveryServerMessage(t *testing.T) { | |||||
| 73 | {name: "pong", message: protocol.Pong{}}, | 73 | {name: "pong", message: protocol.Pong{}}, | ||
| 74 | {name: "sessions", message: protocol.Sessions{}}, | 74 | {name: "sessions", message: protocol.Sessions{}}, | ||
| 75 | {name: "command result", message: protocol.CommandResult{RequestID: 1, OK: true}}, | 75 | {name: "command result", message: protocol.CommandResult{RequestID: 1, OK: true}}, | ||
| 76 | + | {name: "kill result", message: protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded}}, | |||
| 77 | + | {name: "kill result partial failures", message: protocol.KillResult{RequestID: 2, Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "partial", Failures: []protocol.KillFailure{{Class: "stopped", Name: "work", Text: "boom"}}}}, | |||
| 76 | {name: "attach target", message: protocol.AttachTarget{Session: "work", Intent: protocol.IntentAttach}}, | 78 | {name: "attach target", message: protocol.AttachTarget{Session: "work", Intent: protocol.IntentAttach}}, | ||
| 77 | {name: "preview", message: protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewUnavailable}}, | 79 | {name: "preview", message: protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewUnavailable}}, | ||
| 78 | {name: "identity", message: protocol.CommittedRouteIdentity{Target: exact}}, | 80 | {name: "identity", message: protocol.CommittedRouteIdentity{Target: exact}}, | ||
A internal/adapters/sessionwire/kill_result_test.go +250 -0
| @@ -0,0 +1,250 @@ | |||
| 1 | + | package sessionwire | |
| 2 | + | ||
| 3 | + | // Kill/KillResult wire contract: a Kill request carries a unique nonzero | |
| 4 | + | // RequestID, exactly one correlated KillResult answers it, and both directions | |
| 5 | + | // fail closed on malformed, wrong-direction, truncated, trailing, or oversize | |
| 6 | + | // bytes. The result is a bounded control envelope: it never rides the | |
| 7 | + | // Output/bulk ceiling exemption. | |
| 8 | + | ||
| 9 | + | import ( | |
| 10 | + | "testing" | |
| 11 | + | ||
| 12 | + | "google.golang.org/protobuf/proto" | |
| 13 | + | ||
| 14 | + | "github.com/bnema/vev/internal/protocol" | |
| 15 | + | "github.com/bnema/vev/internal/protocol/wire" | |
| 16 | + | "github.com/stretchr/testify/require" | |
| 17 | + | ) | |
| 18 | + | ||
| 19 | + | func testKillResult() protocol.KillResult { | |
| 20 | + | return protocol.KillResult{ | |
| 21 | + | RequestID: 0x1122334455, | |
| 22 | + | Outcome: protocol.KillFailed, | |
| 23 | + | Code: protocol.ErrNoSuchSession, | |
| 24 | + | Text: "no such session: work", | |
| 25 | + | Failures: []protocol.KillFailure{ | |
| 26 | + | {Class: "live", Name: "one", Text: "boom"}, | |
| 27 | + | {Class: "stopped", Name: "two", Text: "gone"}, | |
| 28 | + | }, | |
| 29 | + | } | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | // TestKillResultByteRoundTrip proves one semantic KillResult re-encodes to | |
| 33 | + | // byte-identical bytes after a scan + generated unmarshal + semantic decode. | |
| 34 | + | func TestKillResultByteRoundTrip(t *testing.T) { | |
| 35 | + | message := testKillResult() | |
| 36 | + | raw, err := EncodeServerMessage(message) | |
| 37 | + | require.NoError(t, err) | |
| 38 | + | require.NoError(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, raw)) | |
| 39 | + | ||
| 40 | + | decoded, err := DecodeServerEnvelope(raw) | |
| 41 | + | require.NoError(t, err) | |
| 42 | + | require.Equal(t, message, decoded) | |
| 43 | + | ||
| 44 | + | reencoded, err := EncodeServerMessage(decoded) | |
| 45 | + | require.NoError(t, err) | |
| 46 | + | require.Equal(t, raw, reencoded, "KillResult must re-encode byte-for-byte") | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | // TestKillRequestByteRoundTrip proves one Kill request with a unique nonzero | |
| 50 | + | // RequestID survives the client direction byte-for-byte. | |
| 51 | + | func TestKillRequestByteRoundTrip(t *testing.T) { | |
| 52 | + | message := protocol.Kill{RequestID: 0xDEADBEEF, Name: "work", Scope: protocol.KillSession} | |
| 53 | + | raw, err := EncodeClientMessage(message) | |
| 54 | + | require.NoError(t, err) | |
| 55 | + | require.NoError(t, wire.ScanEnvelope(&wire.ClientEnvelope{}, raw)) | |
| 56 | + | ||
| 57 | + | decoded, err := DecodeClientEnvelope(raw) | |
| 58 | + | require.NoError(t, err) | |
| 59 | + | require.Equal(t, message, decoded) | |
| 60 | + | ||
| 61 | + | reencoded, err := EncodeClientMessage(decoded) | |
| 62 | + | require.NoError(t, err) | |
| 63 | + | require.Equal(t, raw, reencoded, "Kill must re-encode byte-for-byte") | |
| 64 | + | } | |
| 65 | + | ||
| 66 | + | // TestKillResultWrongDirectionFailsClosed proves a server-only KillResult | |
| 67 | + | // payload is refused on the client direction and the server decoder rejects a | |
| 68 | + | // nil envelope, so a result can never be consumed as a request. | |
| 69 | + | func TestKillResultWrongDirectionFailsClosed(t *testing.T) { | |
| 70 | + | raw, err := proto.Marshal(&wire.ServerEnvelope{Payload: &wire.ServerEnvelope_KillResult{KillResult: killResultToWire(testKillResult())}}) | |
| 71 | + | require.NoError(t, err) | |
| 72 | + | ||
| 73 | + | var clientEnvelope wire.ClientEnvelope | |
| 74 | + | err = wire.ScanEnvelope(&clientEnvelope, raw) | |
| 75 | + | require.Error(t, err, "a server KillResult must not scan as a client envelope") | |
| 76 | + | ||
| 77 | + | _, err = decodeProtoClient(&wire.ClientEnvelope{Payload: nil}) | |
| 78 | + | require.ErrorIs(t, err, ErrWrongDirection) | |
| 79 | + | _, err = decodeProtoServer(nil) | |
| 80 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |
| 81 | + | } | |
| 82 | + | ||
| 83 | + | // TestKillAndKillResultRejectMissingOrOutOfRangeIdentity proves the decode | |
| 84 | + | // boundary refuses a zero RequestID, an unknown outcome, an out-of-range code, | |
| 85 | + | // and a nil failure entry instead of aliasing them into valid values. | |
| 86 | + | func TestKillAndKillResultRejectMissingOrOutOfRangeIdentity(t *testing.T) { | |
| 87 | + | tests := []struct { | |
| 88 | + | name string | |
| 89 | + | run func(t *testing.T) error | |
| 90 | + | wantErr error | |
| 91 | + | }{ | |
| 92 | + | { | |
| 93 | + | name: "kill zero request id", | |
| 94 | + | run: func(t *testing.T) error { | |
| 95 | + | _, err := killFromWire(&wire.Kill{Name: "work", Scope: uint32(protocol.KillSession)}) | |
| 96 | + | return err | |
| 97 | + | }, | |
| 98 | + | wantErr: errProtoConvertRange, | |
| 99 | + | }, | |
| 100 | + | { | |
| 101 | + | name: "kill nil", | |
| 102 | + | run: func(t *testing.T) error { _, err := killFromWire(nil); return err }, | |
| 103 | + | wantErr: errProtoConvertRange, | |
| 104 | + | }, | |
| 105 | + | { | |
| 106 | + | name: "kill result nil", | |
| 107 | + | run: func(t *testing.T) error { _, err := killResultFromWire(nil); return err }, | |
| 108 | + | wantErr: errProtoConvertRange, | |
| 109 | + | }, | |
| 110 | + | { | |
| 111 | + | name: "kill result zero request id", | |
| 112 | + | run: func(t *testing.T) error { | |
| 113 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 0, Outcome: uint32(protocol.KillSucceeded)}) | |
| 114 | + | return err | |
| 115 | + | }, | |
| 116 | + | wantErr: errProtoConvertRange, | |
| 117 | + | }, | |
| 118 | + | { | |
| 119 | + | name: "kill result unknown outcome below range", | |
| 120 | + | run: func(t *testing.T) error { | |
| 121 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 1, Outcome: 0}) | |
| 122 | + | return err | |
| 123 | + | }, | |
| 124 | + | wantErr: errProtoConvertRange, | |
| 125 | + | }, | |
| 126 | + | { | |
| 127 | + | name: "kill result unknown outcome above range", | |
| 128 | + | run: func(t *testing.T) error { | |
| 129 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 1, Outcome: uint32(protocol.KillOutcomeUnknown) + 1}) | |
| 130 | + | return err | |
| 131 | + | }, | |
| 132 | + | wantErr: errProtoConvertRange, | |
| 133 | + | }, | |
| 134 | + | { | |
| 135 | + | name: "kill result code overflow", | |
| 136 | + | run: func(t *testing.T) error { | |
| 137 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 1, Outcome: uint32(protocol.KillFailed), Code: 1 << 16}) | |
| 138 | + | return err | |
| 139 | + | }, | |
| 140 | + | wantErr: errProtoConvertRange, | |
| 141 | + | }, | |
| 142 | + | { | |
| 143 | + | name: "kill result nil failure entry", | |
| 144 | + | run: func(t *testing.T) error { | |
| 145 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 1, Outcome: uint32(protocol.KillFailed), Failures: []*wire.KillFailure{nil}}) | |
| 146 | + | return err | |
| 147 | + | }, | |
| 148 | + | wantErr: errProtoConvertRange, | |
| 149 | + | }, | |
| 150 | + | } | |
| 151 | + | for _, tt := range tests { | |
| 152 | + | t.Run(tt.name, func(t *testing.T) { | |
| 153 | + | require.ErrorIs(t, tt.run(t), tt.wantErr) | |
| 154 | + | }) | |
| 155 | + | } | |
| 156 | + | } | |
| 157 | + | ||
| 158 | + | // TestKillResultConversionArmInventory is the sessionwire arm inventory for the | |
| 159 | + | // explicit kill result: every arm of the kill encoders is exercised (server | |
| 160 | + | // result value/pointer, client request value/pointer), each arm shares the | |
| 161 | + | // value arm's bytes, and a typed nil pointer arm fails closed with | |
| 162 | + | // ErrInvalidMessage instead of panicking. A new arm added without coverage fails | |
| 163 | + | // here. | |
| 164 | + | func TestKillResultConversionArmInventory(t *testing.T) { | |
| 165 | + | result := testKillResult() | |
| 166 | + | resultRaw, err := EncodeServerMessage(result) | |
| 167 | + | require.NoError(t, err) | |
| 168 | + | request := protocol.Kill{RequestID: 0xDEADBEEF, Name: "work", Scope: protocol.KillSession} | |
| 169 | + | requestRaw, err := EncodeClientMessage(request) | |
| 170 | + | require.NoError(t, err) | |
| 171 | + | ||
| 172 | + | tests := []struct { | |
| 173 | + | name string | |
| 174 | + | // run encodes one arm and returns the bytes and any error. | |
| 175 | + | run func() ([]byte, error) | |
| 176 | + | wantRaw []byte | |
| 177 | + | // wantErr, when non-nil, is the required failure; otherwise run must | |
| 178 | + | // succeed with wantRaw. | |
| 179 | + | wantErr error | |
| 180 | + | }{ | |
| 181 | + | {name: "result value arm", run: func() ([]byte, error) { return EncodeServerMessage(result) }, wantRaw: resultRaw}, | |
| 182 | + | {name: "result pointer arm", run: func() ([]byte, error) { return EncodeServerMessage(&result) }, wantRaw: resultRaw}, | |
| 183 | + | {name: "result typed nil pointer arm", run: func() ([]byte, error) { return EncodeServerMessage((*protocol.KillResult)(nil)) }, wantErr: ErrInvalidMessage}, | |
| 184 | + | {name: "request value arm", run: func() ([]byte, error) { return EncodeClientMessage(request) }, wantRaw: requestRaw}, | |
| 185 | + | {name: "request pointer arm", run: func() ([]byte, error) { return EncodeClientMessage(&request) }, wantRaw: requestRaw}, | |
| 186 | + | {name: "request typed nil pointer arm", run: func() ([]byte, error) { return EncodeClientMessage((*protocol.Kill)(nil)) }, wantErr: ErrInvalidMessage}, | |
| 187 | + | } | |
| 188 | + | for _, tt := range tests { | |
| 189 | + | t.Run(tt.name, func(t *testing.T) { | |
| 190 | + | raw, err := tt.run() | |
| 191 | + | if tt.wantErr != nil { | |
| 192 | + | require.ErrorIs(t, err, tt.wantErr) | |
| 193 | + | return | |
| 194 | + | } | |
| 195 | + | require.NoError(t, err) | |
| 196 | + | require.Equal(t, tt.wantRaw, raw, "every arm must share the value arm's bytes") | |
| 197 | + | }) | |
| 198 | + | } | |
| 199 | + | ||
| 200 | + | // The pointer arm decodes back to the same semantic value, so it is not a | |
| 201 | + | // side channel around validation. | |
| 202 | + | decoded, err := DecodeServerEnvelope(resultRaw) | |
| 203 | + | require.NoError(t, err) | |
| 204 | + | require.Equal(t, result, decoded) | |
| 205 | + | } | |
| 206 | + | ||
| 207 | + | // TestKillResultEnvelopeStrictScan proves the scan boundary rejects a | |
| 208 | + | // truncated payload, trailing bytes after a complete variant, and a | |
| 209 | + | // concatenated duplicate before any semantic decode. | |
| 210 | + | func TestKillResultEnvelopeStrictScan(t *testing.T) { | |
| 211 | + | valid, err := EncodeServerMessage(testKillResult()) | |
| 212 | + | require.NoError(t, err) | |
| 213 | + | require.NoError(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, valid)) | |
| 214 | + | ||
| 215 | + | require.Error(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, valid[:len(valid)-1])) | |
| 216 | + | require.Error(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, append(append([]byte(nil), valid...), 0xFF))) | |
| 217 | + | require.Error(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, append(append([]byte(nil), valid...), valid...))) | |
| 218 | + | ||
| 219 | + | _, err = DecodeServerEnvelope(valid[:len(valid)-1]) | |
| 220 | + | require.Error(t, err) | |
| 221 | + | var failure *protocol.DecodeFailure | |
| 222 | + | require.ErrorAs(t, err, &failure) | |
| 223 | + | require.Equal(t, protocol.DecodeMalformed, failure.Category) | |
| 224 | + | } | |
| 225 | + | ||
| 226 | + | // TestKillResultIsBoundedControlEnvelope proves a KillResult rides the strict | |
| 227 | + | // control ceiling, never the bulk exemption, on both send and receive. | |
| 228 | + | func TestKillResultIsBoundedControlEnvelope(t *testing.T) { | |
| 229 | + | message := testKillResult() | |
| 230 | + | ||
| 231 | + | raw, err := EncodeServerMessage(message) | |
| 232 | + | require.NoError(t, err) | |
| 233 | + | require.Equal(t, categoryControl, serverEnvelopeCategory(raw), "a KillResult is a control envelope") | |
| 234 | + | ||
| 235 | + | // A text sized so the serialized control envelope crosses the 1 MiB | |
| 236 | + | // ceiling while the semantic value itself is only the declared cap. | |
| 237 | + | oversize, err := EncodeServerMessage(protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded, Text: string(make([]byte, wire.ControlEnvelopeLimit))}) | |
| 238 | + | require.NoError(t, err) | |
| 239 | + | require.ErrorIs(t, checkCategoryCeiling(oversize, categoryControl, wire.AbsoluteEnvelopeLimit), wire.ErrScanLength, | |
| 240 | + | "a KillResult above the control ceiling must fail even under an absolute negotiated ceiling") | |
| 241 | + | ||
| 242 | + | send := &serverConnection{raw: &scriptedTransport{}, ceilings: defaultProtoCeilings()} | |
| 243 | + | require.NoError(t, send.SendServer(message)) | |
| 244 | + | require.ErrorIs(t, send.SendServer(protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded, Text: string(make([]byte, wire.ControlEnvelopeLimit))}), wire.ErrScanLength) | |
| 245 | + | ||
| 246 | + | recv := &clientConnection{raw: &scriptedTransport{recv: []wire.Envelope{{Payload: oversize}}}, ceilings: defaultProtoCeilings()} | |
| 247 | + | recv.preambleOnce.Do(func() {}) | |
| 248 | + | _, err = recv.ReceiveServer() | |
| 249 | + | require.Error(t, err) | |
| 250 | + | } | |
| @@ -0,0 +1,250 @@ | |||||
| 1 | + | package sessionwire | |||
| 2 | + | ||||
| 3 | + | // Kill/KillResult wire contract: a Kill request carries a unique nonzero | |||
| 4 | + | // RequestID, exactly one correlated KillResult answers it, and both directions | |||
| 5 | + | // fail closed on malformed, wrong-direction, truncated, trailing, or oversize | |||
| 6 | + | // bytes. The result is a bounded control envelope: it never rides the | |||
| 7 | + | // Output/bulk ceiling exemption. | |||
| 8 | + | ||||
| 9 | + | import ( | |||
| 10 | + | "testing" | |||
| 11 | + | ||||
| 12 | + | "google.golang.org/protobuf/proto" | |||
| 13 | + | ||||
| 14 | + | "github.com/bnema/vev/internal/protocol" | |||
| 15 | + | "github.com/bnema/vev/internal/protocol/wire" | |||
| 16 | + | "github.com/stretchr/testify/require" | |||
| 17 | + | ) | |||
| 18 | + | ||||
| 19 | + | func testKillResult() protocol.KillResult { | |||
| 20 | + | return protocol.KillResult{ | |||
| 21 | + | RequestID: 0x1122334455, | |||
| 22 | + | Outcome: protocol.KillFailed, | |||
| 23 | + | Code: protocol.ErrNoSuchSession, | |||
| 24 | + | Text: "no such session: work", | |||
| 25 | + | Failures: []protocol.KillFailure{ | |||
| 26 | + | {Class: "live", Name: "one", Text: "boom"}, | |||
| 27 | + | {Class: "stopped", Name: "two", Text: "gone"}, | |||
| 28 | + | }, | |||
| 29 | + | } | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | // TestKillResultByteRoundTrip proves one semantic KillResult re-encodes to | |||
| 33 | + | // byte-identical bytes after a scan + generated unmarshal + semantic decode. | |||
| 34 | + | func TestKillResultByteRoundTrip(t *testing.T) { | |||
| 35 | + | message := testKillResult() | |||
| 36 | + | raw, err := EncodeServerMessage(message) | |||
| 37 | + | require.NoError(t, err) | |||
| 38 | + | require.NoError(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, raw)) | |||
| 39 | + | ||||
| 40 | + | decoded, err := DecodeServerEnvelope(raw) | |||
| 41 | + | require.NoError(t, err) | |||
| 42 | + | require.Equal(t, message, decoded) | |||
| 43 | + | ||||
| 44 | + | reencoded, err := EncodeServerMessage(decoded) | |||
| 45 | + | require.NoError(t, err) | |||
| 46 | + | require.Equal(t, raw, reencoded, "KillResult must re-encode byte-for-byte") | |||
| 47 | + | } | |||
| 48 | + | ||||
| 49 | + | // TestKillRequestByteRoundTrip proves one Kill request with a unique nonzero | |||
| 50 | + | // RequestID survives the client direction byte-for-byte. | |||
| 51 | + | func TestKillRequestByteRoundTrip(t *testing.T) { | |||
| 52 | + | message := protocol.Kill{RequestID: 0xDEADBEEF, Name: "work", Scope: protocol.KillSession} | |||
| 53 | + | raw, err := EncodeClientMessage(message) | |||
| 54 | + | require.NoError(t, err) | |||
| 55 | + | require.NoError(t, wire.ScanEnvelope(&wire.ClientEnvelope{}, raw)) | |||
| 56 | + | ||||
| 57 | + | decoded, err := DecodeClientEnvelope(raw) | |||
| 58 | + | require.NoError(t, err) | |||
| 59 | + | require.Equal(t, message, decoded) | |||
| 60 | + | ||||
| 61 | + | reencoded, err := EncodeClientMessage(decoded) | |||
| 62 | + | require.NoError(t, err) | |||
| 63 | + | require.Equal(t, raw, reencoded, "Kill must re-encode byte-for-byte") | |||
| 64 | + | } | |||
| 65 | + | ||||
| 66 | + | // TestKillResultWrongDirectionFailsClosed proves a server-only KillResult | |||
| 67 | + | // payload is refused on the client direction and the server decoder rejects a | |||
| 68 | + | // nil envelope, so a result can never be consumed as a request. | |||
| 69 | + | func TestKillResultWrongDirectionFailsClosed(t *testing.T) { | |||
| 70 | + | raw, err := proto.Marshal(&wire.ServerEnvelope{Payload: &wire.ServerEnvelope_KillResult{KillResult: killResultToWire(testKillResult())}}) | |||
| 71 | + | require.NoError(t, err) | |||
| 72 | + | ||||
| 73 | + | var clientEnvelope wire.ClientEnvelope | |||
| 74 | + | err = wire.ScanEnvelope(&clientEnvelope, raw) | |||
| 75 | + | require.Error(t, err, "a server KillResult must not scan as a client envelope") | |||
| 76 | + | ||||
| 77 | + | _, err = decodeProtoClient(&wire.ClientEnvelope{Payload: nil}) | |||
| 78 | + | require.ErrorIs(t, err, ErrWrongDirection) | |||
| 79 | + | _, err = decodeProtoServer(nil) | |||
| 80 | + | require.ErrorIs(t, err, ErrInvalidMessage) | |||
| 81 | + | } | |||
| 82 | + | ||||
| 83 | + | // TestKillAndKillResultRejectMissingOrOutOfRangeIdentity proves the decode | |||
| 84 | + | // boundary refuses a zero RequestID, an unknown outcome, an out-of-range code, | |||
| 85 | + | // and a nil failure entry instead of aliasing them into valid values. | |||
| 86 | + | func TestKillAndKillResultRejectMissingOrOutOfRangeIdentity(t *testing.T) { | |||
| 87 | + | tests := []struct { | |||
| 88 | + | name string | |||
| 89 | + | run func(t *testing.T) error | |||
| 90 | + | wantErr error | |||
| 91 | + | }{ | |||
| 92 | + | { | |||
| 93 | + | name: "kill zero request id", | |||
| 94 | + | run: func(t *testing.T) error { | |||
| 95 | + | _, err := killFromWire(&wire.Kill{Name: "work", Scope: uint32(protocol.KillSession)}) | |||
| 96 | + | return err | |||
| 97 | + | }, | |||
| 98 | + | wantErr: errProtoConvertRange, | |||
| 99 | + | }, | |||
| 100 | + | { | |||
| 101 | + | name: "kill nil", | |||
| 102 | + | run: func(t *testing.T) error { _, err := killFromWire(nil); return err }, | |||
| 103 | + | wantErr: errProtoConvertRange, | |||
| 104 | + | }, | |||
| 105 | + | { | |||
| 106 | + | name: "kill result nil", | |||
| 107 | + | run: func(t *testing.T) error { _, err := killResultFromWire(nil); return err }, | |||
| 108 | + | wantErr: errProtoConvertRange, | |||
| 109 | + | }, | |||
| 110 | + | { | |||
| 111 | + | name: "kill result zero request id", | |||
| 112 | + | run: func(t *testing.T) error { | |||
| 113 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 0, Outcome: uint32(protocol.KillSucceeded)}) | |||
| 114 | + | return err | |||
| 115 | + | }, | |||
| 116 | + | wantErr: errProtoConvertRange, | |||
| 117 | + | }, | |||
| 118 | + | { | |||
| 119 | + | name: "kill result unknown outcome below range", | |||
| 120 | + | run: func(t *testing.T) error { | |||
| 121 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 1, Outcome: 0}) | |||
| 122 | + | return err | |||
| 123 | + | }, | |||
| 124 | + | wantErr: errProtoConvertRange, | |||
| 125 | + | }, | |||
| 126 | + | { | |||
| 127 | + | name: "kill result unknown outcome above range", | |||
| 128 | + | run: func(t *testing.T) error { | |||
| 129 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 1, Outcome: uint32(protocol.KillOutcomeUnknown) + 1}) | |||
| 130 | + | return err | |||
| 131 | + | }, | |||
| 132 | + | wantErr: errProtoConvertRange, | |||
| 133 | + | }, | |||
| 134 | + | { | |||
| 135 | + | name: "kill result code overflow", | |||
| 136 | + | run: func(t *testing.T) error { | |||
| 137 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 1, Outcome: uint32(protocol.KillFailed), Code: 1 << 16}) | |||
| 138 | + | return err | |||
| 139 | + | }, | |||
| 140 | + | wantErr: errProtoConvertRange, | |||
| 141 | + | }, | |||
| 142 | + | { | |||
| 143 | + | name: "kill result nil failure entry", | |||
| 144 | + | run: func(t *testing.T) error { | |||
| 145 | + | _, err := killResultFromWire(&wire.KillResult{RequestId: 1, Outcome: uint32(protocol.KillFailed), Failures: []*wire.KillFailure{nil}}) | |||
| 146 | + | return err | |||
| 147 | + | }, | |||
| 148 | + | wantErr: errProtoConvertRange, | |||
| 149 | + | }, | |||
| 150 | + | } | |||
| 151 | + | for _, tt := range tests { | |||
| 152 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 153 | + | require.ErrorIs(t, tt.run(t), tt.wantErr) | |||
| 154 | + | }) | |||
| 155 | + | } | |||
| 156 | + | } | |||
| 157 | + | ||||
| 158 | + | // TestKillResultConversionArmInventory is the sessionwire arm inventory for the | |||
| 159 | + | // explicit kill result: every arm of the kill encoders is exercised (server | |||
| 160 | + | // result value/pointer, client request value/pointer), each arm shares the | |||
| 161 | + | // value arm's bytes, and a typed nil pointer arm fails closed with | |||
| 162 | + | // ErrInvalidMessage instead of panicking. A new arm added without coverage fails | |||
| 163 | + | // here. | |||
| 164 | + | func TestKillResultConversionArmInventory(t *testing.T) { | |||
| 165 | + | result := testKillResult() | |||
| 166 | + | resultRaw, err := EncodeServerMessage(result) | |||
| 167 | + | require.NoError(t, err) | |||
| 168 | + | request := protocol.Kill{RequestID: 0xDEADBEEF, Name: "work", Scope: protocol.KillSession} | |||
| 169 | + | requestRaw, err := EncodeClientMessage(request) | |||
| 170 | + | require.NoError(t, err) | |||
| 171 | + | ||||
| 172 | + | tests := []struct { | |||
| 173 | + | name string | |||
| 174 | + | // run encodes one arm and returns the bytes and any error. | |||
| 175 | + | run func() ([]byte, error) | |||
| 176 | + | wantRaw []byte | |||
| 177 | + | // wantErr, when non-nil, is the required failure; otherwise run must | |||
| 178 | + | // succeed with wantRaw. | |||
| 179 | + | wantErr error | |||
| 180 | + | }{ | |||
| 181 | + | {name: "result value arm", run: func() ([]byte, error) { return EncodeServerMessage(result) }, wantRaw: resultRaw}, | |||
| 182 | + | {name: "result pointer arm", run: func() ([]byte, error) { return EncodeServerMessage(&result) }, wantRaw: resultRaw}, | |||
| 183 | + | {name: "result typed nil pointer arm", run: func() ([]byte, error) { return EncodeServerMessage((*protocol.KillResult)(nil)) }, wantErr: ErrInvalidMessage}, | |||
| 184 | + | {name: "request value arm", run: func() ([]byte, error) { return EncodeClientMessage(request) }, wantRaw: requestRaw}, | |||
| 185 | + | {name: "request pointer arm", run: func() ([]byte, error) { return EncodeClientMessage(&request) }, wantRaw: requestRaw}, | |||
| 186 | + | {name: "request typed nil pointer arm", run: func() ([]byte, error) { return EncodeClientMessage((*protocol.Kill)(nil)) }, wantErr: ErrInvalidMessage}, | |||
| 187 | + | } | |||
| 188 | + | for _, tt := range tests { | |||
| 189 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 190 | + | raw, err := tt.run() | |||
| 191 | + | if tt.wantErr != nil { | |||
| 192 | + | require.ErrorIs(t, err, tt.wantErr) | |||
| 193 | + | return | |||
| 194 | + | } | |||
| 195 | + | require.NoError(t, err) | |||
| 196 | + | require.Equal(t, tt.wantRaw, raw, "every arm must share the value arm's bytes") | |||
| 197 | + | }) | |||
| 198 | + | } | |||
| 199 | + | ||||
| 200 | + | // The pointer arm decodes back to the same semantic value, so it is not a | |||
| 201 | + | // side channel around validation. | |||
| 202 | + | decoded, err := DecodeServerEnvelope(resultRaw) | |||
| 203 | + | require.NoError(t, err) | |||
| 204 | + | require.Equal(t, result, decoded) | |||
| 205 | + | } | |||
| 206 | + | ||||
| 207 | + | // TestKillResultEnvelopeStrictScan proves the scan boundary rejects a | |||
| 208 | + | // truncated payload, trailing bytes after a complete variant, and a | |||
| 209 | + | // concatenated duplicate before any semantic decode. | |||
| 210 | + | func TestKillResultEnvelopeStrictScan(t *testing.T) { | |||
| 211 | + | valid, err := EncodeServerMessage(testKillResult()) | |||
| 212 | + | require.NoError(t, err) | |||
| 213 | + | require.NoError(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, valid)) | |||
| 214 | + | ||||
| 215 | + | require.Error(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, valid[:len(valid)-1])) | |||
| 216 | + | require.Error(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, append(append([]byte(nil), valid...), 0xFF))) | |||
| 217 | + | require.Error(t, wire.ScanEnvelope(&wire.ServerEnvelope{}, append(append([]byte(nil), valid...), valid...))) | |||
| 218 | + | ||||
| 219 | + | _, err = DecodeServerEnvelope(valid[:len(valid)-1]) | |||
| 220 | + | require.Error(t, err) | |||
| 221 | + | var failure *protocol.DecodeFailure | |||
| 222 | + | require.ErrorAs(t, err, &failure) | |||
| 223 | + | require.Equal(t, protocol.DecodeMalformed, failure.Category) | |||
| 224 | + | } | |||
| 225 | + | ||||
| 226 | + | // TestKillResultIsBoundedControlEnvelope proves a KillResult rides the strict | |||
| 227 | + | // control ceiling, never the bulk exemption, on both send and receive. | |||
| 228 | + | func TestKillResultIsBoundedControlEnvelope(t *testing.T) { | |||
| 229 | + | message := testKillResult() | |||
| 230 | + | ||||
| 231 | + | raw, err := EncodeServerMessage(message) | |||
| 232 | + | require.NoError(t, err) | |||
| 233 | + | require.Equal(t, categoryControl, serverEnvelopeCategory(raw), "a KillResult is a control envelope") | |||
| 234 | + | ||||
| 235 | + | // A text sized so the serialized control envelope crosses the 1 MiB | |||
| 236 | + | // ceiling while the semantic value itself is only the declared cap. | |||
| 237 | + | oversize, err := EncodeServerMessage(protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded, Text: string(make([]byte, wire.ControlEnvelopeLimit))}) | |||
| 238 | + | require.NoError(t, err) | |||
| 239 | + | require.ErrorIs(t, checkCategoryCeiling(oversize, categoryControl, wire.AbsoluteEnvelopeLimit), wire.ErrScanLength, | |||
| 240 | + | "a KillResult above the control ceiling must fail even under an absolute negotiated ceiling") | |||
| 241 | + | ||||
| 242 | + | send := &serverConnection{raw: &scriptedTransport{}, ceilings: defaultProtoCeilings()} | |||
| 243 | + | require.NoError(t, send.SendServer(message)) | |||
| 244 | + | require.ErrorIs(t, send.SendServer(protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded, Text: string(make([]byte, wire.ControlEnvelopeLimit))}), wire.ErrScanLength) | |||
| 245 | + | ||||
| 246 | + | recv := &clientConnection{raw: &scriptedTransport{recv: []wire.Envelope{{Payload: oversize}}}, ceilings: defaultProtoCeilings()} | |||
| 247 | + | recv.preambleOnce.Do(func() {}) | |||
| 248 | + | _, err = recv.ReceiveServer() | |||
| 249 | + | require.Error(t, err) | |||
| 250 | + | } | |||
M internal/adapters/sessionwire/proto_convert_misc.go +31 -2
| @@ -187,7 +187,7 @@ func detachedFromWire(message *wire.Detached) (protocol.Detached, error) { | |||
| 187 | 187 | } | |
| 188 | 188 | ||
| 189 | 189 | func killToWire(message protocol.Kill) *wire.Kill { | |
| 190 | - | return &wire.Kill{Name: message.Name, Scope: uint32(message.Scope)} | |
| 190 | + | return &wire.Kill{Name: message.Name, Scope: uint32(message.Scope), RequestId: message.RequestID} | |
| 191 | 191 | } | |
| 192 | 192 | ||
| 193 | 193 | func killFromWire(message *wire.Kill) (protocol.Kill, error) { | |
| @@ -201,7 +201,36 @@ func killFromWire(message *wire.Kill) (protocol.Kill, error) { | |||
| 201 | 201 | if scope != protocol.KillSession && scope != protocol.KillDaemon && scope != protocol.KillAll { | |
| 202 | 202 | return protocol.Kill{}, errProtoConvertRange | |
| 203 | 203 | } | |
| 204 | - | return protocol.Kill{Name: message.GetName(), Scope: scope}, nil | |
| 204 | + | if message.GetRequestId() == 0 { | |
| 205 | + | return protocol.Kill{}, errProtoConvertRange | |
| 206 | + | } | |
| 207 | + | return protocol.Kill{RequestID: message.GetRequestId(), Name: message.GetName(), Scope: scope}, nil | |
| 208 | + | } | |
| 209 | + | ||
| 210 | + | func killResultToWire(message protocol.KillResult) *wire.KillResult { | |
| 211 | + | out := &wire.KillResult{RequestId: message.RequestID, Outcome: uint32(message.Outcome), Code: uint32(message.Code), Text: message.Text} | |
| 212 | + | for _, failure := range message.Failures { | |
| 213 | + | out.Failures = append(out.Failures, &wire.KillFailure{Class: failure.Class, Name: failure.Name, Text: failure.Text}) | |
| 214 | + | } | |
| 215 | + | return out | |
| 216 | + | } | |
| 217 | + | ||
| 218 | + | func killResultFromWire(message *wire.KillResult) (protocol.KillResult, error) { | |
| 219 | + | if message == nil || message.GetRequestId() == 0 || message.GetOutcome() < uint32(protocol.KillSucceeded) || message.GetOutcome() > uint32(protocol.KillOutcomeUnknown) { | |
| 220 | + | return protocol.KillResult{}, errProtoConvertRange | |
| 221 | + | } | |
| 222 | + | code, err := mustUint16(message.GetCode()) | |
| 223 | + | if err != nil { | |
| 224 | + | return protocol.KillResult{}, err | |
| 225 | + | } | |
| 226 | + | out := protocol.KillResult{RequestID: message.GetRequestId(), Outcome: protocol.KillOutcome(message.GetOutcome()), Code: code, Text: message.GetText()} | |
| 227 | + | for _, failure := range message.GetFailures() { | |
| 228 | + | if failure == nil { | |
| 229 | + | return protocol.KillResult{}, errProtoConvertRange | |
| 230 | + | } | |
| 231 | + | out.Failures = append(out.Failures, protocol.KillFailure{Class: failure.GetClass(), Name: failure.GetName(), Text: failure.GetText()}) | |
| 232 | + | } | |
| 233 | + | return out, nil | |
| 205 | 234 | } | |
| 206 | 235 | ||
| 207 | 236 | func sessionsToWire(message protocol.Sessions) *wire.Sessions { | |
| @@ -187,7 +187,7 @@ func detachedFromWire(message *wire.Detached) (protocol.Detached, error) { | |||||
| 187 | } | 187 | } | ||
| 188 | 188 | ||||
| 189 | func killToWire(message protocol.Kill) *wire.Kill { | 189 | func killToWire(message protocol.Kill) *wire.Kill { | ||
| 190 | - | return &wire.Kill{Name: message.Name, Scope: uint32(message.Scope)} | 190 | + | return &wire.Kill{Name: message.Name, Scope: uint32(message.Scope), RequestId: message.RequestID} |
| 191 | } | 191 | } | ||
| 192 | 192 | ||||
| 193 | func killFromWire(message *wire.Kill) (protocol.Kill, error) { | 193 | func killFromWire(message *wire.Kill) (protocol.Kill, error) { | ||
| @@ -201,7 +201,36 @@ func killFromWire(message *wire.Kill) (protocol.Kill, error) { | |||||
| 201 | if scope != protocol.KillSession && scope != protocol.KillDaemon && scope != protocol.KillAll { | 201 | if scope != protocol.KillSession && scope != protocol.KillDaemon && scope != protocol.KillAll { | ||
| 202 | return protocol.Kill{}, errProtoConvertRange | 202 | return protocol.Kill{}, errProtoConvertRange | ||
| 203 | } | 203 | } | ||
| 204 | - | return protocol.Kill{Name: message.GetName(), Scope: scope}, nil | 204 | + | if message.GetRequestId() == 0 { |
| 205 | + | return protocol.Kill{}, errProtoConvertRange | |||
| 206 | + | } | |||
| 207 | + | return protocol.Kill{RequestID: message.GetRequestId(), Name: message.GetName(), Scope: scope}, nil | |||
| 208 | + | } | |||
| 209 | + | ||||
| 210 | + | func killResultToWire(message protocol.KillResult) *wire.KillResult { | |||
| 211 | + | out := &wire.KillResult{RequestId: message.RequestID, Outcome: uint32(message.Outcome), Code: uint32(message.Code), Text: message.Text} | |||
| 212 | + | for _, failure := range message.Failures { | |||
| 213 | + | out.Failures = append(out.Failures, &wire.KillFailure{Class: failure.Class, Name: failure.Name, Text: failure.Text}) | |||
| 214 | + | } | |||
| 215 | + | return out | |||
| 216 | + | } | |||
| 217 | + | ||||
| 218 | + | func killResultFromWire(message *wire.KillResult) (protocol.KillResult, error) { | |||
| 219 | + | if message == nil || message.GetRequestId() == 0 || message.GetOutcome() < uint32(protocol.KillSucceeded) || message.GetOutcome() > uint32(protocol.KillOutcomeUnknown) { | |||
| 220 | + | return protocol.KillResult{}, errProtoConvertRange | |||
| 221 | + | } | |||
| 222 | + | code, err := mustUint16(message.GetCode()) | |||
| 223 | + | if err != nil { | |||
| 224 | + | return protocol.KillResult{}, err | |||
| 225 | + | } | |||
| 226 | + | out := protocol.KillResult{RequestID: message.GetRequestId(), Outcome: protocol.KillOutcome(message.GetOutcome()), Code: code, Text: message.GetText()} | |||
| 227 | + | for _, failure := range message.GetFailures() { | |||
| 228 | + | if failure == nil { | |||
| 229 | + | return protocol.KillResult{}, errProtoConvertRange | |||
| 230 | + | } | |||
| 231 | + | out.Failures = append(out.Failures, protocol.KillFailure{Class: failure.GetClass(), Name: failure.GetName(), Text: failure.GetText()}) | |||
| 232 | + | } | |||
| 233 | + | return out, nil | |||
| 205 | } | 234 | } | ||
| 206 | 235 | ||||
| 207 | func sessionsToWire(message protocol.Sessions) *wire.Sessions { | 236 | func sessionsToWire(message protocol.Sessions) *wire.Sessions { | ||
M internal/adapters/sessionwire/proto_convert_test.go +3 -1
| @@ -42,7 +42,7 @@ func TestProtoClientRoundTrips(t *testing.T) { | |||
| 42 | 42 | protocol.Detach{}, | |
| 43 | 43 | protocol.Ping{}, | |
| 44 | 44 | protocol.List{}, | |
| 45 | - | protocol.Kill{Name: "work"}, | |
| 45 | + | protocol.Kill{RequestID: 1, Name: "work"}, | |
| 46 | 46 | protocol.Theme{TrueColor: true}, | |
| 47 | 47 | protocol.Ack{Epoch: 1, State: 1}, | |
| 48 | 48 | protocol.ImagePush{InputSeq: 1, Mime: "image/png", Data: []byte{1}}, | |
| @@ -94,6 +94,8 @@ func TestProtoServerRoundTrips(t *testing.T) { | |||
| 94 | 94 | protocol.Pong{}, | |
| 95 | 95 | protocol.Sessions{}, | |
| 96 | 96 | protocol.CommandResult{RequestID: 1, OK: true}, | |
| 97 | + | protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded}, | |
| 98 | + | protocol.KillResult{RequestID: 2, Outcome: protocol.KillFailed, Code: protocol.ErrNoSuchSession, Text: "no such session: work", Failures: []protocol.KillFailure{{Class: "stopped", Name: "work", Text: "delete failed"}}}, | |
| 97 | 99 | protocol.AttachTarget{Session: "work", Intent: protocol.IntentAttach}, | |
| 98 | 100 | protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewUnavailable}, | |
| 99 | 101 | protocol.CommittedRouteIdentity{Target: exact}, | |
| @@ -42,7 +42,7 @@ func TestProtoClientRoundTrips(t *testing.T) { | |||||
| 42 | protocol.Detach{}, | 42 | protocol.Detach{}, | ||
| 43 | protocol.Ping{}, | 43 | protocol.Ping{}, | ||
| 44 | protocol.List{}, | 44 | protocol.List{}, | ||
| 45 | - | protocol.Kill{Name: "work"}, | 45 | + | protocol.Kill{RequestID: 1, Name: "work"}, |
| 46 | protocol.Theme{TrueColor: true}, | 46 | protocol.Theme{TrueColor: true}, | ||
| 47 | protocol.Ack{Epoch: 1, State: 1}, | 47 | protocol.Ack{Epoch: 1, State: 1}, | ||
| 48 | protocol.ImagePush{InputSeq: 1, Mime: "image/png", Data: []byte{1}}, | 48 | protocol.ImagePush{InputSeq: 1, Mime: "image/png", Data: []byte{1}}, | ||
| @@ -94,6 +94,8 @@ func TestProtoServerRoundTrips(t *testing.T) { | |||||
| 94 | protocol.Pong{}, | 94 | protocol.Pong{}, | ||
| 95 | protocol.Sessions{}, | 95 | protocol.Sessions{}, | ||
| 96 | protocol.CommandResult{RequestID: 1, OK: true}, | 96 | protocol.CommandResult{RequestID: 1, OK: true}, | ||
| 97 | + | protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded}, | |||
| 98 | + | protocol.KillResult{RequestID: 2, Outcome: protocol.KillFailed, Code: protocol.ErrNoSuchSession, Text: "no such session: work", Failures: []protocol.KillFailure{{Class: "stopped", Name: "work", Text: "delete failed"}}}, | |||
| 97 | protocol.AttachTarget{Session: "work", Intent: protocol.IntentAttach}, | 99 | protocol.AttachTarget{Session: "work", Intent: protocol.IntentAttach}, | ||
| 98 | protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewUnavailable}, | 100 | protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewUnavailable}, | ||
| 99 | protocol.CommittedRouteIdentity{Target: exact}, | 101 | protocol.CommittedRouteIdentity{Target: exact}, | ||
M internal/adapters/sessionwire/proto_envelope.go +9 -0
| @@ -394,6 +394,13 @@ func encodeProtoServer(message protocol.ServerMessage) (*wire.ServerEnvelope, er | |||
| 394 | 394 | return encodeProtoServer(*m) | |
| 395 | 395 | case protocol.CommandResult: | |
| 396 | 396 | return &wire.ServerEnvelope{Payload: &wire.ServerEnvelope_CommandResult{CommandResult: commandResultToWire(m)}}, nil | |
| 397 | + | case protocol.KillResult: | |
| 398 | + | return &wire.ServerEnvelope{Payload: &wire.ServerEnvelope_KillResult{KillResult: killResultToWire(m)}}, nil | |
| 399 | + | case *protocol.KillResult: | |
| 400 | + | if m == nil { | |
| 401 | + | return nil, ErrInvalidMessage | |
| 402 | + | } | |
| 403 | + | return encodeProtoServer(*m) | |
| 397 | 404 | case *protocol.CommandResult: | |
| 398 | 405 | if m == nil { | |
| 399 | 406 | return nil, ErrInvalidMessage | |
| @@ -732,6 +739,8 @@ func decodeProtoServer(envelope *wire.ServerEnvelope) (protocol.ServerMessage, e | |||
| 732 | 739 | return sessionsFromWire(payload.Sessions) | |
| 733 | 740 | case *wire.ServerEnvelope_CommandResult: | |
| 734 | 741 | return commandResultFromWire(payload.CommandResult) | |
| 742 | + | case *wire.ServerEnvelope_KillResult: | |
| 743 | + | return killResultFromWire(payload.KillResult) | |
| 735 | 744 | case *wire.ServerEnvelope_AttachTarget: | |
| 736 | 745 | return attachTargetFromWire(payload.AttachTarget) | |
| 737 | 746 | case *wire.ServerEnvelope_RemotePreview: | |
| @@ -394,6 +394,13 @@ func encodeProtoServer(message protocol.ServerMessage) (*wire.ServerEnvelope, er | |||||
| 394 | return encodeProtoServer(*m) | 394 | return encodeProtoServer(*m) | ||
| 395 | case protocol.CommandResult: | 395 | case protocol.CommandResult: | ||
| 396 | return &wire.ServerEnvelope{Payload: &wire.ServerEnvelope_CommandResult{CommandResult: commandResultToWire(m)}}, nil | 396 | return &wire.ServerEnvelope{Payload: &wire.ServerEnvelope_CommandResult{CommandResult: commandResultToWire(m)}}, nil | ||
| 397 | + | case protocol.KillResult: | |||
| 398 | + | return &wire.ServerEnvelope{Payload: &wire.ServerEnvelope_KillResult{KillResult: killResultToWire(m)}}, nil | |||
| 399 | + | case *protocol.KillResult: | |||
| 400 | + | if m == nil { | |||
| 401 | + | return nil, ErrInvalidMessage | |||
| 402 | + | } | |||
| 403 | + | return encodeProtoServer(*m) | |||
| 397 | case *protocol.CommandResult: | 404 | case *protocol.CommandResult: | ||
| 398 | if m == nil { | 405 | if m == nil { | ||
| 399 | return nil, ErrInvalidMessage | 406 | return nil, ErrInvalidMessage | ||
| @@ -732,6 +739,8 @@ func decodeProtoServer(envelope *wire.ServerEnvelope) (protocol.ServerMessage, e | |||||
| 732 | return sessionsFromWire(payload.Sessions) | 739 | return sessionsFromWire(payload.Sessions) | ||
| 733 | case *wire.ServerEnvelope_CommandResult: | 740 | case *wire.ServerEnvelope_CommandResult: | ||
| 734 | return commandResultFromWire(payload.CommandResult) | 741 | return commandResultFromWire(payload.CommandResult) | ||
| 742 | + | case *wire.ServerEnvelope_KillResult: | |||
| 743 | + | return killResultFromWire(payload.KillResult) | |||
| 735 | case *wire.ServerEnvelope_AttachTarget: | 744 | case *wire.ServerEnvelope_AttachTarget: | ||
| 736 | return attachTargetFromWire(payload.AttachTarget) | 745 | return attachTargetFromWire(payload.AttachTarget) | ||
| 737 | case *wire.ServerEnvelope_RemotePreview: | 746 | case *wire.ServerEnvelope_RemotePreview: | ||
M internal/adapters/sessionwire/server_test.go +5 -1
| @@ -27,7 +27,7 @@ func TestServerConnectionDecodesEveryClientMessage(t *testing.T) { | |||
| 27 | 27 | {name: "detach", want: protocol.Detach{}}, | |
| 28 | 28 | {name: "ping", want: protocol.Ping{}}, | |
| 29 | 29 | {name: "list", want: protocol.List{}}, | |
| 30 | - | {name: "kill", want: protocol.Kill{Name: "work"}}, | |
| 30 | + | {name: "kill", want: protocol.Kill{RequestID: 1, Name: "work"}}, | |
| 31 | 31 | {name: "theme", want: protocol.Theme{TrueColor: true}}, | |
| 32 | 32 | {name: "ack", want: protocol.Ack{Epoch: 1, State: 2}}, | |
| 33 | 33 | {name: "image", want: protocol.ImagePush{InputSeq: 2, Mime: "image/png", Data: []byte{1}}}, | |
| @@ -62,6 +62,8 @@ func TestServerConnectionEncodesEveryServerMessage(t *testing.T) { | |||
| 62 | 62 | pong := protocol.Pong{} | |
| 63 | 63 | sessions := protocol.Sessions{} | |
| 64 | 64 | commandResult := protocol.CommandResult{RequestID: 1, OK: true} | |
| 65 | + | killResult := protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded} | |
| 66 | + | killFailure := protocol.KillResult{RequestID: 2, Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "partial", Failures: []protocol.KillFailure{{Class: "stopped", Name: "work", Text: "boom"}}} | |
| 65 | 67 | attachTarget := protocol.AttachTarget{Session: "work", Intent: protocol.IntentAttach} | |
| 66 | 68 | preview := protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewUnavailable} | |
| 67 | 69 | identity := protocol.CommittedRouteIdentity{Target: exact} | |
| @@ -86,6 +88,8 @@ func TestServerConnectionEncodesEveryServerMessage(t *testing.T) { | |||
| 86 | 88 | {name: "pong", message: pong}, | |
| 87 | 89 | {name: "sessions", message: sessions}, | |
| 88 | 90 | {name: "command result", message: commandResult}, | |
| 91 | + | {name: "kill result", message: killResult}, | |
| 92 | + | {name: "kill result partial failures", message: killFailure}, | |
| 89 | 93 | {name: "attach target", message: attachTarget}, | |
| 90 | 94 | {name: "preview", message: preview}, | |
| 91 | 95 | {name: "identity", message: identity}, | |
| @@ -27,7 +27,7 @@ func TestServerConnectionDecodesEveryClientMessage(t *testing.T) { | |||||
| 27 | {name: "detach", want: protocol.Detach{}}, | 27 | {name: "detach", want: protocol.Detach{}}, | ||
| 28 | {name: "ping", want: protocol.Ping{}}, | 28 | {name: "ping", want: protocol.Ping{}}, | ||
| 29 | {name: "list", want: protocol.List{}}, | 29 | {name: "list", want: protocol.List{}}, | ||
| 30 | - | {name: "kill", want: protocol.Kill{Name: "work"}}, | 30 | + | {name: "kill", want: protocol.Kill{RequestID: 1, Name: "work"}}, |
| 31 | {name: "theme", want: protocol.Theme{TrueColor: true}}, | 31 | {name: "theme", want: protocol.Theme{TrueColor: true}}, | ||
| 32 | {name: "ack", want: protocol.Ack{Epoch: 1, State: 2}}, | 32 | {name: "ack", want: protocol.Ack{Epoch: 1, State: 2}}, | ||
| 33 | {name: "image", want: protocol.ImagePush{InputSeq: 2, Mime: "image/png", Data: []byte{1}}}, | 33 | {name: "image", want: protocol.ImagePush{InputSeq: 2, Mime: "image/png", Data: []byte{1}}}, | ||
| @@ -62,6 +62,8 @@ func TestServerConnectionEncodesEveryServerMessage(t *testing.T) { | |||||
| 62 | pong := protocol.Pong{} | 62 | pong := protocol.Pong{} | ||
| 63 | sessions := protocol.Sessions{} | 63 | sessions := protocol.Sessions{} | ||
| 64 | commandResult := protocol.CommandResult{RequestID: 1, OK: true} | 64 | commandResult := protocol.CommandResult{RequestID: 1, OK: true} | ||
| 65 | + | killResult := protocol.KillResult{RequestID: 1, Outcome: protocol.KillSucceeded} | |||
| 66 | + | killFailure := protocol.KillResult{RequestID: 2, Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "partial", Failures: []protocol.KillFailure{{Class: "stopped", Name: "work", Text: "boom"}}} | |||
| 65 | attachTarget := protocol.AttachTarget{Session: "work", Intent: protocol.IntentAttach} | 67 | attachTarget := protocol.AttachTarget{Session: "work", Intent: protocol.IntentAttach} | ||
| 66 | preview := protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewUnavailable} | 68 | preview := protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewUnavailable} | ||
| 67 | identity := protocol.CommittedRouteIdentity{Target: exact} | 69 | identity := protocol.CommittedRouteIdentity{Target: exact} | ||
| @@ -86,6 +88,8 @@ func TestServerConnectionEncodesEveryServerMessage(t *testing.T) { | |||||
| 86 | {name: "pong", message: pong}, | 88 | {name: "pong", message: pong}, | ||
| 87 | {name: "sessions", message: sessions}, | 89 | {name: "sessions", message: sessions}, | ||
| 88 | {name: "command result", message: commandResult}, | 90 | {name: "command result", message: commandResult}, | ||
| 91 | + | {name: "kill result", message: killResult}, | |||
| 92 | + | {name: "kill result partial failures", message: killFailure}, | |||
| 89 | {name: "attach target", message: attachTarget}, | 93 | {name: "attach target", message: attachTarget}, | ||
| 90 | {name: "preview", message: preview}, | 94 | {name: "preview", message: preview}, | ||
| 91 | {name: "identity", message: identity}, | 95 | {name: "identity", message: identity}, | ||
M internal/app/connectivity_matrix.go +1 -1
| @@ -117,7 +117,7 @@ var connectivityMatrix = []connectivityEntry{ | |||
| 117 | 117 | "forceStopDaemonFallback (force_stop.go)", | |
| 118 | 118 | }, | |
| 119 | 119 | PersistMutation: true, | |
| 120 | - | Notes: "Offline runOfflineNamedKill mutation becomes daemon-owned via broker. Kill-all purges sessions and leaves the daemon running; daemon-stop is the distinct explicit stop.", | |
| 120 | + | Notes: "Offline runOfflineNamedKill mutation becomes daemon-owned via broker. Kill-all purges sessions and leaves the daemon running; daemon-stop is the distinct explicit stop. Direct force-stop is offered only when the typed endpoint is unreachable, never after a request may have been delivered.", | |
| 121 | 121 | }, | |
| 122 | 122 | { | |
| 123 | 123 | Kind: "kindCmd", | |
| @@ -117,7 +117,7 @@ var connectivityMatrix = []connectivityEntry{ | |||||
| 117 | "forceStopDaemonFallback (force_stop.go)", | 117 | "forceStopDaemonFallback (force_stop.go)", | ||
| 118 | }, | 118 | }, | ||
| 119 | PersistMutation: true, | 119 | PersistMutation: true, | ||
| 120 | - | Notes: "Offline runOfflineNamedKill mutation becomes daemon-owned via broker. Kill-all purges sessions and leaves the daemon running; daemon-stop is the distinct explicit stop.", | 120 | + | Notes: "Offline runOfflineNamedKill mutation becomes daemon-owned via broker. Kill-all purges sessions and leaves the daemon running; daemon-stop is the distinct explicit stop. Direct force-stop is offered only when the typed endpoint is unreachable, never after a request may have been delivered.", |
| 121 | }, | 121 | }, | ||
| 122 | { | 122 | { | ||
| 123 | Kind: "kindCmd", | 123 | Kind: "kindCmd", | ||
M internal/app/daemon_integration_test.go +92 -41
| @@ -150,62 +150,53 @@ func killDaemon(dir string) error { | |||
| 150 | 150 | return requestKill(dir, protocol.KillDaemon) | |
| 151 | 151 | } | |
| 152 | 152 | ||
| 153 | + | // nextKillRequestID allocates a unique nonzero correlation ID for one control | |
| 154 | + | // request, matching the production caller contract. | |
| 155 | + | var killRequestSeq atomic.Uint64 | |
| 156 | + | ||
| 157 | + | func nextKillRequestID() uint64 { return killRequestSeq.Add(1) } | |
| 158 | + | ||
| 153 | 159 | // killNamedSession deletes one named live or stopped session over the control | |
| 154 | 160 | // connection, using the same wire scope the client sends for `kill -s NAME`. | |
| 161 | + | // It sends a unique RequestID and consumes the matching KillResult: a failed | |
| 162 | + | // outcome is an error, and a close without a result is never read as success. | |
| 155 | 163 | func killNamedSession(dir, name string) error { | |
| 156 | - | raw, err := ipc.DialContext(context.Background(), dir) | |
| 157 | - | if err != nil { | |
| 158 | - | return err | |
| 159 | - | } | |
| 160 | - | conn := sessionwire.NewClientConnection(raw) | |
| 161 | - | defer func() { _ = conn.Close() }() | |
| 162 | - | if err := conn.SendClient(protocol.Kill{Scope: protocol.KillSession, Name: name}); err != nil { | |
| 163 | - | return err | |
| 164 | - | } | |
| 165 | - | reply, err := receiveKillReply(conn, daemonStopTimeout) | |
| 166 | - | if errors.Is(err, context.DeadlineExceeded) { | |
| 167 | - | return err | |
| 168 | - | } | |
| 169 | - | if errors.Is(err, io.EOF) { | |
| 170 | - | return nil | |
| 171 | - | } | |
| 172 | - | if err != nil { | |
| 173 | - | return err | |
| 174 | - | } | |
| 175 | - | if em, ok := reply.(protocol.ErrorMsg); ok { | |
| 176 | - | return fmt.Errorf("kill failed: %s", em.Text) | |
| 177 | - | } | |
| 178 | - | return nil | |
| 164 | + | return controlKill(dir, protocol.KillScope(protocol.KillSession), name) | |
| 179 | 165 | } | |
| 180 | 166 | ||
| 181 | 167 | func requestKill(dir string, scope protocol.KillScope) error { | |
| 168 | + | return controlKill(dir, scope, "") | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | func controlKill(dir string, scope protocol.KillScope, name string) error { | |
| 182 | 172 | raw, err := ipc.DialContext(context.Background(), dir) | |
| 183 | 173 | if err != nil { | |
| 184 | 174 | return err | |
| 185 | 175 | } | |
| 186 | 176 | conn := sessionwire.NewClientConnection(raw) | |
| 187 | 177 | defer func() { _ = conn.Close() }() | |
| 188 | - | if err := conn.SendClient(protocol.Kill{Scope: scope}); err != nil { | |
| 189 | - | return err | |
| 190 | - | } | |
| 191 | - | reply, err := receiveKillReply(conn, daemonStopTimeout) | |
| 192 | - | if errors.Is(err, context.DeadlineExceeded) { | |
| 178 | + | requestID := nextKillRequestID() | |
| 179 | + | if err := conn.SendClient(protocol.Kill{RequestID: requestID, Scope: scope, Name: name}); err != nil { | |
| 193 | 180 | return err | |
| 194 | 181 | } | |
| 195 | - | if errors.Is(err, io.EOF) { | |
| 196 | - | return nil | |
| 197 | - | } | |
| 182 | + | result, err := awaitKillResult(conn, requestID, daemonStopTimeout) | |
| 198 | 183 | if err != nil { | |
| 199 | 184 | return err | |
| 200 | 185 | } | |
| 201 | - | em, ok := reply.(protocol.ErrorMsg) | |
| 202 | - | if !ok { | |
| 186 | + | switch result.Outcome { | |
| 187 | + | case protocol.KillSucceeded: | |
| 203 | 188 | return nil | |
| 189 | + | case protocol.KillFailed: | |
| 190 | + | return fmt.Errorf("kill failed: %s", result.Text) | |
| 191 | + | default: | |
| 192 | + | return fmt.Errorf("kill outcome unknown: %s", result.Text) | |
| 204 | 193 | } | |
| 205 | - | return fmt.Errorf("kill failed: %s", em.Text) | |
| 206 | 194 | } | |
| 207 | 195 | ||
| 208 | - | func receiveKillReply(conn ports.ClientConnection, timeout time.Duration) (protocol.ServerMessage, error) { | |
| 196 | + | // awaitKillResult reads one correlated KillResult with a bounded timeout. A | |
| 197 | + | // close, a wrong type, or a mismatched RequestID is a definite error rather | |
| 198 | + | // than an inferred success, so a lost result is never reported as a kill. | |
| 199 | + | func awaitKillResult(conn ports.ClientConnection, requestID uint64, timeout time.Duration) (protocol.KillResult, error) { | |
| 209 | 200 | type result struct { | |
| 210 | 201 | message protocol.ServerMessage | |
| 211 | 202 | err error | |
| @@ -218,16 +209,29 @@ func receiveKillReply(conn ports.ClientConnection, timeout time.Duration) (proto | |||
| 218 | 209 | ||
| 219 | 210 | timer := time.NewTimer(timeout) | |
| 220 | 211 | defer timer.Stop() | |
| 212 | + | var reply result | |
| 221 | 213 | select { | |
| 222 | - | case reply := <-received: | |
| 223 | - | return reply.message, reply.err | |
| 214 | + | case reply = <-received: | |
| 224 | 215 | case <-timer.C: | |
| 225 | 216 | _ = conn.Close() | |
| 226 | - | reply := <-received | |
| 227 | - | return reply.message, errors.Join(context.DeadlineExceeded, reply.err) | |
| 217 | + | reply = <-received | |
| 218 | + | return protocol.KillResult{}, errors.Join(context.DeadlineExceeded, reply.err) | |
| 228 | 219 | } | |
| 220 | + | if reply.err != nil { | |
| 221 | + | return protocol.KillResult{}, reply.err | |
| 222 | + | } | |
| 223 | + | killResult, ok := reply.message.(protocol.KillResult) | |
| 224 | + | if !ok { | |
| 225 | + | return protocol.KillResult{}, fmt.Errorf("unexpected reply %T to kill", reply.message) | |
| 226 | + | } | |
| 227 | + | if killResult.RequestID != requestID { | |
| 228 | + | return protocol.KillResult{}, fmt.Errorf("kill result request id %d, want %d", killResult.RequestID, requestID) | |
| 229 | + | } | |
| 230 | + | return killResult, nil | |
| 229 | 231 | } | |
| 230 | 232 | ||
| 233 | + | // blockingClientConnection parks ReceiveServer until Close, so a test can prove | |
| 234 | + | // the bounded await returns on timeout rather than hanging. | |
| 231 | 235 | type blockingClientConnection struct { | |
| 232 | 236 | closed chan struct{} | |
| 233 | 237 | once sync.Once | |
| @@ -252,13 +256,60 @@ func (c *blockingClientConnection) Close() error { | |||
| 252 | 256 | return nil | |
| 253 | 257 | } | |
| 254 | 258 | ||
| 255 | - | func TestReceiveKillReplyTimesOut(t *testing.T) { | |
| 259 | + | // TestAwaitKillResultTimesOut proves the bounded result await reports a timeout | |
| 260 | + | // (never a silent success) and closes the exact connection when no | |
| 261 | + | // KillResult arrives. | |
| 262 | + | func TestAwaitKillResultTimesOut(t *testing.T) { | |
| 256 | 263 | conn := &blockingClientConnection{closed: make(chan struct{})} | |
| 257 | - | _, err := receiveKillReply(conn, 10*time.Millisecond) | |
| 264 | + | _, err := awaitKillResult(conn, 1, 10*time.Millisecond) | |
| 258 | 265 | require.ErrorIs(t, err, context.DeadlineExceeded) | |
| 259 | 266 | require.ErrorIs(t, err, io.ErrClosedPipe) | |
| 260 | 267 | } | |
| 261 | 268 | ||
| 269 | + | // TestAwaitKillResultRejectsUncorrelatedReply proves a reply of the wrong type | |
| 270 | + | // or with a mismatched RequestID is a definite error, so a lost or stale result | |
| 271 | + | // is never mistaken for this request's success. | |
| 272 | + | func TestAwaitKillResultRejectsUncorrelatedReply(t *testing.T) { | |
| 273 | + | tests := []struct { | |
| 274 | + | name string | |
| 275 | + | reply protocol.ServerMessage | |
| 276 | + | }{ | |
| 277 | + | {name: "wrong type", reply: protocol.Pong{}}, | |
| 278 | + | {name: "wrong request id", reply: protocol.KillResult{RequestID: 9, Outcome: protocol.KillSucceeded}}, | |
| 279 | + | } | |
| 280 | + | for _, tt := range tests { | |
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |
| 282 | + | conn := &scriptedKillReplyConnection{reply: tt.reply} | |
| 283 | + | _, err := awaitKillResult(conn, 1, time.Second) | |
| 284 | + | require.Error(t, err) | |
| 285 | + | }) | |
| 286 | + | } | |
| 287 | + | } | |
| 288 | + | ||
| 289 | + | // scriptedKillReplyConnection returns exactly one canned server message. | |
| 290 | + | type scriptedKillReplyConnection struct { | |
| 291 | + | reply protocol.ServerMessage | |
| 292 | + | } | |
| 293 | + | ||
| 294 | + | func (*scriptedKillReplyConnection) SendClient(protocol.ClientMessage) error { return nil } | |
| 295 | + | ||
| 296 | + | func (c *scriptedKillReplyConnection) ReceiveServer() (protocol.ServerMessage, error) { | |
| 297 | + | if c.reply == nil { | |
| 298 | + | return nil, io.EOF | |
| 299 | + | } | |
| 300 | + | reply := c.reply | |
| 301 | + | c.reply = nil | |
| 302 | + | return reply, nil | |
| 303 | + | } | |
| 304 | + | ||
| 305 | + | func (*scriptedKillReplyConnection) Capabilities() protocol.ConnectionCapabilities { | |
| 306 | + | return protocol.ConnectionCapabilities{} | |
| 307 | + | } | |
| 308 | + | ||
| 309 | + | func (*scriptedKillReplyConnection) LinkState() ports.LinkState { return ports.LinkState(0) } | |
| 310 | + | func (*scriptedKillReplyConnection) LinkEvents() <-chan ports.LinkEvent { return nil } | |
| 311 | + | func (*scriptedKillReplyConnection) Close() error { return nil } | |
| 312 | + | ||
| 262 | 313 | // mustHelloBytes encodes a minimal valid Hello for first-frame shape probes. | |
| 263 | 314 | func mustHelloBytes() []byte { | |
| 264 | 315 | raw, err := sessionwire.EncodeClientMessage(protocol.Hello{ | |
| @@ -150,62 +150,53 @@ func killDaemon(dir string) error { | |||||
| 150 | return requestKill(dir, protocol.KillDaemon) | 150 | return requestKill(dir, protocol.KillDaemon) | ||
| 151 | } | 151 | } | ||
| 152 | 152 | ||||
| 153 | + | // nextKillRequestID allocates a unique nonzero correlation ID for one control | |||
| 154 | + | // request, matching the production caller contract. | |||
| 155 | + | var killRequestSeq atomic.Uint64 | |||
| 156 | + | ||||
| 157 | + | func nextKillRequestID() uint64 { return killRequestSeq.Add(1) } | |||
| 158 | + | ||||
| 153 | // killNamedSession deletes one named live or stopped session over the control | 159 | // killNamedSession deletes one named live or stopped session over the control | ||
| 154 | // connection, using the same wire scope the client sends for `kill -s NAME`. | 160 | // connection, using the same wire scope the client sends for `kill -s NAME`. | ||
| 161 | + | // It sends a unique RequestID and consumes the matching KillResult: a failed | |||
| 162 | + | // outcome is an error, and a close without a result is never read as success. | |||
| 155 | func killNamedSession(dir, name string) error { | 163 | func killNamedSession(dir, name string) error { | ||
| 156 | - | raw, err := ipc.DialContext(context.Background(), dir) | 164 | + | return controlKill(dir, protocol.KillScope(protocol.KillSession), name) |
| 157 | - | if err != nil { | |||
| 158 | - | return err | |||
| 159 | - | } | |||
| 160 | - | conn := sessionwire.NewClientConnection(raw) | |||
| 161 | - | defer func() { _ = conn.Close() }() | |||
| 162 | - | if err := conn.SendClient(protocol.Kill{Scope: protocol.KillSession, Name: name}); err != nil { | |||
| 163 | - | return err | |||
| 164 | - | } | |||
| 165 | - | reply, err := receiveKillReply(conn, daemonStopTimeout) | |||
| 166 | - | if errors.Is(err, context.DeadlineExceeded) { | |||
| 167 | - | return err | |||
| 168 | - | } | |||
| 169 | - | if errors.Is(err, io.EOF) { | |||
| 170 | - | return nil | |||
| 171 | - | } | |||
| 172 | - | if err != nil { | |||
| 173 | - | return err | |||
| 174 | - | } | |||
| 175 | - | if em, ok := reply.(protocol.ErrorMsg); ok { | |||
| 176 | - | return fmt.Errorf("kill failed: %s", em.Text) | |||
| 177 | - | } | |||
| 178 | - | return nil | |||
| 179 | } | 165 | } | ||
| 180 | 166 | ||||
| 181 | func requestKill(dir string, scope protocol.KillScope) error { | 167 | func requestKill(dir string, scope protocol.KillScope) error { | ||
| 168 | + | return controlKill(dir, scope, "") | |||
| 169 | + | } | |||
| 170 | + | ||||
| 171 | + | func controlKill(dir string, scope protocol.KillScope, name string) error { | |||
| 182 | raw, err := ipc.DialContext(context.Background(), dir) | 172 | raw, err := ipc.DialContext(context.Background(), dir) | ||
| 183 | if err != nil { | 173 | if err != nil { | ||
| 184 | return err | 174 | return err | ||
| 185 | } | 175 | } | ||
| 186 | conn := sessionwire.NewClientConnection(raw) | 176 | conn := sessionwire.NewClientConnection(raw) | ||
| 187 | defer func() { _ = conn.Close() }() | 177 | defer func() { _ = conn.Close() }() | ||
| 188 | - | if err := conn.SendClient(protocol.Kill{Scope: scope}); err != nil { | 178 | + | requestID := nextKillRequestID() |
| 189 | - | return err | 179 | + | if err := conn.SendClient(protocol.Kill{RequestID: requestID, Scope: scope, Name: name}); err != nil { |
| 190 | - | } | |||
| 191 | - | reply, err := receiveKillReply(conn, daemonStopTimeout) | |||
| 192 | - | if errors.Is(err, context.DeadlineExceeded) { | |||
| 193 | return err | 180 | return err | ||
| 194 | } | 181 | } | ||
| 195 | - | if errors.Is(err, io.EOF) { | 182 | + | result, err := awaitKillResult(conn, requestID, daemonStopTimeout) |
| 196 | - | return nil | |||
| 197 | - | } | |||
| 198 | if err != nil { | 183 | if err != nil { | ||
| 199 | return err | 184 | return err | ||
| 200 | } | 185 | } | ||
| 201 | - | em, ok := reply.(protocol.ErrorMsg) | 186 | + | switch result.Outcome { |
| 202 | - | if !ok { | 187 | + | case protocol.KillSucceeded: |
| 203 | return nil | 188 | return nil | ||
| 189 | + | case protocol.KillFailed: | |||
| 190 | + | return fmt.Errorf("kill failed: %s", result.Text) | |||
| 191 | + | default: | |||
| 192 | + | return fmt.Errorf("kill outcome unknown: %s", result.Text) | |||
| 204 | } | 193 | } | ||
| 205 | - | return fmt.Errorf("kill failed: %s", em.Text) | |||
| 206 | } | 194 | } | ||
| 207 | 195 | ||||
| 208 | - | func receiveKillReply(conn ports.ClientConnection, timeout time.Duration) (protocol.ServerMessage, error) { | 196 | + | // awaitKillResult reads one correlated KillResult with a bounded timeout. A |
| 197 | + | // close, a wrong type, or a mismatched RequestID is a definite error rather | |||
| 198 | + | // than an inferred success, so a lost result is never reported as a kill. | |||
| 199 | + | func awaitKillResult(conn ports.ClientConnection, requestID uint64, timeout time.Duration) (protocol.KillResult, error) { | |||
| 209 | type result struct { | 200 | type result struct { | ||
| 210 | message protocol.ServerMessage | 201 | message protocol.ServerMessage | ||
| 211 | err error | 202 | err error | ||
| @@ -218,16 +209,29 @@ func receiveKillReply(conn ports.ClientConnection, timeout time.Duration) (proto | |||||
| 218 | 209 | ||||
| 219 | timer := time.NewTimer(timeout) | 210 | timer := time.NewTimer(timeout) | ||
| 220 | defer timer.Stop() | 211 | defer timer.Stop() | ||
| 212 | + | var reply result | |||
| 221 | select { | 213 | select { | ||
| 222 | - | case reply := <-received: | 214 | + | case reply = <-received: |
| 223 | - | return reply.message, reply.err | |||
| 224 | case <-timer.C: | 215 | case <-timer.C: | ||
| 225 | _ = conn.Close() | 216 | _ = conn.Close() | ||
| 226 | - | reply := <-received | 217 | + | reply = <-received |
| 227 | - | return reply.message, errors.Join(context.DeadlineExceeded, reply.err) | 218 | + | return protocol.KillResult{}, errors.Join(context.DeadlineExceeded, reply.err) |
| 228 | } | 219 | } | ||
| 220 | + | if reply.err != nil { | |||
| 221 | + | return protocol.KillResult{}, reply.err | |||
| 222 | + | } | |||
| 223 | + | killResult, ok := reply.message.(protocol.KillResult) | |||
| 224 | + | if !ok { | |||
| 225 | + | return protocol.KillResult{}, fmt.Errorf("unexpected reply %T to kill", reply.message) | |||
| 226 | + | } | |||
| 227 | + | if killResult.RequestID != requestID { | |||
| 228 | + | return protocol.KillResult{}, fmt.Errorf("kill result request id %d, want %d", killResult.RequestID, requestID) | |||
| 229 | + | } | |||
| 230 | + | return killResult, nil | |||
| 229 | } | 231 | } | ||
| 230 | 232 | ||||
| 233 | + | // blockingClientConnection parks ReceiveServer until Close, so a test can prove | |||
| 234 | + | // the bounded await returns on timeout rather than hanging. | |||
| 231 | type blockingClientConnection struct { | 235 | type blockingClientConnection struct { | ||
| 232 | closed chan struct{} | 236 | closed chan struct{} | ||
| 233 | once sync.Once | 237 | once sync.Once | ||
| @@ -252,13 +256,60 @@ func (c *blockingClientConnection) Close() error { | |||||
| 252 | return nil | 256 | return nil | ||
| 253 | } | 257 | } | ||
| 254 | 258 | ||||
| 255 | - | func TestReceiveKillReplyTimesOut(t *testing.T) { | 259 | + | // TestAwaitKillResultTimesOut proves the bounded result await reports a timeout |
| 260 | + | // (never a silent success) and closes the exact connection when no | |||
| 261 | + | // KillResult arrives. | |||
| 262 | + | func TestAwaitKillResultTimesOut(t *testing.T) { | |||
| 256 | conn := &blockingClientConnection{closed: make(chan struct{})} | 263 | conn := &blockingClientConnection{closed: make(chan struct{})} | ||
| 257 | - | _, err := receiveKillReply(conn, 10*time.Millisecond) | 264 | + | _, err := awaitKillResult(conn, 1, 10*time.Millisecond) |
| 258 | require.ErrorIs(t, err, context.DeadlineExceeded) | 265 | require.ErrorIs(t, err, context.DeadlineExceeded) | ||
| 259 | require.ErrorIs(t, err, io.ErrClosedPipe) | 266 | require.ErrorIs(t, err, io.ErrClosedPipe) | ||
| 260 | } | 267 | } | ||
| 261 | 268 | ||||
| 269 | + | // TestAwaitKillResultRejectsUncorrelatedReply proves a reply of the wrong type | |||
| 270 | + | // or with a mismatched RequestID is a definite error, so a lost or stale result | |||
| 271 | + | // is never mistaken for this request's success. | |||
| 272 | + | func TestAwaitKillResultRejectsUncorrelatedReply(t *testing.T) { | |||
| 273 | + | tests := []struct { | |||
| 274 | + | name string | |||
| 275 | + | reply protocol.ServerMessage | |||
| 276 | + | }{ | |||
| 277 | + | {name: "wrong type", reply: protocol.Pong{}}, | |||
| 278 | + | {name: "wrong request id", reply: protocol.KillResult{RequestID: 9, Outcome: protocol.KillSucceeded}}, | |||
| 279 | + | } | |||
| 280 | + | for _, tt := range tests { | |||
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 282 | + | conn := &scriptedKillReplyConnection{reply: tt.reply} | |||
| 283 | + | _, err := awaitKillResult(conn, 1, time.Second) | |||
| 284 | + | require.Error(t, err) | |||
| 285 | + | }) | |||
| 286 | + | } | |||
| 287 | + | } | |||
| 288 | + | ||||
| 289 | + | // scriptedKillReplyConnection returns exactly one canned server message. | |||
| 290 | + | type scriptedKillReplyConnection struct { | |||
| 291 | + | reply protocol.ServerMessage | |||
| 292 | + | } | |||
| 293 | + | ||||
| 294 | + | func (*scriptedKillReplyConnection) SendClient(protocol.ClientMessage) error { return nil } | |||
| 295 | + | ||||
| 296 | + | func (c *scriptedKillReplyConnection) ReceiveServer() (protocol.ServerMessage, error) { | |||
| 297 | + | if c.reply == nil { | |||
| 298 | + | return nil, io.EOF | |||
| 299 | + | } | |||
| 300 | + | reply := c.reply | |||
| 301 | + | c.reply = nil | |||
| 302 | + | return reply, nil | |||
| 303 | + | } | |||
| 304 | + | ||||
| 305 | + | func (*scriptedKillReplyConnection) Capabilities() protocol.ConnectionCapabilities { | |||
| 306 | + | return protocol.ConnectionCapabilities{} | |||
| 307 | + | } | |||
| 308 | + | ||||
| 309 | + | func (*scriptedKillReplyConnection) LinkState() ports.LinkState { return ports.LinkState(0) } | |||
| 310 | + | func (*scriptedKillReplyConnection) LinkEvents() <-chan ports.LinkEvent { return nil } | |||
| 311 | + | func (*scriptedKillReplyConnection) Close() error { return nil } | |||
| 312 | + | ||||
| 262 | // mustHelloBytes encodes a minimal valid Hello for first-frame shape probes. | 313 | // mustHelloBytes encodes a minimal valid Hello for first-frame shape probes. | ||
| 263 | func mustHelloBytes() []byte { | 314 | func mustHelloBytes() []byte { | ||
| 264 | raw, err := sessionwire.EncodeClientMessage(protocol.Hello{ | 315 | raw, err := sessionwire.EncodeClientMessage(protocol.Hello{ | ||
M internal/app/force_stop.go +7 -5
| @@ -138,7 +138,7 @@ func forceStopDaemon(ctx context.Context, runtimeDir string, hooks forceStopHook | |||
| 138 | 138 | ||
| 139 | 139 | confirmed, err := hooks.confirm(fmt.Sprintf( | |
| 140 | 140 | "vev: daemon process %d (%s) still owns %s after the typed kill request failed.\n"+ | |
| 141 | - | "vev: it may be incompatible or unreachable. Send SIGTERM to process %d?", | |
| 141 | + | "vev: it is unreachable through the typed control endpoint. Send SIGTERM to process %d?", | |
| 142 | 142 | candidate.PID, candidate.Command, lifecycle.Path(runtimeDir), candidate.PID)) | |
| 143 | 143 | if err != nil { | |
| 144 | 144 | return err | |
| @@ -284,10 +284,12 @@ var forceStopDaemonFn = func(ctx context.Context) error { | |||
| 284 | 284 | return forceStopDaemon(ctx, ipc.SocketDir(), defaultForceStopHooks()) | |
| 285 | 285 | } | |
| 286 | 286 | ||
| 287 | - | // forceStopDaemonFallback offers the interactive force-stop after the typed | |
| 288 | - | // KillDaemon protocol failed. When no verified daemon process holds lifecycle | |
| 289 | - | // ownership, the original failure is returned unchanged so the caller keeps its | |
| 290 | - | // own diagnosis. | |
| 287 | + | // forceStopDaemonFallback offers the interactive force-stop when the daemon | |
| 288 | + | // cannot be reached through the typed protocol. Once a request may have been | |
| 289 | + | // delivered, the caller preserves its explicit or outcome-unknown result rather | |
| 290 | + | // than bypassing the protocol with a direct process signal. When no verified | |
| 291 | + | // daemon process holds lifecycle ownership, the original failure is returned | |
| 292 | + | // unchanged so the caller keeps its own diagnosis. | |
| 291 | 293 | func forceStopDaemonFallback(ctx context.Context, cause error) error { | |
| 292 | 294 | err := forceStopDaemonFn(ctx) | |
| 293 | 295 | if errors.Is(err, errForceStopNotApplicable) { | |
| @@ -138,7 +138,7 @@ func forceStopDaemon(ctx context.Context, runtimeDir string, hooks forceStopHook | |||||
| 138 | 138 | ||||
| 139 | confirmed, err := hooks.confirm(fmt.Sprintf( | 139 | confirmed, err := hooks.confirm(fmt.Sprintf( | ||
| 140 | "vev: daemon process %d (%s) still owns %s after the typed kill request failed.\n"+ | 140 | "vev: daemon process %d (%s) still owns %s after the typed kill request failed.\n"+ | ||
| 141 | - | "vev: it may be incompatible or unreachable. Send SIGTERM to process %d?", | 141 | + | "vev: it is unreachable through the typed control endpoint. Send SIGTERM to process %d?", |
| 142 | candidate.PID, candidate.Command, lifecycle.Path(runtimeDir), candidate.PID)) | 142 | candidate.PID, candidate.Command, lifecycle.Path(runtimeDir), candidate.PID)) | ||
| 143 | if err != nil { | 143 | if err != nil { | ||
| 144 | return err | 144 | return err | ||
| @@ -284,10 +284,12 @@ var forceStopDaemonFn = func(ctx context.Context) error { | |||||
| 284 | return forceStopDaemon(ctx, ipc.SocketDir(), defaultForceStopHooks()) | 284 | return forceStopDaemon(ctx, ipc.SocketDir(), defaultForceStopHooks()) | ||
| 285 | } | 285 | } | ||
| 286 | 286 | ||||
| 287 | - | // forceStopDaemonFallback offers the interactive force-stop after the typed | 287 | + | // forceStopDaemonFallback offers the interactive force-stop when the daemon |
| 288 | - | // KillDaemon protocol failed. When no verified daemon process holds lifecycle | 288 | + | // cannot be reached through the typed protocol. Once a request may have been |
| 289 | - | // ownership, the original failure is returned unchanged so the caller keeps its | 289 | + | // delivered, the caller preserves its explicit or outcome-unknown result rather |
| 290 | - | // own diagnosis. | 290 | + | // than bypassing the protocol with a direct process signal. When no verified |
| 291 | + | // daemon process holds lifecycle ownership, the original failure is returned | |||
| 292 | + | // unchanged so the caller keeps its own diagnosis. | |||
| 291 | func forceStopDaemonFallback(ctx context.Context, cause error) error { | 293 | func forceStopDaemonFallback(ctx context.Context, cause error) error { | ||
| 292 | err := forceStopDaemonFn(ctx) | 294 | err := forceStopDaemonFn(ctx) | ||
| 293 | if errors.Is(err, errForceStopNotApplicable) { | 295 | if errors.Is(err, errForceStopNotApplicable) { | ||
A internal/app/kill_result_test.go +429 -0
| @@ -0,0 +1,429 @@ | |||
| 1 | + | package app | |
| 2 | + | ||
| 3 | + | // KillResult caller contract (explicit result slice). runKill and | |
| 4 | + | // requestDaemonStop send one Kill with a unique nonzero RequestID and consume | |
| 5 | + | // exactly one matching KillResult; a lost, uncorrelated, or wrong-typed reply is | |
| 6 | + | // outcome-unknown and is never replayed, while a caller canceled before the | |
| 7 | + | // send is a definite not-sent outcome. These tests drive the two helpers those | |
| 8 | + | // callers share with a controllable in-memory client connection, so every case | |
| 9 | + | // is bounded and uses no sleeps. | |
| 10 | + | ||
| 11 | + | import ( | |
| 12 | + | "context" | |
| 13 | + | "errors" | |
| 14 | + | "io" | |
| 15 | + | "sync" | |
| 16 | + | "sync/atomic" | |
| 17 | + | "testing" | |
| 18 | + | "time" | |
| 19 | + | ||
| 20 | + | "github.com/stretchr/testify/require" | |
| 21 | + | ||
| 22 | + | "github.com/bnema/vev/internal/adapters/ipc" | |
| 23 | + | "github.com/bnema/vev/internal/adapters/sessionwire" | |
| 24 | + | "github.com/bnema/vev/internal/ports" | |
| 25 | + | "github.com/bnema/vev/internal/protocol" | |
| 26 | + | ) | |
| 27 | + | ||
| 28 | + | // fakeKillConnection is a scripted ports.ClientConnection. Sends are recorded | |
| 29 | + | // and counted; ReceiveServer serves queued replies in order, then either a | |
| 30 | + | // reply derived from the last sent Kill (replyForKill), a terminal error, or a | |
| 31 | + | // block until Close. | |
| 32 | + | type fakeKillConnection struct { | |
| 33 | + | mu sync.Mutex | |
| 34 | + | sends []protocol.ClientMessage | |
| 35 | + | recv []protocol.ServerMessage | |
| 36 | + | replyForKill func(protocol.Kill) protocol.ServerMessage | |
| 37 | + | sendErr error | |
| 38 | + | recvErr error | |
| 39 | + | // block: once no reply is available, ReceiveServer parks until Close. | |
| 40 | + | block bool | |
| 41 | + | closed chan struct{} | |
| 42 | + | once sync.Once | |
| 43 | + | // recvEntered reports that a blocked ReceiveServer was actually entered. | |
| 44 | + | recvEntered chan struct{} | |
| 45 | + | enterOnce sync.Once | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | func newFakeKillConnection() *fakeKillConnection { | |
| 49 | + | return &fakeKillConnection{closed: make(chan struct{}), recvEntered: make(chan struct{})} | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | func (c *fakeKillConnection) SendClient(message protocol.ClientMessage) error { | |
| 53 | + | c.mu.Lock() | |
| 54 | + | defer c.mu.Unlock() | |
| 55 | + | if c.sendErr != nil { | |
| 56 | + | return c.sendErr | |
| 57 | + | } | |
| 58 | + | c.sends = append(c.sends, message) | |
| 59 | + | return nil | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | func (c *fakeKillConnection) ReceiveServer() (protocol.ServerMessage, error) { | |
| 63 | + | c.mu.Lock() | |
| 64 | + | if len(c.recv) > 0 { | |
| 65 | + | next := c.recv[0] | |
| 66 | + | c.recv = c.recv[1:] | |
| 67 | + | c.mu.Unlock() | |
| 68 | + | return next, nil | |
| 69 | + | } | |
| 70 | + | // replyForKill derives a matching result from the exact request that was | |
| 71 | + | // sent, so a test never has to predict the fresh random RequestID. | |
| 72 | + | if c.replyForKill != nil && len(c.sends) > 0 { | |
| 73 | + | if kill, ok := c.sends[len(c.sends)-1].(protocol.Kill); ok { | |
| 74 | + | reply := c.replyForKill(kill) | |
| 75 | + | c.mu.Unlock() | |
| 76 | + | return reply, nil | |
| 77 | + | } | |
| 78 | + | } | |
| 79 | + | block, recvErr := c.block, c.recvErr | |
| 80 | + | c.mu.Unlock() | |
| 81 | + | if !block { | |
| 82 | + | return nil, recvErr | |
| 83 | + | } | |
| 84 | + | c.enterOnce.Do(func() { close(c.recvEntered) }) | |
| 85 | + | <-c.closed | |
| 86 | + | return nil, io.ErrClosedPipe | |
| 87 | + | } | |
| 88 | + | ||
| 89 | + | func (c *fakeKillConnection) Capabilities() protocol.ConnectionCapabilities { | |
| 90 | + | return protocol.ConnectionCapabilities{} | |
| 91 | + | } | |
| 92 | + | ||
| 93 | + | func (*fakeKillConnection) LinkState() ports.LinkState { return ports.LinkState(0) } | |
| 94 | + | func (*fakeKillConnection) LinkEvents() <-chan ports.LinkEvent { return nil } | |
| 95 | + | ||
| 96 | + | func (c *fakeKillConnection) Close() error { | |
| 97 | + | c.once.Do(func() { close(c.closed) }) | |
| 98 | + | return nil | |
| 99 | + | } | |
| 100 | + | ||
| 101 | + | func (c *fakeKillConnection) sentCount() int { | |
| 102 | + | c.mu.Lock() | |
| 103 | + | defer c.mu.Unlock() | |
| 104 | + | return len(c.sends) | |
| 105 | + | } | |
| 106 | + | ||
| 107 | + | func (c *fakeKillConnection) sentKill(t *testing.T) protocol.Kill { | |
| 108 | + | t.Helper() | |
| 109 | + | c.mu.Lock() | |
| 110 | + | defer c.mu.Unlock() | |
| 111 | + | require.Len(t, c.sends, 1, "exactly one Kill must be sent (no replay)") | |
| 112 | + | kill, ok := c.sends[0].(protocol.Kill) | |
| 113 | + | require.True(t, ok, "sent message %T is not a Kill", c.sends[0]) | |
| 114 | + | return kill | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | // sendAndAwait drives the shared send/receive pair once, as runKill and | |
| 118 | + | // requestDaemonStop do. | |
| 119 | + | func sendAndAwait(ctx context.Context, conn *fakeKillConnection, request protocol.Kill) error { | |
| 120 | + | requestID, err := sendKillRequest(ctx, conn, request) | |
| 121 | + | if err != nil { | |
| 122 | + | return err | |
| 123 | + | } | |
| 124 | + | return receiveKillResult(ctx, conn, requestID) | |
| 125 | + | } | |
| 126 | + | ||
| 127 | + | // matchingResult mirrors one Kill's RequestID into a successful result. | |
| 128 | + | func matchingResult(outcome protocol.KillOutcome, code uint16, text string) func(protocol.Kill) protocol.ServerMessage { | |
| 129 | + | return func(kill protocol.Kill) protocol.ServerMessage { | |
| 130 | + | return protocol.KillResult{RequestID: kill.RequestID, Outcome: outcome, Code: code, Text: text} | |
| 131 | + | } | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | func TestKillResultMatchingSuccessConsumesOneCorrelatedResult(t *testing.T) { | |
| 135 | + | conn := newFakeKillConnection() | |
| 136 | + | conn.replyForKill = matchingResult(protocol.KillSucceeded, 0, "") | |
| 137 | + | ||
| 138 | + | require.NoError(t, sendAndAwait(context.Background(), conn, protocol.Kill{Name: "work", Scope: protocol.KillSession})) | |
| 139 | + | ||
| 140 | + | kill := conn.sentKill(t) | |
| 141 | + | require.NotZero(t, kill.RequestID, "a Kill must carry a nonzero RequestID") | |
| 142 | + | require.Equal(t, "work", kill.Name) | |
| 143 | + | require.Equal(t, protocol.KillSession, kill.Scope) | |
| 144 | + | require.Equal(t, 1, conn.sentCount()) | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | func TestKillResultFailedDefiniteIsNotUnknown(t *testing.T) { | |
| 148 | + | tests := []struct { | |
| 149 | + | name string | |
| 150 | + | text string | |
| 151 | + | }{ | |
| 152 | + | {name: "no such session", text: "no such session: work"}, | |
| 153 | + | {name: "participants changed", text: "session changed during the operation; try again"}, | |
| 154 | + | } | |
| 155 | + | for _, tt := range tests { | |
| 156 | + | t.Run(tt.name, func(t *testing.T) { | |
| 157 | + | conn := newFakeKillConnection() | |
| 158 | + | conn.replyForKill = matchingResult(protocol.KillFailed, protocol.ErrInternal, tt.text) | |
| 159 | + | ||
| 160 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) | |
| 161 | + | require.Error(t, err) | |
| 162 | + | require.ErrorContains(t, err, tt.text) | |
| 163 | + | require.NotErrorIs(t, err, errKillOutcomeUnknown, "a definite failure must not be reported as unknown") | |
| 164 | + | require.Equal(t, 1, conn.sentCount(), "a definite failure must not be replayed") | |
| 165 | + | }) | |
| 166 | + | } | |
| 167 | + | } | |
| 168 | + | ||
| 169 | + | func TestKillResultUnknownOutcomeStaysUnknown(t *testing.T) { | |
| 170 | + | conn := newFakeKillConnection() | |
| 171 | + | conn.replyForKill = matchingResult(protocol.KillOutcomeUnknown, protocol.ErrServerShutdown, "daemon is shutting down") | |
| 172 | + | ||
| 173 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Scope: protocol.KillAll}) | |
| 174 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |
| 175 | + | require.Equal(t, 1, conn.sentCount(), "an unknown outcome must never be replayed") | |
| 176 | + | } | |
| 177 | + | ||
| 178 | + | func TestKillResultWrongTypeOrIDIsOutcomeUnknown(t *testing.T) { | |
| 179 | + | tests := []struct { | |
| 180 | + | name string | |
| 181 | + | reply protocol.ServerMessage | |
| 182 | + | }{ | |
| 183 | + | {name: "wrong type", reply: protocol.Pong{}}, | |
| 184 | + | {name: "wrong type error", reply: protocol.ErrorMsg{Code: protocol.ErrInternal, Text: "boom"}}, | |
| 185 | + | {name: "wrong id", reply: protocol.KillResult{RequestID: 99, Outcome: protocol.KillSucceeded}}, | |
| 186 | + | {name: "zero id", reply: protocol.KillResult{RequestID: 0, Outcome: protocol.KillSucceeded}}, | |
| 187 | + | } | |
| 188 | + | for _, tt := range tests { | |
| 189 | + | t.Run(tt.name, func(t *testing.T) { | |
| 190 | + | conn := newFakeKillConnection() | |
| 191 | + | conn.recv = []protocol.ServerMessage{tt.reply} | |
| 192 | + | ||
| 193 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Scope: protocol.KillSession}) | |
| 194 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |
| 195 | + | require.Equal(t, 1, conn.sentCount(), "an uncorrelated reply must not trigger a replay") | |
| 196 | + | }) | |
| 197 | + | } | |
| 198 | + | } | |
| 199 | + | ||
| 200 | + | func TestKillResultClosedBeforeReplyIsOutcomeUnknown(t *testing.T) { | |
| 201 | + | conn := newFakeKillConnection() | |
| 202 | + | conn.recvErr = io.EOF | |
| 203 | + | ||
| 204 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Scope: protocol.KillSession}) | |
| 205 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |
| 206 | + | require.Equal(t, 1, conn.sentCount(), "a close before the reply must never be read as success") | |
| 207 | + | } | |
| 208 | + | ||
| 209 | + | // TestKillResultCancelAfterSendIsOutcomeUnknownWithoutReplay proves a caller | |
| 210 | + | // canceled while awaiting the result closes the exact connection, reports an | |
| 211 | + | // unknown outcome (never EOF-as-success), and never resends the request. | |
| 212 | + | func TestKillResultCancelAfterSendIsOutcomeUnknownWithoutReplay(t *testing.T) { | |
| 213 | + | conn := newFakeKillConnection() | |
| 214 | + | conn.block = true | |
| 215 | + | ||
| 216 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 217 | + | defer cancel() | |
| 218 | + | ||
| 219 | + | done := make(chan error, 1) | |
| 220 | + | go func() { done <- sendAndAwait(ctx, conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) }() | |
| 221 | + | ||
| 222 | + | // Wait until the receive is actually parked, then cancel. No sleep: the | |
| 223 | + | // fake signals entry deterministically. | |
| 224 | + | <-conn.recvEntered | |
| 225 | + | cancel() | |
| 226 | + | ||
| 227 | + | require.ErrorIs(t, <-done, errKillOutcomeUnknown) | |
| 228 | + | require.Equal(t, 1, conn.sentCount(), "a canceled kill must never be replayed") | |
| 229 | + | ||
| 230 | + | select { | |
| 231 | + | case <-conn.closed: | |
| 232 | + | default: | |
| 233 | + | t.Fatal("cancellation after send must close the exact connection") | |
| 234 | + | } | |
| 235 | + | } | |
| 236 | + | ||
| 237 | + | // TestKillResultTimeoutAfterSendIsOutcomeUnknownWithoutReplay proves a caller | |
| 238 | + | // whose result deadline expires while awaiting the reply closes the exact | |
| 239 | + | // connection, reports an unknown outcome, and never resends the request. | |
| 240 | + | func TestKillResultTimeoutAfterSendIsOutcomeUnknownWithoutReplay(t *testing.T) { | |
| 241 | + | conn := newFakeKillConnection() | |
| 242 | + | conn.block = true | |
| 243 | + | ||
| 244 | + | ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) | |
| 245 | + | defer cancel() | |
| 246 | + | ||
| 247 | + | done := make(chan error, 1) | |
| 248 | + | go func() { done <- sendAndAwait(ctx, conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) }() | |
| 249 | + | ||
| 250 | + | <-conn.recvEntered | |
| 251 | + | require.ErrorIs(t, <-done, errKillOutcomeUnknown) | |
| 252 | + | require.Equal(t, 1, conn.sentCount(), "a timed-out kill must never be replayed") | |
| 253 | + | ||
| 254 | + | select { | |
| 255 | + | case <-conn.closed: | |
| 256 | + | default: | |
| 257 | + | t.Fatal("a result timeout must close the exact connection") | |
| 258 | + | } | |
| 259 | + | } | |
| 260 | + | ||
| 261 | + | // TestKillResultCancelBeforeSendIsDefiniteNotSent proves a caller canceled | |
| 262 | + | // before the request is sent reports a definite not-sent outcome and puts no | |
| 263 | + | // bytes on the wire. | |
| 264 | + | func TestKillResultCancelBeforeSendIsDefiniteNotSent(t *testing.T) { | |
| 265 | + | conn := newFakeKillConnection() | |
| 266 | + | ||
| 267 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 268 | + | cancel() | |
| 269 | + | ||
| 270 | + | err := sendAndAwait(ctx, conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) | |
| 271 | + | require.ErrorIs(t, err, errKillNotSent) | |
| 272 | + | require.NotErrorIs(t, err, errKillOutcomeUnknown, "a not-sent request has a definite outcome") | |
| 273 | + | require.Zero(t, conn.sentCount(), "a canceled-before-send kill must not be sent") | |
| 274 | + | } | |
| 275 | + | ||
| 276 | + | func TestKillRequestSendFailureIsReported(t *testing.T) { | |
| 277 | + | conn := newFakeKillConnection() | |
| 278 | + | conn.sendErr = errors.New("write failed") | |
| 279 | + | ||
| 280 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) | |
| 281 | + | require.ErrorContains(t, err, "write failed") | |
| 282 | + | require.Zero(t, conn.sentCount()) | |
| 283 | + | } | |
| 284 | + | ||
| 285 | + | // TestKillRequestIDsAreUniqueAndNonzero proves each request allocates a fresh, | |
| 286 | + | // nonzero correlation ID so a reply for one request can never match another. | |
| 287 | + | func TestKillRequestIDsAreUniqueAndNonzero(t *testing.T) { | |
| 288 | + | seen := make(map[uint64]struct{}) | |
| 289 | + | for range 64 { | |
| 290 | + | conn := newFakeKillConnection() | |
| 291 | + | conn.replyForKill = matchingResult(protocol.KillSucceeded, 0, "") | |
| 292 | + | require.NoError(t, sendAndAwait(context.Background(), conn, protocol.Kill{Scope: protocol.KillAll})) | |
| 293 | + | id := conn.sentKill(t).RequestID | |
| 294 | + | require.NotZero(t, id) | |
| 295 | + | _, duplicate := seen[id] | |
| 296 | + | require.False(t, duplicate, "duplicate RequestID %d", id) | |
| 297 | + | seen[id] = struct{}{} | |
| 298 | + | } | |
| 299 | + | } | |
| 300 | + | ||
| 301 | + | // countingLifecycleProbe records every lifecycle-acquire attempt and returns | |
| 302 | + | // the configured owner or error, so a test can prove both whether the transfer | |
| 303 | + | // wait was entered and how it resolved. | |
| 304 | + | type countingLifecycleProbe struct { | |
| 305 | + | attempts atomic.Int64 | |
| 306 | + | owner lifecycleOwnership | |
| 307 | + | err error | |
| 308 | + | } | |
| 309 | + | ||
| 310 | + | func (p *countingLifecycleProbe) TryAcquire(string) (lifecycleOwnership, error) { | |
| 311 | + | p.attempts.Add(1) | |
| 312 | + | return p.owner, p.err | |
| 313 | + | } | |
| 314 | + | ||
| 315 | + | // startServingOneKill binds ipc.SocketDir() under a temp runtime root, points | |
| 316 | + | // the lifecycle probe at a counting fake, and serves exactly one control | |
| 317 | + | // connection in the background. reply==nil closes the connection without | |
| 318 | + | // answering, which models a lost reply; otherwise the reply is correlated to the | |
| 319 | + | // exact request read from the wire. The returned channel closes once the | |
| 320 | + | // scripted server has finished. | |
| 321 | + | func startServingOneKill(t *testing.T, probe *countingLifecycleProbe, reply func(protocol.Kill) protocol.ServerMessage) <-chan struct{} { | |
| 322 | + | t.Helper() | |
| 323 | + | t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) | |
| 324 | + | t.Setenv("XDG_STATE_HOME", t.TempDir()) | |
| 325 | + | ||
| 326 | + | // Bound the transfer wait so a regression that wrongly waits fails fast | |
| 327 | + | // instead of burning the production retry budget. | |
| 328 | + | originalBackoff := defaultBackoff | |
| 329 | + | t.Cleanup(func() { defaultBackoff = originalBackoff }) | |
| 330 | + | defaultBackoff = backoffConfig{initial: time.Millisecond, max: time.Millisecond, total: 5 * time.Millisecond} | |
| 331 | + | ||
| 332 | + | originalProbe := daemonLifecycleProbe | |
| 333 | + | t.Cleanup(func() { daemonLifecycleProbe = originalProbe }) | |
| 334 | + | daemonLifecycleProbe = probe | |
| 335 | + | ||
| 336 | + | listener, err := ipc.Listen(ipc.SocketDir()) | |
| 337 | + | require.NoError(t, err) | |
| 338 | + | t.Cleanup(func() { _ = listener.Close() }) | |
| 339 | + | ||
| 340 | + | done := make(chan struct{}) | |
| 341 | + | go func() { | |
| 342 | + | defer close(done) | |
| 343 | + | raw, err := listener.Accept() | |
| 344 | + | if err != nil { | |
| 345 | + | return | |
| 346 | + | } | |
| 347 | + | defer func() { _ = raw.Close() }() | |
| 348 | + | connection := sessionwire.NewServerConnection(raw) | |
| 349 | + | message, err := connection.ReceiveClient() | |
| 350 | + | if err != nil { | |
| 351 | + | return | |
| 352 | + | } | |
| 353 | + | kill, ok := message.(protocol.Kill) | |
| 354 | + | if !ok || reply == nil { | |
| 355 | + | return | |
| 356 | + | } | |
| 357 | + | _ = connection.SendServer(reply(kill)) | |
| 358 | + | }() | |
| 359 | + | return done | |
| 360 | + | } | |
| 361 | + | ||
| 362 | + | func awaitServingOneKill(t *testing.T, done <-chan struct{}) { | |
| 363 | + | t.Helper() | |
| 364 | + | select { | |
| 365 | + | case <-done: | |
| 366 | + | case <-time.After(2 * time.Second): | |
| 367 | + | t.Fatal("the scripted control server did not finish") | |
| 368 | + | } | |
| 369 | + | } | |
| 370 | + | ||
| 371 | + | // TestDaemonStopLostReplyConfirmedByLifecycleTransfer proves a daemon-stop whose | |
| 372 | + | // explicit result is lost is still reported as success once the caller directly | |
| 373 | + | // acquires lifecycle ownership: the authoritative transfer confirms the accepted | |
| 374 | + | // shutdown completed. | |
| 375 | + | func TestDaemonStopLostReplyConfirmedByLifecycleTransfer(t *testing.T) { | |
| 376 | + | probe := &countingLifecycleProbe{owner: fakeLifecycleOwnership{release: func() error { return nil }}} | |
| 377 | + | done := startServingOneKill(t, probe, nil) | |
| 378 | + | ||
| 379 | + | require.NoError(t, requestDaemonStop(context.Background())) | |
| 380 | + | awaitServingOneKill(t, done) | |
| 381 | + | require.EqualValues(t, 1, probe.attempts.Load(), "a lost reply must be resolved by exactly one ownership probe") | |
| 382 | + | } | |
| 383 | + | ||
| 384 | + | // TestDaemonStopLostReplyAndTransferFailurePreservesUnknownOutcome proves a | |
| 385 | + | // daemon-stop whose result is lost AND whose lifecycle confirmation fails keeps | |
| 386 | + | // errors.Is(err, errKillOutcomeUnknown) and carries both diagnostics, so the | |
| 387 | + | // caller never reads a lost reply as a definite success or a definite failure. | |
| 388 | + | func TestDaemonStopLostReplyAndTransferFailurePreservesUnknownOutcome(t *testing.T) { | |
| 389 | + | probe := &countingLifecycleProbe{err: errors.New("lifecycle still busy")} | |
| 390 | + | done := startServingOneKill(t, probe, nil) | |
| 391 | + | ||
| 392 | + | err := requestDaemonStop(context.Background()) | |
| 393 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |
| 394 | + | require.ErrorContains(t, err, "awaiting explicit result", "the lost-reply diagnostic must survive") | |
| 395 | + | require.ErrorContains(t, err, "lifecycle confirmation also failed") | |
| 396 | + | awaitServingOneKill(t, done) | |
| 397 | + | } | |
| 398 | + | ||
| 399 | + | // TestDaemonStopDefiniteFailureReturnsWithoutLifecycleWait proves a definite | |
| 400 | + | // KillFailed daemon-stop reply is returned immediately and never enters the | |
| 401 | + | // lifecycle-transfer wait, so a rejected stop cannot be retried as if the | |
| 402 | + | // outcome were still unknown. | |
| 403 | + | func TestDaemonStopDefiniteFailureReturnsWithoutLifecycleWait(t *testing.T) { | |
| 404 | + | probe := &countingLifecycleProbe{err: errors.New("lifecycle must not be probed")} | |
| 405 | + | done := startServingOneKill(t, probe, func(kill protocol.Kill) protocol.ServerMessage { | |
| 406 | + | return protocol.KillResult{RequestID: kill.RequestID, Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "daemon refused stop"} | |
| 407 | + | }) | |
| 408 | + | ||
| 409 | + | err := requestDaemonStop(context.Background()) | |
| 410 | + | require.Error(t, err) | |
| 411 | + | require.ErrorContains(t, err, "daemon refused stop") | |
| 412 | + | require.NotErrorIs(t, err, errKillOutcomeUnknown, "a definite failure must not be reported as unknown") | |
| 413 | + | require.Zero(t, probe.attempts.Load(), "a definite failure must return without waiting for ownership transfer") | |
| 414 | + | awaitServingOneKill(t, done) | |
| 415 | + | } | |
| 416 | + | ||
| 417 | + | // TestDaemonStopUnknownResultKeepsUnknownWhenOwnershipReleaseFails pins the | |
| 418 | + | // ownership-release branch: even when the probe grants ownership, a release | |
| 419 | + | // failure keeps the lost-reply diagnostic instead of discarding it. | |
| 420 | + | func TestDaemonStopUnknownResultKeepsUnknownWhenOwnershipReleaseFails(t *testing.T) { | |
| 421 | + | probe := &countingLifecycleProbe{owner: fakeLifecycleOwnership{release: func() error { return errors.New("unlock failed") }}} | |
| 422 | + | done := startServingOneKill(t, probe, nil) | |
| 423 | + | ||
| 424 | + | err := requestDaemonStop(context.Background()) | |
| 425 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |
| 426 | + | require.ErrorContains(t, err, "confirmed ownership release failed") | |
| 427 | + | require.ErrorContains(t, err, "unlock failed") | |
| 428 | + | awaitServingOneKill(t, done) | |
| 429 | + | } | |
| @@ -0,0 +1,429 @@ | |||||
| 1 | + | package app | |||
| 2 | + | ||||
| 3 | + | // KillResult caller contract (explicit result slice). runKill and | |||
| 4 | + | // requestDaemonStop send one Kill with a unique nonzero RequestID and consume | |||
| 5 | + | // exactly one matching KillResult; a lost, uncorrelated, or wrong-typed reply is | |||
| 6 | + | // outcome-unknown and is never replayed, while a caller canceled before the | |||
| 7 | + | // send is a definite not-sent outcome. These tests drive the two helpers those | |||
| 8 | + | // callers share with a controllable in-memory client connection, so every case | |||
| 9 | + | // is bounded and uses no sleeps. | |||
| 10 | + | ||||
| 11 | + | import ( | |||
| 12 | + | "context" | |||
| 13 | + | "errors" | |||
| 14 | + | "io" | |||
| 15 | + | "sync" | |||
| 16 | + | "sync/atomic" | |||
| 17 | + | "testing" | |||
| 18 | + | "time" | |||
| 19 | + | ||||
| 20 | + | "github.com/stretchr/testify/require" | |||
| 21 | + | ||||
| 22 | + | "github.com/bnema/vev/internal/adapters/ipc" | |||
| 23 | + | "github.com/bnema/vev/internal/adapters/sessionwire" | |||
| 24 | + | "github.com/bnema/vev/internal/ports" | |||
| 25 | + | "github.com/bnema/vev/internal/protocol" | |||
| 26 | + | ) | |||
| 27 | + | ||||
| 28 | + | // fakeKillConnection is a scripted ports.ClientConnection. Sends are recorded | |||
| 29 | + | // and counted; ReceiveServer serves queued replies in order, then either a | |||
| 30 | + | // reply derived from the last sent Kill (replyForKill), a terminal error, or a | |||
| 31 | + | // block until Close. | |||
| 32 | + | type fakeKillConnection struct { | |||
| 33 | + | mu sync.Mutex | |||
| 34 | + | sends []protocol.ClientMessage | |||
| 35 | + | recv []protocol.ServerMessage | |||
| 36 | + | replyForKill func(protocol.Kill) protocol.ServerMessage | |||
| 37 | + | sendErr error | |||
| 38 | + | recvErr error | |||
| 39 | + | // block: once no reply is available, ReceiveServer parks until Close. | |||
| 40 | + | block bool | |||
| 41 | + | closed chan struct{} | |||
| 42 | + | once sync.Once | |||
| 43 | + | // recvEntered reports that a blocked ReceiveServer was actually entered. | |||
| 44 | + | recvEntered chan struct{} | |||
| 45 | + | enterOnce sync.Once | |||
| 46 | + | } | |||
| 47 | + | ||||
| 48 | + | func newFakeKillConnection() *fakeKillConnection { | |||
| 49 | + | return &fakeKillConnection{closed: make(chan struct{}), recvEntered: make(chan struct{})} | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | func (c *fakeKillConnection) SendClient(message protocol.ClientMessage) error { | |||
| 53 | + | c.mu.Lock() | |||
| 54 | + | defer c.mu.Unlock() | |||
| 55 | + | if c.sendErr != nil { | |||
| 56 | + | return c.sendErr | |||
| 57 | + | } | |||
| 58 | + | c.sends = append(c.sends, message) | |||
| 59 | + | return nil | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | func (c *fakeKillConnection) ReceiveServer() (protocol.ServerMessage, error) { | |||
| 63 | + | c.mu.Lock() | |||
| 64 | + | if len(c.recv) > 0 { | |||
| 65 | + | next := c.recv[0] | |||
| 66 | + | c.recv = c.recv[1:] | |||
| 67 | + | c.mu.Unlock() | |||
| 68 | + | return next, nil | |||
| 69 | + | } | |||
| 70 | + | // replyForKill derives a matching result from the exact request that was | |||
| 71 | + | // sent, so a test never has to predict the fresh random RequestID. | |||
| 72 | + | if c.replyForKill != nil && len(c.sends) > 0 { | |||
| 73 | + | if kill, ok := c.sends[len(c.sends)-1].(protocol.Kill); ok { | |||
| 74 | + | reply := c.replyForKill(kill) | |||
| 75 | + | c.mu.Unlock() | |||
| 76 | + | return reply, nil | |||
| 77 | + | } | |||
| 78 | + | } | |||
| 79 | + | block, recvErr := c.block, c.recvErr | |||
| 80 | + | c.mu.Unlock() | |||
| 81 | + | if !block { | |||
| 82 | + | return nil, recvErr | |||
| 83 | + | } | |||
| 84 | + | c.enterOnce.Do(func() { close(c.recvEntered) }) | |||
| 85 | + | <-c.closed | |||
| 86 | + | return nil, io.ErrClosedPipe | |||
| 87 | + | } | |||
| 88 | + | ||||
| 89 | + | func (c *fakeKillConnection) Capabilities() protocol.ConnectionCapabilities { | |||
| 90 | + | return protocol.ConnectionCapabilities{} | |||
| 91 | + | } | |||
| 92 | + | ||||
| 93 | + | func (*fakeKillConnection) LinkState() ports.LinkState { return ports.LinkState(0) } | |||
| 94 | + | func (*fakeKillConnection) LinkEvents() <-chan ports.LinkEvent { return nil } | |||
| 95 | + | ||||
| 96 | + | func (c *fakeKillConnection) Close() error { | |||
| 97 | + | c.once.Do(func() { close(c.closed) }) | |||
| 98 | + | return nil | |||
| 99 | + | } | |||
| 100 | + | ||||
| 101 | + | func (c *fakeKillConnection) sentCount() int { | |||
| 102 | + | c.mu.Lock() | |||
| 103 | + | defer c.mu.Unlock() | |||
| 104 | + | return len(c.sends) | |||
| 105 | + | } | |||
| 106 | + | ||||
| 107 | + | func (c *fakeKillConnection) sentKill(t *testing.T) protocol.Kill { | |||
| 108 | + | t.Helper() | |||
| 109 | + | c.mu.Lock() | |||
| 110 | + | defer c.mu.Unlock() | |||
| 111 | + | require.Len(t, c.sends, 1, "exactly one Kill must be sent (no replay)") | |||
| 112 | + | kill, ok := c.sends[0].(protocol.Kill) | |||
| 113 | + | require.True(t, ok, "sent message %T is not a Kill", c.sends[0]) | |||
| 114 | + | return kill | |||
| 115 | + | } | |||
| 116 | + | ||||
| 117 | + | // sendAndAwait drives the shared send/receive pair once, as runKill and | |||
| 118 | + | // requestDaemonStop do. | |||
| 119 | + | func sendAndAwait(ctx context.Context, conn *fakeKillConnection, request protocol.Kill) error { | |||
| 120 | + | requestID, err := sendKillRequest(ctx, conn, request) | |||
| 121 | + | if err != nil { | |||
| 122 | + | return err | |||
| 123 | + | } | |||
| 124 | + | return receiveKillResult(ctx, conn, requestID) | |||
| 125 | + | } | |||
| 126 | + | ||||
| 127 | + | // matchingResult mirrors one Kill's RequestID into a successful result. | |||
| 128 | + | func matchingResult(outcome protocol.KillOutcome, code uint16, text string) func(protocol.Kill) protocol.ServerMessage { | |||
| 129 | + | return func(kill protocol.Kill) protocol.ServerMessage { | |||
| 130 | + | return protocol.KillResult{RequestID: kill.RequestID, Outcome: outcome, Code: code, Text: text} | |||
| 131 | + | } | |||
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | func TestKillResultMatchingSuccessConsumesOneCorrelatedResult(t *testing.T) { | |||
| 135 | + | conn := newFakeKillConnection() | |||
| 136 | + | conn.replyForKill = matchingResult(protocol.KillSucceeded, 0, "") | |||
| 137 | + | ||||
| 138 | + | require.NoError(t, sendAndAwait(context.Background(), conn, protocol.Kill{Name: "work", Scope: protocol.KillSession})) | |||
| 139 | + | ||||
| 140 | + | kill := conn.sentKill(t) | |||
| 141 | + | require.NotZero(t, kill.RequestID, "a Kill must carry a nonzero RequestID") | |||
| 142 | + | require.Equal(t, "work", kill.Name) | |||
| 143 | + | require.Equal(t, protocol.KillSession, kill.Scope) | |||
| 144 | + | require.Equal(t, 1, conn.sentCount()) | |||
| 145 | + | } | |||
| 146 | + | ||||
| 147 | + | func TestKillResultFailedDefiniteIsNotUnknown(t *testing.T) { | |||
| 148 | + | tests := []struct { | |||
| 149 | + | name string | |||
| 150 | + | text string | |||
| 151 | + | }{ | |||
| 152 | + | {name: "no such session", text: "no such session: work"}, | |||
| 153 | + | {name: "participants changed", text: "session changed during the operation; try again"}, | |||
| 154 | + | } | |||
| 155 | + | for _, tt := range tests { | |||
| 156 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 157 | + | conn := newFakeKillConnection() | |||
| 158 | + | conn.replyForKill = matchingResult(protocol.KillFailed, protocol.ErrInternal, tt.text) | |||
| 159 | + | ||||
| 160 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) | |||
| 161 | + | require.Error(t, err) | |||
| 162 | + | require.ErrorContains(t, err, tt.text) | |||
| 163 | + | require.NotErrorIs(t, err, errKillOutcomeUnknown, "a definite failure must not be reported as unknown") | |||
| 164 | + | require.Equal(t, 1, conn.sentCount(), "a definite failure must not be replayed") | |||
| 165 | + | }) | |||
| 166 | + | } | |||
| 167 | + | } | |||
| 168 | + | ||||
| 169 | + | func TestKillResultUnknownOutcomeStaysUnknown(t *testing.T) { | |||
| 170 | + | conn := newFakeKillConnection() | |||
| 171 | + | conn.replyForKill = matchingResult(protocol.KillOutcomeUnknown, protocol.ErrServerShutdown, "daemon is shutting down") | |||
| 172 | + | ||||
| 173 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Scope: protocol.KillAll}) | |||
| 174 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |||
| 175 | + | require.Equal(t, 1, conn.sentCount(), "an unknown outcome must never be replayed") | |||
| 176 | + | } | |||
| 177 | + | ||||
| 178 | + | func TestKillResultWrongTypeOrIDIsOutcomeUnknown(t *testing.T) { | |||
| 179 | + | tests := []struct { | |||
| 180 | + | name string | |||
| 181 | + | reply protocol.ServerMessage | |||
| 182 | + | }{ | |||
| 183 | + | {name: "wrong type", reply: protocol.Pong{}}, | |||
| 184 | + | {name: "wrong type error", reply: protocol.ErrorMsg{Code: protocol.ErrInternal, Text: "boom"}}, | |||
| 185 | + | {name: "wrong id", reply: protocol.KillResult{RequestID: 99, Outcome: protocol.KillSucceeded}}, | |||
| 186 | + | {name: "zero id", reply: protocol.KillResult{RequestID: 0, Outcome: protocol.KillSucceeded}}, | |||
| 187 | + | } | |||
| 188 | + | for _, tt := range tests { | |||
| 189 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 190 | + | conn := newFakeKillConnection() | |||
| 191 | + | conn.recv = []protocol.ServerMessage{tt.reply} | |||
| 192 | + | ||||
| 193 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Scope: protocol.KillSession}) | |||
| 194 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |||
| 195 | + | require.Equal(t, 1, conn.sentCount(), "an uncorrelated reply must not trigger a replay") | |||
| 196 | + | }) | |||
| 197 | + | } | |||
| 198 | + | } | |||
| 199 | + | ||||
| 200 | + | func TestKillResultClosedBeforeReplyIsOutcomeUnknown(t *testing.T) { | |||
| 201 | + | conn := newFakeKillConnection() | |||
| 202 | + | conn.recvErr = io.EOF | |||
| 203 | + | ||||
| 204 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Scope: protocol.KillSession}) | |||
| 205 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |||
| 206 | + | require.Equal(t, 1, conn.sentCount(), "a close before the reply must never be read as success") | |||
| 207 | + | } | |||
| 208 | + | ||||
| 209 | + | // TestKillResultCancelAfterSendIsOutcomeUnknownWithoutReplay proves a caller | |||
| 210 | + | // canceled while awaiting the result closes the exact connection, reports an | |||
| 211 | + | // unknown outcome (never EOF-as-success), and never resends the request. | |||
| 212 | + | func TestKillResultCancelAfterSendIsOutcomeUnknownWithoutReplay(t *testing.T) { | |||
| 213 | + | conn := newFakeKillConnection() | |||
| 214 | + | conn.block = true | |||
| 215 | + | ||||
| 216 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 217 | + | defer cancel() | |||
| 218 | + | ||||
| 219 | + | done := make(chan error, 1) | |||
| 220 | + | go func() { done <- sendAndAwait(ctx, conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) }() | |||
| 221 | + | ||||
| 222 | + | // Wait until the receive is actually parked, then cancel. No sleep: the | |||
| 223 | + | // fake signals entry deterministically. | |||
| 224 | + | <-conn.recvEntered | |||
| 225 | + | cancel() | |||
| 226 | + | ||||
| 227 | + | require.ErrorIs(t, <-done, errKillOutcomeUnknown) | |||
| 228 | + | require.Equal(t, 1, conn.sentCount(), "a canceled kill must never be replayed") | |||
| 229 | + | ||||
| 230 | + | select { | |||
| 231 | + | case <-conn.closed: | |||
| 232 | + | default: | |||
| 233 | + | t.Fatal("cancellation after send must close the exact connection") | |||
| 234 | + | } | |||
| 235 | + | } | |||
| 236 | + | ||||
| 237 | + | // TestKillResultTimeoutAfterSendIsOutcomeUnknownWithoutReplay proves a caller | |||
| 238 | + | // whose result deadline expires while awaiting the reply closes the exact | |||
| 239 | + | // connection, reports an unknown outcome, and never resends the request. | |||
| 240 | + | func TestKillResultTimeoutAfterSendIsOutcomeUnknownWithoutReplay(t *testing.T) { | |||
| 241 | + | conn := newFakeKillConnection() | |||
| 242 | + | conn.block = true | |||
| 243 | + | ||||
| 244 | + | ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) | |||
| 245 | + | defer cancel() | |||
| 246 | + | ||||
| 247 | + | done := make(chan error, 1) | |||
| 248 | + | go func() { done <- sendAndAwait(ctx, conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) }() | |||
| 249 | + | ||||
| 250 | + | <-conn.recvEntered | |||
| 251 | + | require.ErrorIs(t, <-done, errKillOutcomeUnknown) | |||
| 252 | + | require.Equal(t, 1, conn.sentCount(), "a timed-out kill must never be replayed") | |||
| 253 | + | ||||
| 254 | + | select { | |||
| 255 | + | case <-conn.closed: | |||
| 256 | + | default: | |||
| 257 | + | t.Fatal("a result timeout must close the exact connection") | |||
| 258 | + | } | |||
| 259 | + | } | |||
| 260 | + | ||||
| 261 | + | // TestKillResultCancelBeforeSendIsDefiniteNotSent proves a caller canceled | |||
| 262 | + | // before the request is sent reports a definite not-sent outcome and puts no | |||
| 263 | + | // bytes on the wire. | |||
| 264 | + | func TestKillResultCancelBeforeSendIsDefiniteNotSent(t *testing.T) { | |||
| 265 | + | conn := newFakeKillConnection() | |||
| 266 | + | ||||
| 267 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 268 | + | cancel() | |||
| 269 | + | ||||
| 270 | + | err := sendAndAwait(ctx, conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) | |||
| 271 | + | require.ErrorIs(t, err, errKillNotSent) | |||
| 272 | + | require.NotErrorIs(t, err, errKillOutcomeUnknown, "a not-sent request has a definite outcome") | |||
| 273 | + | require.Zero(t, conn.sentCount(), "a canceled-before-send kill must not be sent") | |||
| 274 | + | } | |||
| 275 | + | ||||
| 276 | + | func TestKillRequestSendFailureIsReported(t *testing.T) { | |||
| 277 | + | conn := newFakeKillConnection() | |||
| 278 | + | conn.sendErr = errors.New("write failed") | |||
| 279 | + | ||||
| 280 | + | err := sendAndAwait(context.Background(), conn, protocol.Kill{Name: "work", Scope: protocol.KillSession}) | |||
| 281 | + | require.ErrorContains(t, err, "write failed") | |||
| 282 | + | require.Zero(t, conn.sentCount()) | |||
| 283 | + | } | |||
| 284 | + | ||||
| 285 | + | // TestKillRequestIDsAreUniqueAndNonzero proves each request allocates a fresh, | |||
| 286 | + | // nonzero correlation ID so a reply for one request can never match another. | |||
| 287 | + | func TestKillRequestIDsAreUniqueAndNonzero(t *testing.T) { | |||
| 288 | + | seen := make(map[uint64]struct{}) | |||
| 289 | + | for range 64 { | |||
| 290 | + | conn := newFakeKillConnection() | |||
| 291 | + | conn.replyForKill = matchingResult(protocol.KillSucceeded, 0, "") | |||
| 292 | + | require.NoError(t, sendAndAwait(context.Background(), conn, protocol.Kill{Scope: protocol.KillAll})) | |||
| 293 | + | id := conn.sentKill(t).RequestID | |||
| 294 | + | require.NotZero(t, id) | |||
| 295 | + | _, duplicate := seen[id] | |||
| 296 | + | require.False(t, duplicate, "duplicate RequestID %d", id) | |||
| 297 | + | seen[id] = struct{}{} | |||
| 298 | + | } | |||
| 299 | + | } | |||
| 300 | + | ||||
| 301 | + | // countingLifecycleProbe records every lifecycle-acquire attempt and returns | |||
| 302 | + | // the configured owner or error, so a test can prove both whether the transfer | |||
| 303 | + | // wait was entered and how it resolved. | |||
| 304 | + | type countingLifecycleProbe struct { | |||
| 305 | + | attempts atomic.Int64 | |||
| 306 | + | owner lifecycleOwnership | |||
| 307 | + | err error | |||
| 308 | + | } | |||
| 309 | + | ||||
| 310 | + | func (p *countingLifecycleProbe) TryAcquire(string) (lifecycleOwnership, error) { | |||
| 311 | + | p.attempts.Add(1) | |||
| 312 | + | return p.owner, p.err | |||
| 313 | + | } | |||
| 314 | + | ||||
| 315 | + | // startServingOneKill binds ipc.SocketDir() under a temp runtime root, points | |||
| 316 | + | // the lifecycle probe at a counting fake, and serves exactly one control | |||
| 317 | + | // connection in the background. reply==nil closes the connection without | |||
| 318 | + | // answering, which models a lost reply; otherwise the reply is correlated to the | |||
| 319 | + | // exact request read from the wire. The returned channel closes once the | |||
| 320 | + | // scripted server has finished. | |||
| 321 | + | func startServingOneKill(t *testing.T, probe *countingLifecycleProbe, reply func(protocol.Kill) protocol.ServerMessage) <-chan struct{} { | |||
| 322 | + | t.Helper() | |||
| 323 | + | t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) | |||
| 324 | + | t.Setenv("XDG_STATE_HOME", t.TempDir()) | |||
| 325 | + | ||||
| 326 | + | // Bound the transfer wait so a regression that wrongly waits fails fast | |||
| 327 | + | // instead of burning the production retry budget. | |||
| 328 | + | originalBackoff := defaultBackoff | |||
| 329 | + | t.Cleanup(func() { defaultBackoff = originalBackoff }) | |||
| 330 | + | defaultBackoff = backoffConfig{initial: time.Millisecond, max: time.Millisecond, total: 5 * time.Millisecond} | |||
| 331 | + | ||||
| 332 | + | originalProbe := daemonLifecycleProbe | |||
| 333 | + | t.Cleanup(func() { daemonLifecycleProbe = originalProbe }) | |||
| 334 | + | daemonLifecycleProbe = probe | |||
| 335 | + | ||||
| 336 | + | listener, err := ipc.Listen(ipc.SocketDir()) | |||
| 337 | + | require.NoError(t, err) | |||
| 338 | + | t.Cleanup(func() { _ = listener.Close() }) | |||
| 339 | + | ||||
| 340 | + | done := make(chan struct{}) | |||
| 341 | + | go func() { | |||
| 342 | + | defer close(done) | |||
| 343 | + | raw, err := listener.Accept() | |||
| 344 | + | if err != nil { | |||
| 345 | + | return | |||
| 346 | + | } | |||
| 347 | + | defer func() { _ = raw.Close() }() | |||
| 348 | + | connection := sessionwire.NewServerConnection(raw) | |||
| 349 | + | message, err := connection.ReceiveClient() | |||
| 350 | + | if err != nil { | |||
| 351 | + | return | |||
| 352 | + | } | |||
| 353 | + | kill, ok := message.(protocol.Kill) | |||
| 354 | + | if !ok || reply == nil { | |||
| 355 | + | return | |||
| 356 | + | } | |||
| 357 | + | _ = connection.SendServer(reply(kill)) | |||
| 358 | + | }() | |||
| 359 | + | return done | |||
| 360 | + | } | |||
| 361 | + | ||||
| 362 | + | func awaitServingOneKill(t *testing.T, done <-chan struct{}) { | |||
| 363 | + | t.Helper() | |||
| 364 | + | select { | |||
| 365 | + | case <-done: | |||
| 366 | + | case <-time.After(2 * time.Second): | |||
| 367 | + | t.Fatal("the scripted control server did not finish") | |||
| 368 | + | } | |||
| 369 | + | } | |||
| 370 | + | ||||
| 371 | + | // TestDaemonStopLostReplyConfirmedByLifecycleTransfer proves a daemon-stop whose | |||
| 372 | + | // explicit result is lost is still reported as success once the caller directly | |||
| 373 | + | // acquires lifecycle ownership: the authoritative transfer confirms the accepted | |||
| 374 | + | // shutdown completed. | |||
| 375 | + | func TestDaemonStopLostReplyConfirmedByLifecycleTransfer(t *testing.T) { | |||
| 376 | + | probe := &countingLifecycleProbe{owner: fakeLifecycleOwnership{release: func() error { return nil }}} | |||
| 377 | + | done := startServingOneKill(t, probe, nil) | |||
| 378 | + | ||||
| 379 | + | require.NoError(t, requestDaemonStop(context.Background())) | |||
| 380 | + | awaitServingOneKill(t, done) | |||
| 381 | + | require.EqualValues(t, 1, probe.attempts.Load(), "a lost reply must be resolved by exactly one ownership probe") | |||
| 382 | + | } | |||
| 383 | + | ||||
| 384 | + | // TestDaemonStopLostReplyAndTransferFailurePreservesUnknownOutcome proves a | |||
| 385 | + | // daemon-stop whose result is lost AND whose lifecycle confirmation fails keeps | |||
| 386 | + | // errors.Is(err, errKillOutcomeUnknown) and carries both diagnostics, so the | |||
| 387 | + | // caller never reads a lost reply as a definite success or a definite failure. | |||
| 388 | + | func TestDaemonStopLostReplyAndTransferFailurePreservesUnknownOutcome(t *testing.T) { | |||
| 389 | + | probe := &countingLifecycleProbe{err: errors.New("lifecycle still busy")} | |||
| 390 | + | done := startServingOneKill(t, probe, nil) | |||
| 391 | + | ||||
| 392 | + | err := requestDaemonStop(context.Background()) | |||
| 393 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |||
| 394 | + | require.ErrorContains(t, err, "awaiting explicit result", "the lost-reply diagnostic must survive") | |||
| 395 | + | require.ErrorContains(t, err, "lifecycle confirmation also failed") | |||
| 396 | + | awaitServingOneKill(t, done) | |||
| 397 | + | } | |||
| 398 | + | ||||
| 399 | + | // TestDaemonStopDefiniteFailureReturnsWithoutLifecycleWait proves a definite | |||
| 400 | + | // KillFailed daemon-stop reply is returned immediately and never enters the | |||
| 401 | + | // lifecycle-transfer wait, so a rejected stop cannot be retried as if the | |||
| 402 | + | // outcome were still unknown. | |||
| 403 | + | func TestDaemonStopDefiniteFailureReturnsWithoutLifecycleWait(t *testing.T) { | |||
| 404 | + | probe := &countingLifecycleProbe{err: errors.New("lifecycle must not be probed")} | |||
| 405 | + | done := startServingOneKill(t, probe, func(kill protocol.Kill) protocol.ServerMessage { | |||
| 406 | + | return protocol.KillResult{RequestID: kill.RequestID, Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "daemon refused stop"} | |||
| 407 | + | }) | |||
| 408 | + | ||||
| 409 | + | err := requestDaemonStop(context.Background()) | |||
| 410 | + | require.Error(t, err) | |||
| 411 | + | require.ErrorContains(t, err, "daemon refused stop") | |||
| 412 | + | require.NotErrorIs(t, err, errKillOutcomeUnknown, "a definite failure must not be reported as unknown") | |||
| 413 | + | require.Zero(t, probe.attempts.Load(), "a definite failure must return without waiting for ownership transfer") | |||
| 414 | + | awaitServingOneKill(t, done) | |||
| 415 | + | } | |||
| 416 | + | ||||
| 417 | + | // TestDaemonStopUnknownResultKeepsUnknownWhenOwnershipReleaseFails pins the | |||
| 418 | + | // ownership-release branch: even when the probe grants ownership, a release | |||
| 419 | + | // failure keeps the lost-reply diagnostic instead of discarding it. | |||
| 420 | + | func TestDaemonStopUnknownResultKeepsUnknownWhenOwnershipReleaseFails(t *testing.T) { | |||
| 421 | + | probe := &countingLifecycleProbe{owner: fakeLifecycleOwnership{release: func() error { return errors.New("unlock failed") }}} | |||
| 422 | + | done := startServingOneKill(t, probe, nil) | |||
| 423 | + | ||||
| 424 | + | err := requestDaemonStop(context.Background()) | |||
| 425 | + | require.ErrorIs(t, err, errKillOutcomeUnknown) | |||
| 426 | + | require.ErrorContains(t, err, "confirmed ownership release failed") | |||
| 427 | + | require.ErrorContains(t, err, "unlock failed") | |||
| 428 | + | awaitServingOneKill(t, done) | |||
| 429 | + | } | |||
M internal/app/run.go +97 -27
| @@ -1178,7 +1178,81 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac | |||
| 1178 | 1178 | ||
| 1179 | 1179 | const daemonStopTimeout = 2 * time.Second | |
| 1180 | 1180 | ||
| 1181 | - | var errDaemonNotRunning = errors.New("vev: no daemon running") | |
| 1181 | + | var ( | |
| 1182 | + | errDaemonNotRunning = errors.New("vev: no daemon running") | |
| 1183 | + | errKillOutcomeUnknown = errors.New("vev: kill outcome unknown") | |
| 1184 | + | // errKillNotSent reports a Kill that was never placed on the wire because | |
| 1185 | + | // its caller was already canceled. The outcome is definite: the daemon | |
| 1186 | + | // cannot have acted, so it must not be reported as an unknown outcome. | |
| 1187 | + | errKillNotSent = errors.New("vev: kill request not sent") | |
| 1188 | + | ) | |
| 1189 | + | ||
| 1190 | + | func newControlRequestID() (uint64, error) { | |
| 1191 | + | var raw [8]byte | |
| 1192 | + | if _, err := rand.Read(raw[:]); err != nil { | |
| 1193 | + | return 0, err | |
| 1194 | + | } | |
| 1195 | + | id := uint64(0) | |
| 1196 | + | for _, b := range raw { | |
| 1197 | + | id = id<<8 | uint64(b) | |
| 1198 | + | } | |
| 1199 | + | if id == 0 { | |
| 1200 | + | id = 1 | |
| 1201 | + | } | |
| 1202 | + | return id, nil | |
| 1203 | + | } | |
| 1204 | + | ||
| 1205 | + | // sendKillRequest allocates a unique nonzero RequestID and sends one Kill | |
| 1206 | + | // request, returning the ID its matching KillResult must carry. A caller | |
| 1207 | + | // already canceled before the send returns errKillNotSent (a definite | |
| 1208 | + | // not-sent outcome) instead of sending a request whose reply could no longer be | |
| 1209 | + | // awaited; the request is never replayed. | |
| 1210 | + | func sendKillRequest(ctx context.Context, connection ports.ClientConnection, request protocol.Kill) (uint64, error) { | |
| 1211 | + | requestID, err := newControlRequestID() | |
| 1212 | + | if err != nil { | |
| 1213 | + | return 0, fmt.Errorf("preparing request: %w", err) | |
| 1214 | + | } | |
| 1215 | + | request.RequestID = requestID | |
| 1216 | + | if err := ctx.Err(); err != nil { | |
| 1217 | + | return 0, fmt.Errorf("%w: %w", errKillNotSent, err) | |
| 1218 | + | } | |
| 1219 | + | if err := connection.SendClient(request); err != nil { | |
| 1220 | + | return 0, err | |
| 1221 | + | } | |
| 1222 | + | return requestID, nil | |
| 1223 | + | } | |
| 1224 | + | ||
| 1225 | + | // receiveKillResult consumes exactly one correlated KillResult from a control | |
| 1226 | + | // connection. A definite failed result becomes a plain error; a close, | |
| 1227 | + | // cancellation, or an uncorrelated reply becomes errKillOutcomeUnknown, so a | |
| 1228 | + | // caller never treats a lost result as success and never replays the kill. | |
| 1229 | + | func receiveKillResult(ctx context.Context, connection ports.ClientConnection, requestID uint64) error { | |
| 1230 | + | // Cancellation after the request was sent closes the exact connection so a | |
| 1231 | + | // blocked receive unwinds; the lost reply stays outcome-unknown rather than | |
| 1232 | + | // becoming a silent success or a blind retry. | |
| 1233 | + | stop := context.AfterFunc(ctx, func() { _ = connection.Close() }) | |
| 1234 | + | defer stop() | |
| 1235 | + | ||
| 1236 | + | reply, err := connection.ReceiveServer() | |
| 1237 | + | if err != nil { | |
| 1238 | + | if ctxErr := ctx.Err(); ctxErr != nil { | |
| 1239 | + | return fmt.Errorf("%w: awaiting explicit result: %v", errKillOutcomeUnknown, ctxErr) | |
| 1240 | + | } | |
| 1241 | + | return fmt.Errorf("%w: awaiting explicit result: %v", errKillOutcomeUnknown, err) | |
| 1242 | + | } | |
| 1243 | + | result, ok := reply.(protocol.KillResult) | |
| 1244 | + | if !ok || result.RequestID != requestID { | |
| 1245 | + | return fmt.Errorf("%w: unexpected reply %T", errKillOutcomeUnknown, reply) | |
| 1246 | + | } | |
| 1247 | + | switch result.Outcome { | |
| 1248 | + | case protocol.KillSucceeded: | |
| 1249 | + | return nil | |
| 1250 | + | case protocol.KillFailed: | |
| 1251 | + | return fmt.Errorf("vev: %s", result.Text) | |
| 1252 | + | default: | |
| 1253 | + | return fmt.Errorf("%w: %s", errKillOutcomeUnknown, result.Text) | |
| 1254 | + | } | |
| 1255 | + | } | |
| 1182 | 1256 | ||
| 1183 | 1257 | type localDaemonDialer struct { | |
| 1184 | 1258 | dir string | |
| @@ -1305,18 +1379,31 @@ func requestDaemonStop(ctx context.Context) error { | |||
| 1305 | 1379 | } | |
| 1306 | 1380 | defer func() { _ = transport.Close() }() | |
| 1307 | 1381 | connection := sessionwire.NewClientConnection(transport) | |
| 1308 | - | if err := connection.SendClient(protocol.Kill{Scope: protocol.KillDaemon}); err != nil { | |
| 1382 | + | requestID, err := sendKillRequest(ctx, connection, protocol.Kill{Scope: protocol.KillDaemon}) | |
| 1383 | + | if err != nil { | |
| 1309 | 1384 | return fmt.Errorf("vev: requesting daemon stop: %w", err) | |
| 1310 | 1385 | } | |
| 1311 | - | if _, err := connection.ReceiveServer(); err != nil && !errors.Is(err, io.EOF) { | |
| 1312 | - | return fmt.Errorf("vev: reading daemon stop reply: %w", err) | |
| 1386 | + | resultErr := receiveKillResult(ctx, connection, requestID) | |
| 1387 | + | if resultErr != nil && !errors.Is(resultErr, errKillOutcomeUnknown) { | |
| 1388 | + | return fmt.Errorf("vev: reading daemon stop reply: %w", resultErr) | |
| 1313 | 1389 | } | |
| 1314 | 1390 | ||
| 1315 | 1391 | owner, err = waitForLifecycleAvailability(ctx, ipc.SocketDir(), defaultBackoff) | |
| 1316 | 1392 | if err != nil { | |
| 1393 | + | if resultErr != nil { | |
| 1394 | + | return fmt.Errorf("vev: reading daemon stop reply: %w (lifecycle confirmation also failed: %v)", resultErr, err) | |
| 1395 | + | } | |
| 1317 | 1396 | return fmt.Errorf("vev: waiting for daemon ownership transfer: %w", err) | |
| 1318 | 1397 | } | |
| 1319 | - | return owner.Release() | |
| 1398 | + | if err := owner.Release(); err != nil { | |
| 1399 | + | if resultErr != nil { | |
| 1400 | + | return fmt.Errorf("vev: reading daemon stop reply: %w (confirmed ownership release failed: %v)", resultErr, err) | |
| 1401 | + | } | |
| 1402 | + | return err | |
| 1403 | + | } | |
| 1404 | + | // When the explicit reply was lost, acquiring daemon lifecycle ownership is | |
| 1405 | + | // direct authoritative confirmation that the accepted shutdown completed. | |
| 1406 | + | return nil | |
| 1320 | 1407 | } | |
| 1321 | 1408 | ||
| 1322 | 1409 | // runStdio is the hidden remote-side mode used by `ssh host vev _stdio`: it | |
| @@ -1753,29 +1840,12 @@ func runKill(ctx context.Context, name string, all, daemon bool) (retErr error) | |||
| 1753 | 1840 | } else if daemon { | |
| 1754 | 1841 | scope = protocol.KillDaemon | |
| 1755 | 1842 | } | |
| 1756 | - | if err := connection.SendClient(protocol.Kill{Name: name, Scope: scope}); err != nil { | |
| 1757 | - | cause := fmt.Errorf("vev: requesting kill: %w", err) | |
| 1758 | - | if daemon { | |
| 1759 | - | return forceStopDaemonFallback(ctx, cause) | |
| 1760 | - | } | |
| 1761 | - | return cause | |
| 1843 | + | requestID, err := sendKillRequest(ctx, connection, protocol.Kill{Name: name, Scope: scope}) | |
| 1844 | + | if err != nil { | |
| 1845 | + | return fmt.Errorf("vev: requesting kill: %w", err) | |
| 1762 | 1846 | } | |
| 1763 | - | reply, err := connection.ReceiveServer() | |
| 1764 | - | if err != nil && !errors.Is(err, io.EOF) { | |
| 1765 | - | cause := fmt.Errorf("vev: reading kill reply: %w", err) | |
| 1766 | - | if daemon { | |
| 1767 | - | return forceStopDaemonFallback(ctx, cause) | |
| 1768 | - | } | |
| 1769 | - | return cause | |
| 1770 | - | } | |
| 1771 | - | if err == nil { | |
| 1772 | - | if em, ok := reply.(protocol.ErrorMsg); ok { | |
| 1773 | - | cause := fmt.Errorf("vev: %s", em.Text) | |
| 1774 | - | if daemon { | |
| 1775 | - | return forceStopDaemonFallback(ctx, cause) | |
| 1776 | - | } | |
| 1777 | - | return cause | |
| 1778 | - | } | |
| 1847 | + | if err := receiveKillResult(ctx, connection, requestID); err != nil { | |
| 1848 | + | return fmt.Errorf("vev: reading kill reply: %w", err) | |
| 1779 | 1849 | } | |
| 1780 | 1850 | // Only an explicit daemon stop ends the daemon; kill-all purges sessions and | |
| 1781 | 1851 | // leaves it running, so it must not wait for an ownership transfer. | |
| @@ -1178,7 +1178,81 @@ func runAttachWithDeps(ctx context.Context, intent uint8, name, remoteTarget, ac | |||||
| 1178 | 1178 | ||||
| 1179 | const daemonStopTimeout = 2 * time.Second | 1179 | const daemonStopTimeout = 2 * time.Second | ||
| 1180 | 1180 | ||||
| 1181 | - | var errDaemonNotRunning = errors.New("vev: no daemon running") | 1181 | + | var ( |
| 1182 | + | errDaemonNotRunning = errors.New("vev: no daemon running") | |||
| 1183 | + | errKillOutcomeUnknown = errors.New("vev: kill outcome unknown") | |||
| 1184 | + | // errKillNotSent reports a Kill that was never placed on the wire because | |||
| 1185 | + | // its caller was already canceled. The outcome is definite: the daemon | |||
| 1186 | + | // cannot have acted, so it must not be reported as an unknown outcome. | |||
| 1187 | + | errKillNotSent = errors.New("vev: kill request not sent") | |||
| 1188 | + | ) | |||
| 1189 | + | ||||
| 1190 | + | func newControlRequestID() (uint64, error) { | |||
| 1191 | + | var raw [8]byte | |||
| 1192 | + | if _, err := rand.Read(raw[:]); err != nil { | |||
| 1193 | + | return 0, err | |||
| 1194 | + | } | |||
| 1195 | + | id := uint64(0) | |||
| 1196 | + | for _, b := range raw { | |||
| 1197 | + | id = id<<8 | uint64(b) | |||
| 1198 | + | } | |||
| 1199 | + | if id == 0 { | |||
| 1200 | + | id = 1 | |||
| 1201 | + | } | |||
| 1202 | + | return id, nil | |||
| 1203 | + | } | |||
| 1204 | + | ||||
| 1205 | + | // sendKillRequest allocates a unique nonzero RequestID and sends one Kill | |||
| 1206 | + | // request, returning the ID its matching KillResult must carry. A caller | |||
| 1207 | + | // already canceled before the send returns errKillNotSent (a definite | |||
| 1208 | + | // not-sent outcome) instead of sending a request whose reply could no longer be | |||
| 1209 | + | // awaited; the request is never replayed. | |||
| 1210 | + | func sendKillRequest(ctx context.Context, connection ports.ClientConnection, request protocol.Kill) (uint64, error) { | |||
| 1211 | + | requestID, err := newControlRequestID() | |||
| 1212 | + | if err != nil { | |||
| 1213 | + | return 0, fmt.Errorf("preparing request: %w", err) | |||
| 1214 | + | } | |||
| 1215 | + | request.RequestID = requestID | |||
| 1216 | + | if err := ctx.Err(); err != nil { | |||
| 1217 | + | return 0, fmt.Errorf("%w: %w", errKillNotSent, err) | |||
| 1218 | + | } | |||
| 1219 | + | if err := connection.SendClient(request); err != nil { | |||
| 1220 | + | return 0, err | |||
| 1221 | + | } | |||
| 1222 | + | return requestID, nil | |||
| 1223 | + | } | |||
| 1224 | + | ||||
| 1225 | + | // receiveKillResult consumes exactly one correlated KillResult from a control | |||
| 1226 | + | // connection. A definite failed result becomes a plain error; a close, | |||
| 1227 | + | // cancellation, or an uncorrelated reply becomes errKillOutcomeUnknown, so a | |||
| 1228 | + | // caller never treats a lost result as success and never replays the kill. | |||
| 1229 | + | func receiveKillResult(ctx context.Context, connection ports.ClientConnection, requestID uint64) error { | |||
| 1230 | + | // Cancellation after the request was sent closes the exact connection so a | |||
| 1231 | + | // blocked receive unwinds; the lost reply stays outcome-unknown rather than | |||
| 1232 | + | // becoming a silent success or a blind retry. | |||
| 1233 | + | stop := context.AfterFunc(ctx, func() { _ = connection.Close() }) | |||
| 1234 | + | defer stop() | |||
| 1235 | + | ||||
| 1236 | + | reply, err := connection.ReceiveServer() | |||
| 1237 | + | if err != nil { | |||
| 1238 | + | if ctxErr := ctx.Err(); ctxErr != nil { | |||
| 1239 | + | return fmt.Errorf("%w: awaiting explicit result: %v", errKillOutcomeUnknown, ctxErr) | |||
| 1240 | + | } | |||
| 1241 | + | return fmt.Errorf("%w: awaiting explicit result: %v", errKillOutcomeUnknown, err) | |||
| 1242 | + | } | |||
| 1243 | + | result, ok := reply.(protocol.KillResult) | |||
| 1244 | + | if !ok || result.RequestID != requestID { | |||
| 1245 | + | return fmt.Errorf("%w: unexpected reply %T", errKillOutcomeUnknown, reply) | |||
| 1246 | + | } | |||
| 1247 | + | switch result.Outcome { | |||
| 1248 | + | case protocol.KillSucceeded: | |||
| 1249 | + | return nil | |||
| 1250 | + | case protocol.KillFailed: | |||
| 1251 | + | return fmt.Errorf("vev: %s", result.Text) | |||
| 1252 | + | default: | |||
| 1253 | + | return fmt.Errorf("%w: %s", errKillOutcomeUnknown, result.Text) | |||
| 1254 | + | } | |||
| 1255 | + | } | |||
| 1182 | 1256 | ||||
| 1183 | type localDaemonDialer struct { | 1257 | type localDaemonDialer struct { | ||
| 1184 | dir string | 1258 | dir string | ||
| @@ -1305,18 +1379,31 @@ func requestDaemonStop(ctx context.Context) error { | |||||
| 1305 | } | 1379 | } | ||
| 1306 | defer func() { _ = transport.Close() }() | 1380 | defer func() { _ = transport.Close() }() | ||
| 1307 | connection := sessionwire.NewClientConnection(transport) | 1381 | connection := sessionwire.NewClientConnection(transport) | ||
| 1308 | - | if err := connection.SendClient(protocol.Kill{Scope: protocol.KillDaemon}); err != nil { | 1382 | + | requestID, err := sendKillRequest(ctx, connection, protocol.Kill{Scope: protocol.KillDaemon}) |
| 1383 | + | if err != nil { | |||
| 1309 | return fmt.Errorf("vev: requesting daemon stop: %w", err) | 1384 | return fmt.Errorf("vev: requesting daemon stop: %w", err) | ||
| 1310 | } | 1385 | } | ||
| 1311 | - | if _, err := connection.ReceiveServer(); err != nil && !errors.Is(err, io.EOF) { | 1386 | + | resultErr := receiveKillResult(ctx, connection, requestID) |
| 1312 | - | return fmt.Errorf("vev: reading daemon stop reply: %w", err) | 1387 | + | if resultErr != nil && !errors.Is(resultErr, errKillOutcomeUnknown) { |
| 1388 | + | return fmt.Errorf("vev: reading daemon stop reply: %w", resultErr) | |||
| 1313 | } | 1389 | } | ||
| 1314 | 1390 | ||||
| 1315 | owner, err = waitForLifecycleAvailability(ctx, ipc.SocketDir(), defaultBackoff) | 1391 | owner, err = waitForLifecycleAvailability(ctx, ipc.SocketDir(), defaultBackoff) | ||
| 1316 | if err != nil { | 1392 | if err != nil { | ||
| 1393 | + | if resultErr != nil { | |||
| 1394 | + | return fmt.Errorf("vev: reading daemon stop reply: %w (lifecycle confirmation also failed: %v)", resultErr, err) | |||
| 1395 | + | } | |||
| 1317 | return fmt.Errorf("vev: waiting for daemon ownership transfer: %w", err) | 1396 | return fmt.Errorf("vev: waiting for daemon ownership transfer: %w", err) | ||
| 1318 | } | 1397 | } | ||
| 1319 | - | return owner.Release() | 1398 | + | if err := owner.Release(); err != nil { |
| 1399 | + | if resultErr != nil { | |||
| 1400 | + | return fmt.Errorf("vev: reading daemon stop reply: %w (confirmed ownership release failed: %v)", resultErr, err) | |||
| 1401 | + | } | |||
| 1402 | + | return err | |||
| 1403 | + | } | |||
| 1404 | + | // When the explicit reply was lost, acquiring daemon lifecycle ownership is | |||
| 1405 | + | // direct authoritative confirmation that the accepted shutdown completed. | |||
| 1406 | + | return nil | |||
| 1320 | } | 1407 | } | ||
| 1321 | 1408 | ||||
| 1322 | // runStdio is the hidden remote-side mode used by `ssh host vev _stdio`: it | 1409 | // runStdio is the hidden remote-side mode used by `ssh host vev _stdio`: it | ||
| @@ -1753,29 +1840,12 @@ func runKill(ctx context.Context, name string, all, daemon bool) (retErr error) | |||||
| 1753 | } else if daemon { | 1840 | } else if daemon { | ||
| 1754 | scope = protocol.KillDaemon | 1841 | scope = protocol.KillDaemon | ||
| 1755 | } | 1842 | } | ||
| 1756 | - | if err := connection.SendClient(protocol.Kill{Name: name, Scope: scope}); err != nil { | 1843 | + | requestID, err := sendKillRequest(ctx, connection, protocol.Kill{Name: name, Scope: scope}) |
| 1757 | - | cause := fmt.Errorf("vev: requesting kill: %w", err) | 1844 | + | if err != nil { |
| 1758 | - | if daemon { | 1845 | + | return fmt.Errorf("vev: requesting kill: %w", err) |
| 1759 | - | return forceStopDaemonFallback(ctx, cause) | |||
| 1760 | - | } | |||
| 1761 | - | return cause | |||
| 1762 | } | 1846 | } | ||
| 1763 | - | reply, err := connection.ReceiveServer() | 1847 | + | if err := receiveKillResult(ctx, connection, requestID); err != nil { |
| 1764 | - | if err != nil && !errors.Is(err, io.EOF) { | 1848 | + | return fmt.Errorf("vev: reading kill reply: %w", err) |
| 1765 | - | cause := fmt.Errorf("vev: reading kill reply: %w", err) | |||
| 1766 | - | if daemon { | |||
| 1767 | - | return forceStopDaemonFallback(ctx, cause) | |||
| 1768 | - | } | |||
| 1769 | - | return cause | |||
| 1770 | - | } | |||
| 1771 | - | if err == nil { | |||
| 1772 | - | if em, ok := reply.(protocol.ErrorMsg); ok { | |||
| 1773 | - | cause := fmt.Errorf("vev: %s", em.Text) | |||
| 1774 | - | if daemon { | |||
| 1775 | - | return forceStopDaemonFallback(ctx, cause) | |||
| 1776 | - | } | |||
| 1777 | - | return cause | |||
| 1778 | - | } | |||
| 1779 | } | 1849 | } | ||
| 1780 | // Only an explicit daemon stop ends the daemon; kill-all purges sessions and | 1850 | // Only an explicit daemon stop ends the daemon; kill-all purges sessions and | ||
| 1781 | // leaves it running, so it must not wait for an ownership transfer. | 1851 | // leaves it running, so it must not wait for an ownership transfer. | ||
M internal/protocol/messages.go +1 -0
| @@ -42,6 +42,7 @@ func (Detached) serverMessage() {} | |||
| 42 | 42 | func (Pong) serverMessage() {} | |
| 43 | 43 | func (Sessions) serverMessage() {} | |
| 44 | 44 | func (CommandResult) serverMessage() {} | |
| 45 | + | func (KillResult) serverMessage() {} | |
| 45 | 46 | func (AttachTarget) serverMessage() {} | |
| 46 | 47 | func (RemotePreview) serverMessage() {} | |
| 47 | 48 | func (CommittedRouteIdentity) serverMessage() {} | |
| @@ -42,6 +42,7 @@ func (Detached) serverMessage() {} | |||||
| 42 | func (Pong) serverMessage() {} | 42 | func (Pong) serverMessage() {} | ||
| 43 | func (Sessions) serverMessage() {} | 43 | func (Sessions) serverMessage() {} | ||
| 44 | func (CommandResult) serverMessage() {} | 44 | func (CommandResult) serverMessage() {} | ||
| 45 | + | func (KillResult) serverMessage() {} | |||
| 45 | func (AttachTarget) serverMessage() {} | 46 | func (AttachTarget) serverMessage() {} | ||
| 46 | func (RemotePreview) serverMessage() {} | 47 | func (RemotePreview) serverMessage() {} | ||
| 47 | func (CommittedRouteIdentity) serverMessage() {} | 48 | func (CommittedRouteIdentity) serverMessage() {} | ||
M internal/protocol/session.go +25 -2
| @@ -16,7 +16,7 @@ import ( | |||
| 16 | 16 | // daemonmux physical preamble conversation (multiplex.proto), whose | |
| 17 | 17 | // negotiated ceilings and accepted daemon binding ride the same wire | |
| 18 | 18 | // version as the session and broker conversations. | |
| 19 | - | const Version uint16 = 56 | |
| 19 | + | const Version uint16 = 57 | |
| 20 | 20 | ||
| 21 | 21 | // HandshakeTimeout bounds every transport handshake from connect through the | |
| 22 | 22 | // first committed publication. It excludes the preceding client-local | |
| @@ -295,8 +295,31 @@ const ( | |||
| 295 | 295 | ) | |
| 296 | 296 | ||
| 297 | 297 | type Kill struct { | |
| 298 | + | RequestID uint64 | |
| 299 | + | Name string | |
| 300 | + | Scope KillScope | |
| 301 | + | } | |
| 302 | + | ||
| 303 | + | type KillOutcome uint8 | |
| 304 | + | ||
| 305 | + | const ( | |
| 306 | + | KillSucceeded KillOutcome = iota + 1 | |
| 307 | + | KillFailed | |
| 308 | + | KillOutcomeUnknown | |
| 309 | + | ) | |
| 310 | + | ||
| 311 | + | type KillFailure struct { | |
| 312 | + | Class string | |
| 298 | 313 | Name string | |
| 299 | - | Scope KillScope | |
| 314 | + | Text string | |
| 315 | + | } | |
| 316 | + | ||
| 317 | + | type KillResult struct { | |
| 318 | + | RequestID uint64 | |
| 319 | + | Outcome KillOutcome | |
| 320 | + | Code uint16 | |
| 321 | + | Text string | |
| 322 | + | Failures []KillFailure | |
| 300 | 323 | } | |
| 301 | 324 | ||
| 302 | 325 | type SessionState uint8 | |
| @@ -16,7 +16,7 @@ import ( | |||||
| 16 | // daemonmux physical preamble conversation (multiplex.proto), whose | 16 | // daemonmux physical preamble conversation (multiplex.proto), whose | ||
| 17 | // negotiated ceilings and accepted daemon binding ride the same wire | 17 | // negotiated ceilings and accepted daemon binding ride the same wire | ||
| 18 | // version as the session and broker conversations. | 18 | // version as the session and broker conversations. | ||
| 19 | - | const Version uint16 = 56 | 19 | + | const Version uint16 = 57 |
| 20 | 20 | ||||
| 21 | // HandshakeTimeout bounds every transport handshake from connect through the | 21 | // HandshakeTimeout bounds every transport handshake from connect through the | ||
| 22 | // first committed publication. It excludes the preceding client-local | 22 | // first committed publication. It excludes the preceding client-local | ||
| @@ -295,8 +295,31 @@ const ( | |||||
| 295 | ) | 295 | ) | ||
| 296 | 296 | ||||
| 297 | type Kill struct { | 297 | type Kill struct { | ||
| 298 | + | RequestID uint64 | |||
| 299 | + | Name string | |||
| 300 | + | Scope KillScope | |||
| 301 | + | } | |||
| 302 | + | ||||
| 303 | + | type KillOutcome uint8 | |||
| 304 | + | ||||
| 305 | + | const ( | |||
| 306 | + | KillSucceeded KillOutcome = iota + 1 | |||
| 307 | + | KillFailed | |||
| 308 | + | KillOutcomeUnknown | |||
| 309 | + | ) | |||
| 310 | + | ||||
| 311 | + | type KillFailure struct { | |||
| 312 | + | Class string | |||
| 298 | Name string | 313 | Name string | ||
| 299 | - | Scope KillScope | 314 | + | Text string |
| 315 | + | } | |||
| 316 | + | ||||
| 317 | + | type KillResult struct { | |||
| 318 | + | RequestID uint64 | |||
| 319 | + | Outcome KillOutcome | |||
| 320 | + | Code uint16 | |||
| 321 | + | Text string | |||
| 322 | + | Failures []KillFailure | |||
| 300 | } | 323 | } | ||
| 301 | 324 | ||||
| 302 | type SessionState uint8 | 325 | type SessionState uint8 | ||
M internal/protocol/session_test.go +1 -1
| @@ -10,7 +10,7 @@ import ( | |||
| 10 | 10 | ) | |
| 11 | 11 | ||
| 12 | 12 | func TestVersionRemainsWireCompatible(t *testing.T) { | |
| 13 | - | require.Equal(t, uint16(56), Version) | |
| 13 | + | require.Equal(t, uint16(57), Version) | |
| 14 | 14 | require.Equal(t, (16<<20)-407, MaxOutputDataLen) | |
| 15 | 15 | } | |
| 16 | 16 | ||
| @@ -10,7 +10,7 @@ import ( | |||||
| 10 | ) | 10 | ) | ||
| 11 | 11 | ||||
| 12 | func TestVersionRemainsWireCompatible(t *testing.T) { | 12 | func TestVersionRemainsWireCompatible(t *testing.T) { | ||
| 13 | - | require.Equal(t, uint16(56), Version) | 13 | + | require.Equal(t, uint16(57), Version) |
| 14 | require.Equal(t, (16<<20)-407, MaxOutputDataLen) | 14 | require.Equal(t, (16<<20)-407, MaxOutputDataLen) | ||
| 15 | } | 15 | } | ||
| 16 | 16 | ||||
M internal/protocol/wire/envelope.pb.go +28 -7 generated file: collapsed
M internal/protocol/wire/envelope_inventory_test.go +3 -2
| @@ -15,8 +15,8 @@ import ( | |||
| 15 | 15 | // shares a payload type except route navigation failure, and the preamble | |
| 16 | 16 | // constants agree with the schema authority. | |
| 17 | 17 | func TestEnvelopeInventoryProvesDirectionAndSemanticPath(t *testing.T) { | |
| 18 | - | if protocol.Version != 56 { | |
| 19 | - | t.Fatalf("protocol.Version = %d, want 56", protocol.Version) | |
| 18 | + | if protocol.Version != 57 { | |
| 19 | + | t.Fatalf("protocol.Version = %d, want 57", protocol.Version) | |
| 20 | 20 | } | |
| 21 | 21 | if ProtocolEpoch != 1 { | |
| 22 | 22 | t.Fatalf("ProtocolEpoch = %d, want 1", ProtocolEpoch) | |
| @@ -54,6 +54,7 @@ func TestEnvelopeInventoryProvesDirectionAndSemanticPath(t *testing.T) { | |||
| 54 | 54 | "navigation_inventory_selection": true, "picker_offer": true, | |
| 55 | 55 | "picker_snapshot": true, "picker_closed": true, "picker_result": true, | |
| 56 | 56 | "picker_failure": true, "picker_preview": true, "picker_control_response": true, | |
| 57 | + | "kill_result": true, | |
| 57 | 58 | } | |
| 58 | 59 | ||
| 59 | 60 | if len(clientVariants) != len(clientSemantic) { | |
| @@ -15,8 +15,8 @@ import ( | |||||
| 15 | // shares a payload type except route navigation failure, and the preamble | 15 | // shares a payload type except route navigation failure, and the preamble | ||
| 16 | // constants agree with the schema authority. | 16 | // constants agree with the schema authority. | ||
| 17 | func TestEnvelopeInventoryProvesDirectionAndSemanticPath(t *testing.T) { | 17 | func TestEnvelopeInventoryProvesDirectionAndSemanticPath(t *testing.T) { | ||
| 18 | - | if protocol.Version != 56 { | 18 | + | if protocol.Version != 57 { |
| 19 | - | t.Fatalf("protocol.Version = %d, want 56", protocol.Version) | 19 | + | t.Fatalf("protocol.Version = %d, want 57", protocol.Version) |
| 20 | } | 20 | } | ||
| 21 | if ProtocolEpoch != 1 { | 21 | if ProtocolEpoch != 1 { | ||
| 22 | t.Fatalf("ProtocolEpoch = %d, want 1", ProtocolEpoch) | 22 | t.Fatalf("ProtocolEpoch = %d, want 1", ProtocolEpoch) | ||
| @@ -54,6 +54,7 @@ func TestEnvelopeInventoryProvesDirectionAndSemanticPath(t *testing.T) { | |||||
| 54 | "navigation_inventory_selection": true, "picker_offer": true, | 54 | "navigation_inventory_selection": true, "picker_offer": true, | ||
| 55 | "picker_snapshot": true, "picker_closed": true, "picker_result": true, | 55 | "picker_snapshot": true, "picker_closed": true, "picker_result": true, | ||
| 56 | "picker_failure": true, "picker_preview": true, "picker_control_response": true, | 56 | "picker_failure": true, "picker_preview": true, "picker_control_response": true, | ||
| 57 | + | "kill_result": true, | |||
| 57 | } | 58 | } | ||
| 58 | 59 | ||||
| 59 | if len(clientVariants) != len(clientSemantic) { | 60 | if len(clientVariants) != len(clientSemantic) { | ||
M internal/protocol/wire/schema/envelope.proto +1 -0
| @@ -138,5 +138,6 @@ message ServerEnvelope { | |||
| 138 | 138 | PickerControlResponse picker_control_response = 28; | |
| 139 | 139 | AttachmentSuspended attachment_suspended = 31; | |
| 140 | 140 | AttachmentActivated attachment_activated = 32; | |
| 141 | + | KillResult kill_result = 33; | |
| 141 | 142 | } | |
| 142 | 143 | } | |
| @@ -138,5 +138,6 @@ message ServerEnvelope { | |||||
| 138 | PickerControlResponse picker_control_response = 28; | 138 | PickerControlResponse picker_control_response = 28; | ||
| 139 | AttachmentSuspended attachment_suspended = 31; | 139 | AttachmentSuspended attachment_suspended = 31; | ||
| 140 | AttachmentActivated attachment_activated = 32; | 140 | AttachmentActivated attachment_activated = 32; | ||
| 141 | + | KillResult kill_result = 33; | |||
| 141 | } | 142 | } | ||
| 142 | } | 143 | } | ||
M internal/protocol/wire/schema/session.proto +16 -0
| @@ -90,6 +90,22 @@ message Kill { | |||
| 90 | 90 | string name = 1; | |
| 91 | 91 | // Closed scope taxonomy: 0 = session, 1 = daemon, 2 = all. | |
| 92 | 92 | uint32 scope = 2; | |
| 93 | + | uint64 request_id = 3; | |
| 94 | + | } | |
| 95 | + | ||
| 96 | + | message KillFailure { | |
| 97 | + | string class = 1; | |
| 98 | + | string name = 2; | |
| 99 | + | string text = 3; | |
| 100 | + | } | |
| 101 | + | ||
| 102 | + | message KillResult { | |
| 103 | + | uint64 request_id = 1; | |
| 104 | + | // Closed outcome taxonomy: 1 = succeeded, 2 = failed, 3 = outcome unknown. | |
| 105 | + | uint32 outcome = 2; | |
| 106 | + | uint32 code = 3; | |
| 107 | + | string text = 4; | |
| 108 | + | repeated KillFailure failures = 5; | |
| 93 | 109 | } | |
| 94 | 110 | ||
| 95 | 111 | // Detached reports why an attachment ended. | |
| @@ -90,6 +90,22 @@ message Kill { | |||||
| 90 | string name = 1; | 90 | string name = 1; | ||
| 91 | // Closed scope taxonomy: 0 = session, 1 = daemon, 2 = all. | 91 | // Closed scope taxonomy: 0 = session, 1 = daemon, 2 = all. | ||
| 92 | uint32 scope = 2; | 92 | uint32 scope = 2; | ||
| 93 | + | uint64 request_id = 3; | |||
| 94 | + | } | |||
| 95 | + | ||||
| 96 | + | message KillFailure { | |||
| 97 | + | string class = 1; | |||
| 98 | + | string name = 2; | |||
| 99 | + | string text = 3; | |||
| 100 | + | } | |||
| 101 | + | ||||
| 102 | + | message KillResult { | |||
| 103 | + | uint64 request_id = 1; | |||
| 104 | + | // Closed outcome taxonomy: 1 = succeeded, 2 = failed, 3 = outcome unknown. | |||
| 105 | + | uint32 outcome = 2; | |||
| 106 | + | uint32 code = 3; | |||
| 107 | + | string text = 4; | |||
| 108 | + | repeated KillFailure failures = 5; | |||
| 93 | } | 109 | } | ||
| 94 | 110 | ||||
| 95 | // Detached reports why an attachment ended. | 111 | // Detached reports why an attachment ended. | ||
M internal/protocol/wire/session.pb.go +282 -120 generated file: collapsed
M internal/usecase/client/typed_connection_test.go +2 -0
| @@ -349,6 +349,8 @@ func serverMessageNameForTest(payload []byte) string { | |||
| 349 | 349 | return "Sessions" | |
| 350 | 350 | case protocol.CommandResult: | |
| 351 | 351 | return "CommandResult" | |
| 352 | + | case protocol.KillResult: | |
| 353 | + | return "KillResult" | |
| 352 | 354 | case protocol.AttachTarget: | |
| 353 | 355 | return "AttachTarget" | |
| 354 | 356 | case protocol.RemotePreview: | |
| @@ -349,6 +349,8 @@ func serverMessageNameForTest(payload []byte) string { | |||||
| 349 | return "Sessions" | 349 | return "Sessions" | ||
| 350 | case protocol.CommandResult: | 350 | case protocol.CommandResult: | ||
| 351 | return "CommandResult" | 351 | return "CommandResult" | ||
| 352 | + | case protocol.KillResult: | |||
| 353 | + | return "KillResult" | |||
| 352 | case protocol.AttachTarget: | 354 | case protocol.AttachTarget: | ||
| 353 | return "AttachTarget" | 355 | return "AttachTarget" | ||
| 354 | case protocol.RemotePreview: | 356 | case protocol.RemotePreview: | ||
M internal/usecase/client/wire_external_test_helpers_test.go +2 -0
| @@ -147,6 +147,8 @@ func serverMessageNameForTest(payload []byte) string { | |||
| 147 | 147 | return "Sessions" | |
| 148 | 148 | case protocol.CommandResult: | |
| 149 | 149 | return "CommandResult" | |
| 150 | + | case protocol.KillResult: | |
| 151 | + | return "KillResult" | |
| 150 | 152 | case protocol.AttachTarget: | |
| 151 | 153 | return "AttachTarget" | |
| 152 | 154 | case protocol.RemotePreview: | |
| @@ -147,6 +147,8 @@ func serverMessageNameForTest(payload []byte) string { | |||||
| 147 | return "Sessions" | 147 | return "Sessions" | ||
| 148 | case protocol.CommandResult: | 148 | case protocol.CommandResult: | ||
| 149 | return "CommandResult" | 149 | return "CommandResult" | ||
| 150 | + | case protocol.KillResult: | |||
| 151 | + | return "KillResult" | |||
| 150 | case protocol.AttachTarget: | 152 | case protocol.AttachTarget: | ||
| 151 | return "AttachTarget" | 153 | return "AttachTarget" | ||
| 152 | case protocol.RemotePreview: | 154 | case protocol.RemotePreview: | ||
M internal/usecase/daemon/control.go +7 -5
| @@ -87,12 +87,14 @@ func (d *Daemon) boundedControlSend(tr ports.ServerConnection, message protocol. | |||
| 87 | 87 | return err | |
| 88 | 88 | } | |
| 89 | 89 | ||
| 90 | - | // logControlSendFailure records that an error response could not be delivered. | |
| 91 | - | // A one-shot control client treats EOF as success, so a failed error send would | |
| 92 | - | // otherwise let it infer success from a silent close. | |
| 93 | - | func (d *Daemon) logControlSendFailure(operation string, err error) { | |
| 90 | + | // logKillResultSendFailure records that one one-shot kill result could not be | |
| 91 | + | // delivered. The kill path answers with an explicit correlated KillResult, so a | |
| 92 | + | // failed result send leaves the client at outcome-unknown rather than letting it | |
| 93 | + | // infer success from a silent close; the outcome and code fields preserve what | |
| 94 | + | // the client never observed. | |
| 95 | + | func (d *Daemon) logKillResultSendFailure(result protocol.KillResult, err error) { | |
| 94 | 96 | if err != nil { | |
| 95 | - | d.log.Warn("control error response send failed", "operation", operation, "err", err) | |
| 97 | + | d.log.Warn("control error response send failed", "operation", "kill result", "outcome", result.Outcome, "code", result.Code, "err", err) | |
| 96 | 98 | } | |
| 97 | 99 | } | |
| 98 | 100 | ||
| @@ -87,12 +87,14 @@ func (d *Daemon) boundedControlSend(tr ports.ServerConnection, message protocol. | |||||
| 87 | return err | 87 | return err | ||
| 88 | } | 88 | } | ||
| 89 | 89 | ||||
| 90 | - | // logControlSendFailure records that an error response could not be delivered. | 90 | + | // logKillResultSendFailure records that one one-shot kill result could not be |
| 91 | - | // A one-shot control client treats EOF as success, so a failed error send would | 91 | + | // delivered. The kill path answers with an explicit correlated KillResult, so a |
| 92 | - | // otherwise let it infer success from a silent close. | 92 | + | // failed result send leaves the client at outcome-unknown rather than letting it |
| 93 | - | func (d *Daemon) logControlSendFailure(operation string, err error) { | 93 | + | // infer success from a silent close; the outcome and code fields preserve what |
| 94 | + | // the client never observed. | |||
| 95 | + | func (d *Daemon) logKillResultSendFailure(result protocol.KillResult, err error) { | |||
| 94 | if err != nil { | 96 | if err != nil { | ||
| 95 | - | d.log.Warn("control error response send failed", "operation", operation, "err", err) | 97 | + | d.log.Warn("control error response send failed", "operation", "kill result", "outcome", result.Outcome, "code", result.Code, "err", err) |
| 96 | } | 98 | } | ||
| 97 | } | 99 | } | ||
| 98 | 100 | ||||
M internal/usecase/daemon/control_test.go +7 -5
| @@ -1482,13 +1482,15 @@ func TestHandleKillSessionParticipantsChangedReportsRetryable(t *testing.T) { | |||
| 1482 | 1482 | t.Cleanup(func() { d.afterAttachmentEffectParticipantsSnapshotted = nil }) | |
| 1483 | 1483 | ||
| 1484 | 1484 | tr, sends := newCapturingTransport(t) | |
| 1485 | - | d.handleKill(tr, protocol.Kill{Scope: protocol.KillSession, Name: "work"}) | |
| 1485 | + | d.handleKill(tr, protocol.Kill{RequestID: 1, Scope: protocol.KillSession, Name: "work"}) | |
| 1486 | 1486 | ||
| 1487 | - | frame := awaitFrame(t, sends, "Error") | |
| 1488 | - | msg, ok := decodeServerMessage(t, frame).(protocol.ErrorMsg) | |
| 1487 | + | frame := awaitFrame(t, sends, "KillResult") | |
| 1488 | + | result, ok := decodeServerMessage(t, frame).(protocol.KillResult) | |
| 1489 | 1489 | require.True(t, ok) | |
| 1490 | - | require.Equal(t, protocol.ErrInternal, msg.Code) | |
| 1491 | - | require.Equal(t, sessionKillRetryMessage, msg.Text) | |
| 1490 | + | require.Equal(t, uint64(1), result.RequestID) | |
| 1491 | + | require.Equal(t, protocol.KillFailed, result.Outcome) | |
| 1492 | + | require.Equal(t, protocol.ErrInternal, result.Code) | |
| 1493 | + | require.Equal(t, sessionKillRetryMessage, result.Text) | |
| 1492 | 1494 | require.Equal(t, 1, sessionCount(d), "an aborted direct kill must leave the session live") | |
| 1493 | 1495 | require.Same(t, sess, firstSession(d)) | |
| 1494 | 1496 | } | |
| @@ -1482,13 +1482,15 @@ func TestHandleKillSessionParticipantsChangedReportsRetryable(t *testing.T) { | |||||
| 1482 | t.Cleanup(func() { d.afterAttachmentEffectParticipantsSnapshotted = nil }) | 1482 | t.Cleanup(func() { d.afterAttachmentEffectParticipantsSnapshotted = nil }) | ||
| 1483 | 1483 | ||||
| 1484 | tr, sends := newCapturingTransport(t) | 1484 | tr, sends := newCapturingTransport(t) | ||
| 1485 | - | d.handleKill(tr, protocol.Kill{Scope: protocol.KillSession, Name: "work"}) | 1485 | + | d.handleKill(tr, protocol.Kill{RequestID: 1, Scope: protocol.KillSession, Name: "work"}) |
| 1486 | 1486 | ||||
| 1487 | - | frame := awaitFrame(t, sends, "Error") | 1487 | + | frame := awaitFrame(t, sends, "KillResult") |
| 1488 | - | msg, ok := decodeServerMessage(t, frame).(protocol.ErrorMsg) | 1488 | + | result, ok := decodeServerMessage(t, frame).(protocol.KillResult) |
| 1489 | require.True(t, ok) | 1489 | require.True(t, ok) | ||
| 1490 | - | require.Equal(t, protocol.ErrInternal, msg.Code) | 1490 | + | require.Equal(t, uint64(1), result.RequestID) |
| 1491 | - | require.Equal(t, sessionKillRetryMessage, msg.Text) | 1491 | + | require.Equal(t, protocol.KillFailed, result.Outcome) |
| 1492 | + | require.Equal(t, protocol.ErrInternal, result.Code) | |||
| 1493 | + | require.Equal(t, sessionKillRetryMessage, result.Text) | |||
| 1492 | require.Equal(t, 1, sessionCount(d), "an aborted direct kill must leave the session live") | 1494 | require.Equal(t, 1, sessionCount(d), "an aborted direct kill must leave the session live") | ||
| 1493 | require.Same(t, sess, firstSession(d)) | 1495 | require.Same(t, sess, firstSession(d)) | ||
| 1494 | } | 1496 | } | ||
M internal/usecase/daemon/daemon.go +25 -10
| @@ -1326,29 +1326,40 @@ func (d *Daemon) handleList(tr ports.ServerConnection) { | |||
| 1326 | 1326 | } | |
| 1327 | 1327 | ||
| 1328 | 1328 | // handleKill terminates the requested live session or stopped named session, | |
| 1329 | - | // purges every session (KillAll), or stops the daemon (KillDaemon), and closes | |
| 1330 | - | // the control connection; the resulting EOF is the client's success signal. | |
| 1331 | - | // KillAll leaves the daemon active; KillDaemon is the explicit stop. | |
| 1329 | + | // purges every session (KillAll), or accepts daemon shutdown (KillDaemon). | |
| 1330 | + | // Every normally decoded request receives exactly one correlated result before | |
| 1331 | + | // the control connection closes. KillAll leaves the daemon active. | |
| 1332 | 1332 | func (d *Daemon) handleKill(tr ports.ServerConnection, request protocol.Kill) { | |
| 1333 | 1333 | defer func() { _ = tr.Close() }() | |
| 1334 | + | send := func(result protocol.KillResult) { | |
| 1335 | + | result.RequestID = request.RequestID | |
| 1336 | + | d.logKillResultSendFailure(result, d.boundedControlSend(tr, result)) | |
| 1337 | + | } | |
| 1334 | 1338 | ||
| 1335 | 1339 | switch request.Scope { | |
| 1336 | 1340 | case protocol.KillAll: | |
| 1337 | 1341 | result := d.purgeAllSessions(protocol.ReasonSessionKilled) | |
| 1338 | 1342 | if result.Superseded() { | |
| 1339 | - | d.logControlSendFailure("kill all superseded response", d.boundedControlSend(tr, serverError(protocol.ErrServerShutdown, "daemon is shutting down"))) | |
| 1343 | + | send(protocol.KillResult{Outcome: protocol.KillOutcomeUnknown, Code: protocol.ErrServerShutdown, Text: "daemon is shutting down"}) | |
| 1340 | 1344 | return | |
| 1341 | 1345 | } | |
| 1342 | 1346 | if result.Failed() { | |
| 1343 | - | d.logControlSendFailure("kill all failure response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, result.summary()))) | |
| 1347 | + | failures := make([]protocol.KillFailure, 0, min(len(result.Failures), purgeSummaryFailureLimit)) | |
| 1348 | + | for _, failure := range result.Failures[:min(len(result.Failures), purgeSummaryFailureLimit)] { | |
| 1349 | + | failures = append(failures, protocol.KillFailure{Class: failure.Class.String(), Name: failure.Name, Text: failure.Err.Error()}) | |
| 1350 | + | } | |
| 1351 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: result.summary(), Failures: failures}) | |
| 1352 | + | return | |
| 1344 | 1353 | } | |
| 1354 | + | send(protocol.KillResult{Outcome: protocol.KillSucceeded}) | |
| 1345 | 1355 | return | |
| 1346 | 1356 | case protocol.KillDaemon: | |
| 1357 | + | send(protocol.KillResult{Outcome: protocol.KillSucceeded}) | |
| 1347 | 1358 | d.shutdownAll(protocol.ReasonServerShutdown) | |
| 1348 | 1359 | return | |
| 1349 | 1360 | case protocol.KillSession: | |
| 1350 | 1361 | default: | |
| 1351 | - | d.logControlSendFailure("invalid kill scope response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, "invalid kill scope"))) | |
| 1362 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "invalid kill scope"}) | |
| 1352 | 1363 | return | |
| 1353 | 1364 | } | |
| 1354 | 1365 | ||
| @@ -1361,15 +1372,17 @@ func (d *Daemon) handleKill(tr ports.ServerConnection, request protocol.Kill) { | |||
| 1361 | 1372 | // deletion order as live and offline purges. | |
| 1362 | 1373 | if err := d.retryStoppedPurge(request.Name); err != nil { | |
| 1363 | 1374 | d.log.Warn("deleting stopped session failed", "err", err, "session", request.Name) | |
| 1364 | - | d.logControlSendFailure("stopped delete failure response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, "deleting stopped session failed"))) | |
| 1375 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "deleting stopped session failed"}) | |
| 1376 | + | return | |
| 1365 | 1377 | } | |
| 1378 | + | send(protocol.KillResult{Outcome: protocol.KillSucceeded}) | |
| 1366 | 1379 | return | |
| 1367 | 1380 | } | |
| 1368 | 1381 | } | |
| 1369 | 1382 | d.mu.Unlock() | |
| 1370 | 1383 | ||
| 1371 | 1384 | if target == nil { | |
| 1372 | - | d.logControlSendFailure("no such session response", d.boundedControlSend(tr, serverError(protocol.ErrNoSuchSession, "no such session: "+request.Name))) | |
| 1385 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrNoSuchSession, Text: "no such session: " + request.Name}) | |
| 1373 | 1386 | return | |
| 1374 | 1387 | } | |
| 1375 | 1388 | if err := d.killSession(target, protocol.ReasonSessionKilled, true); err != nil { | |
| @@ -1377,11 +1390,13 @@ func (d *Daemon) handleKill(tr ports.ServerConnection, request protocol.Kill) { | |||
| 1377 | 1390 | // The exact session was not removed; a concurrent detach or token | |
| 1378 | 1391 | // resume invalidated the teardown snapshot. Report the retryable | |
| 1379 | 1392 | // outcome instead of a permanent delete failure. | |
| 1380 | - | d.logControlSendFailure("session delete retry response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, sessionKillRetryMessage))) | |
| 1393 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: sessionKillRetryMessage}) | |
| 1381 | 1394 | return | |
| 1382 | 1395 | } | |
| 1383 | - | d.logControlSendFailure("session delete failure response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, "deleting persisted session failed"))) | |
| 1396 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "deleting persisted session failed"}) | |
| 1397 | + | return | |
| 1384 | 1398 | } | |
| 1399 | + | send(protocol.KillResult{Outcome: protocol.KillSucceeded}) | |
| 1385 | 1400 | } | |
| 1386 | 1401 | ||
| 1387 | 1402 | // handleHello runs the attach handshake for direct package callers. Accepted | |
| @@ -1326,29 +1326,40 @@ func (d *Daemon) handleList(tr ports.ServerConnection) { | |||||
| 1326 | } | 1326 | } | ||
| 1327 | 1327 | ||||
| 1328 | // handleKill terminates the requested live session or stopped named session, | 1328 | // handleKill terminates the requested live session or stopped named session, | ||
| 1329 | - | // purges every session (KillAll), or stops the daemon (KillDaemon), and closes | 1329 | + | // purges every session (KillAll), or accepts daemon shutdown (KillDaemon). |
| 1330 | - | // the control connection; the resulting EOF is the client's success signal. | 1330 | + | // Every normally decoded request receives exactly one correlated result before |
| 1331 | - | // KillAll leaves the daemon active; KillDaemon is the explicit stop. | 1331 | + | // the control connection closes. KillAll leaves the daemon active. |
| 1332 | func (d *Daemon) handleKill(tr ports.ServerConnection, request protocol.Kill) { | 1332 | func (d *Daemon) handleKill(tr ports.ServerConnection, request protocol.Kill) { | ||
| 1333 | defer func() { _ = tr.Close() }() | 1333 | defer func() { _ = tr.Close() }() | ||
| 1334 | + | send := func(result protocol.KillResult) { | |||
| 1335 | + | result.RequestID = request.RequestID | |||
| 1336 | + | d.logKillResultSendFailure(result, d.boundedControlSend(tr, result)) | |||
| 1337 | + | } | |||
| 1334 | 1338 | ||||
| 1335 | switch request.Scope { | 1339 | switch request.Scope { | ||
| 1336 | case protocol.KillAll: | 1340 | case protocol.KillAll: | ||
| 1337 | result := d.purgeAllSessions(protocol.ReasonSessionKilled) | 1341 | result := d.purgeAllSessions(protocol.ReasonSessionKilled) | ||
| 1338 | if result.Superseded() { | 1342 | if result.Superseded() { | ||
| 1339 | - | d.logControlSendFailure("kill all superseded response", d.boundedControlSend(tr, serverError(protocol.ErrServerShutdown, "daemon is shutting down"))) | 1343 | + | send(protocol.KillResult{Outcome: protocol.KillOutcomeUnknown, Code: protocol.ErrServerShutdown, Text: "daemon is shutting down"}) |
| 1340 | return | 1344 | return | ||
| 1341 | } | 1345 | } | ||
| 1342 | if result.Failed() { | 1346 | if result.Failed() { | ||
| 1343 | - | d.logControlSendFailure("kill all failure response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, result.summary()))) | 1347 | + | failures := make([]protocol.KillFailure, 0, min(len(result.Failures), purgeSummaryFailureLimit)) |
| 1348 | + | for _, failure := range result.Failures[:min(len(result.Failures), purgeSummaryFailureLimit)] { | |||
| 1349 | + | failures = append(failures, protocol.KillFailure{Class: failure.Class.String(), Name: failure.Name, Text: failure.Err.Error()}) | |||
| 1350 | + | } | |||
| 1351 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: result.summary(), Failures: failures}) | |||
| 1352 | + | return | |||
| 1344 | } | 1353 | } | ||
| 1354 | + | send(protocol.KillResult{Outcome: protocol.KillSucceeded}) | |||
| 1345 | return | 1355 | return | ||
| 1346 | case protocol.KillDaemon: | 1356 | case protocol.KillDaemon: | ||
| 1357 | + | send(protocol.KillResult{Outcome: protocol.KillSucceeded}) | |||
| 1347 | d.shutdownAll(protocol.ReasonServerShutdown) | 1358 | d.shutdownAll(protocol.ReasonServerShutdown) | ||
| 1348 | return | 1359 | return | ||
| 1349 | case protocol.KillSession: | 1360 | case protocol.KillSession: | ||
| 1350 | default: | 1361 | default: | ||
| 1351 | - | d.logControlSendFailure("invalid kill scope response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, "invalid kill scope"))) | 1362 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "invalid kill scope"}) |
| 1352 | return | 1363 | return | ||
| 1353 | } | 1364 | } | ||
| 1354 | 1365 | ||||
| @@ -1361,15 +1372,17 @@ func (d *Daemon) handleKill(tr ports.ServerConnection, request protocol.Kill) { | |||||
| 1361 | // deletion order as live and offline purges. | 1372 | // deletion order as live and offline purges. | ||
| 1362 | if err := d.retryStoppedPurge(request.Name); err != nil { | 1373 | if err := d.retryStoppedPurge(request.Name); err != nil { | ||
| 1363 | d.log.Warn("deleting stopped session failed", "err", err, "session", request.Name) | 1374 | d.log.Warn("deleting stopped session failed", "err", err, "session", request.Name) | ||
| 1364 | - | d.logControlSendFailure("stopped delete failure response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, "deleting stopped session failed"))) | 1375 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "deleting stopped session failed"}) |
| 1376 | + | return | |||
| 1365 | } | 1377 | } | ||
| 1378 | + | send(protocol.KillResult{Outcome: protocol.KillSucceeded}) | |||
| 1366 | return | 1379 | return | ||
| 1367 | } | 1380 | } | ||
| 1368 | } | 1381 | } | ||
| 1369 | d.mu.Unlock() | 1382 | d.mu.Unlock() | ||
| 1370 | 1383 | ||||
| 1371 | if target == nil { | 1384 | if target == nil { | ||
| 1372 | - | d.logControlSendFailure("no such session response", d.boundedControlSend(tr, serverError(protocol.ErrNoSuchSession, "no such session: "+request.Name))) | 1385 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrNoSuchSession, Text: "no such session: " + request.Name}) |
| 1373 | return | 1386 | return | ||
| 1374 | } | 1387 | } | ||
| 1375 | if err := d.killSession(target, protocol.ReasonSessionKilled, true); err != nil { | 1388 | if err := d.killSession(target, protocol.ReasonSessionKilled, true); err != nil { | ||
| @@ -1377,11 +1390,13 @@ func (d *Daemon) handleKill(tr ports.ServerConnection, request protocol.Kill) { | |||||
| 1377 | // The exact session was not removed; a concurrent detach or token | 1390 | // The exact session was not removed; a concurrent detach or token | ||
| 1378 | // resume invalidated the teardown snapshot. Report the retryable | 1391 | // resume invalidated the teardown snapshot. Report the retryable | ||
| 1379 | // outcome instead of a permanent delete failure. | 1392 | // outcome instead of a permanent delete failure. | ||
| 1380 | - | d.logControlSendFailure("session delete retry response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, sessionKillRetryMessage))) | 1393 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: sessionKillRetryMessage}) |
| 1381 | return | 1394 | return | ||
| 1382 | } | 1395 | } | ||
| 1383 | - | d.logControlSendFailure("session delete failure response", d.boundedControlSend(tr, serverError(protocol.ErrInternal, "deleting persisted session failed"))) | 1396 | + | send(protocol.KillResult{Outcome: protocol.KillFailed, Code: protocol.ErrInternal, Text: "deleting persisted session failed"}) |
| 1397 | + | return | |||
| 1384 | } | 1398 | } | ||
| 1399 | + | send(protocol.KillResult{Outcome: protocol.KillSucceeded}) | |||
| 1385 | } | 1400 | } | ||
| 1386 | 1401 | ||||
| 1387 | // handleHello runs the attach handshake for direct package callers. Accepted | 1402 | // handleHello runs the attach handshake for direct package callers. Accepted | ||
A internal/usecase/daemon/kill_result_test.go +298 -0
| @@ -0,0 +1,298 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | // Explicit KillResult daemon contract. handleKill answers every normally | |
| 4 | + | // decoded request with exactly one correlated KillResult before the control | |
| 5 | + | // connection closes; a KillDaemon result is delivered before teardown begins. | |
| 6 | + | // These tests are deterministic (no sleeps) and bounded by awaitFrame timeouts. | |
| 7 | + | ||
| 8 | + | import ( | |
| 9 | + | "errors" | |
| 10 | + | "fmt" | |
| 11 | + | "io" | |
| 12 | + | "log/slog" | |
| 13 | + | "strconv" | |
| 14 | + | "sync" | |
| 15 | + | "testing" | |
| 16 | + | ||
| 17 | + | "github.com/stretchr/testify/mock" | |
| 18 | + | "github.com/stretchr/testify/require" | |
| 19 | + | ||
| 20 | + | "github.com/bnema/vev/internal/adapters/sessionwire" | |
| 21 | + | "github.com/bnema/vev/internal/domain" | |
| 22 | + | "github.com/bnema/vev/internal/protocol" | |
| 23 | + | "github.com/bnema/vev/internal/protocol/wire" | |
| 24 | + | ) | |
| 25 | + | ||
| 26 | + | func awaitKillResult(t *testing.T, sends chan wire.Envelope, requestID uint64) protocol.KillResult { | |
| 27 | + | t.Helper() | |
| 28 | + | frame := awaitFrame(t, sends, "KillResult") | |
| 29 | + | result, ok := decodeServerMessage(t, frame).(protocol.KillResult) | |
| 30 | + | require.True(t, ok, "expected a KillResult, got %T", decodeServerMessage(t, frame)) | |
| 31 | + | require.Equal(t, requestID, result.RequestID, "result must correlate to its request") | |
| 32 | + | return result | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | // requireNoFurtherSends proves the handler produced exactly one result. | |
| 36 | + | func requireNoFurtherSends(t *testing.T, sends chan wire.Envelope) { | |
| 37 | + | t.Helper() | |
| 38 | + | select { | |
| 39 | + | case extra := <-sends: | |
| 40 | + | t.Fatalf("unexpected extra send %s", envelopeMessageName(t, extra.Payload)) | |
| 41 | + | default: | |
| 42 | + | } | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | // TestHandleKillSessionSendsExactlyOneSucceededResult proves a direct kill of a | |
| 46 | + | // live session answers with one KillSucceeded result correlated to the request | |
| 47 | + | // and removes the session. | |
| 48 | + | func TestHandleKillSessionSendsExactlyOneSucceededResult(t *testing.T) { | |
| 49 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 50 | + | addControlSession(d, "work", "t_work", "p_work") | |
| 51 | + | ||
| 52 | + | tr, sends := newCapturingTransport(t) | |
| 53 | + | d.handleKill(tr, protocol.Kill{RequestID: 7, Scope: protocol.KillSession, Name: "work"}) | |
| 54 | + | ||
| 55 | + | result := awaitKillResult(t, sends, 7) | |
| 56 | + | require.Equal(t, protocol.KillResult{RequestID: 7, Outcome: protocol.KillSucceeded}, result) | |
| 57 | + | require.Zero(t, sessionCount(d), "a succeeded kill must remove the session") | |
| 58 | + | requireNoFurtherSends(t, sends) | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | // TestHandleKillSessionSendsDefiniteFailureResult proves a kill of a missing | |
| 62 | + | // session answers with one definite KillFailed result, not a close-as-success, | |
| 63 | + | // and leaves the registry untouched. | |
| 64 | + | func TestHandleKillSessionSendsDefiniteFailureResult(t *testing.T) { | |
| 65 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 66 | + | ||
| 67 | + | tr, sends := newCapturingTransport(t) | |
| 68 | + | d.handleKill(tr, protocol.Kill{RequestID: 9, Scope: protocol.KillSession, Name: "ghost"}) | |
| 69 | + | ||
| 70 | + | result := awaitKillResult(t, sends, 9) | |
| 71 | + | require.Equal(t, protocol.KillFailed, result.Outcome) | |
| 72 | + | require.Equal(t, protocol.ErrNoSuchSession, result.Code) | |
| 73 | + | require.Equal(t, "no such session: ghost", result.Text) | |
| 74 | + | requireNoFurtherSends(t, sends) | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | // TestHandleKillInvalidScopeSendsDefiniteFailureResult proves an invalid scope | |
| 78 | + | // still receives exactly one definite failure instead of a silent close. | |
| 79 | + | func TestHandleKillInvalidScopeSendsDefiniteFailureResult(t *testing.T) { | |
| 80 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 81 | + | ||
| 82 | + | tr, sends := newCapturingTransport(t) | |
| 83 | + | d.handleKill(tr, protocol.Kill{RequestID: 3, Scope: protocol.KillScope(9)}) | |
| 84 | + | ||
| 85 | + | result := awaitKillResult(t, sends, 3) | |
| 86 | + | require.Equal(t, protocol.KillFailed, result.Outcome) | |
| 87 | + | require.Equal(t, protocol.ErrInternal, result.Code) | |
| 88 | + | require.Equal(t, "invalid kill scope", result.Text) | |
| 89 | + | } | |
| 90 | + | ||
| 91 | + | // TestHandleKillAllEmptyDaemonSendsOneSucceededResult proves a successful | |
| 92 | + | // kill-all answers with one KillSucceeded result and leaves the daemon serving. | |
| 93 | + | func TestHandleKillAllEmptyDaemonSendsOneSucceededResult(t *testing.T) { | |
| 94 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 95 | + | ||
| 96 | + | tr, sends := newCapturingTransport(t) | |
| 97 | + | d.handleKill(tr, protocol.Kill{RequestID: 11, Scope: protocol.KillAll}) | |
| 98 | + | ||
| 99 | + | result := awaitKillResult(t, sends, 11) | |
| 100 | + | require.Equal(t, protocol.KillSucceeded, result.Outcome) | |
| 101 | + | select { | |
| 102 | + | case <-d.done: | |
| 103 | + | t.Fatal("kill-all must not end the daemon") | |
| 104 | + | default: | |
| 105 | + | } | |
| 106 | + | requireNoFurtherSends(t, sends) | |
| 107 | + | } | |
| 108 | + | ||
| 109 | + | // TestHandleKillAllReportsBoundedStructuredPartialFailures proves a kill-all | |
| 110 | + | // whose stopped records cannot be purged answers with one definite KillFailed | |
| 111 | + | // result carrying the total count plus at most purgeSummaryFailureLimit | |
| 112 | + | // structured failures, so a control response stays bounded. | |
| 113 | + | func TestHandleKillAllReportsBoundedStructuredPartialFailures(t *testing.T) { | |
| 114 | + | const total = purgeSummaryFailureLimit + 3 | |
| 115 | + | ||
| 116 | + | fail := make(map[domain.IncarnationID]error, total) | |
| 117 | + | repository := newSelectivePurgeRepository(fail) | |
| 118 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 119 | + | WithSnapshotRepository(repository)(d) | |
| 120 | + | store, _ := newMockStore(t) | |
| 121 | + | WithStore(t, store)(d) | |
| 122 | + | ||
| 123 | + | for i := 0; i < total; i++ { | |
| 124 | + | incarnation := domain.IncarnationID{byte(i + 1)} | |
| 125 | + | fail[incarnation] = errors.New("delete failed") | |
| 126 | + | record := domain.CatalogueRecord{Name: fmt.Sprintf("s%02d", i), IncarnationID: incarnation, CreatedAt: int64(i + 1)} | |
| 127 | + | require.NoError(t, d.catalogue.Create(record)) | |
| 128 | + | d.inactive[record.Name] = inactiveSessionFromRecord(record, protocol.SessionDown, nil) | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | tr, sends := newCapturingTransport(t) | |
| 132 | + | d.handleKill(tr, protocol.Kill{RequestID: 13, Scope: protocol.KillAll}) | |
| 133 | + | ||
| 134 | + | result := awaitKillResult(t, sends, 13) | |
| 135 | + | require.Equal(t, protocol.KillFailed, result.Outcome) | |
| 136 | + | require.Equal(t, protocol.ErrInternal, result.Code) | |
| 137 | + | require.Len(t, result.Failures, purgeSummaryFailureLimit, "wire detail must be bounded") | |
| 138 | + | require.Contains(t, result.Text, fmt.Sprintf("%d record(s) could not be purged", total)) | |
| 139 | + | require.Contains(t, result.Text, "more omitted") | |
| 140 | + | requireNoFurtherSends(t, sends) | |
| 141 | + | } | |
| 142 | + | ||
| 143 | + | // killOrderTransport records the order of the KillResult send and the control | |
| 144 | + | // close, and whether the daemon's shutdown signal was already closed when the | |
| 145 | + | // result was written. | |
| 146 | + | type killOrderTransport struct { | |
| 147 | + | mu sync.Mutex | |
| 148 | + | order []string | |
| 149 | + | resultBeforeTeardown bool | |
| 150 | + | } | |
| 151 | + | ||
| 152 | + | func (r *killOrderTransport) record(name string) { | |
| 153 | + | r.mu.Lock() | |
| 154 | + | r.order = append(r.order, name) | |
| 155 | + | r.mu.Unlock() | |
| 156 | + | } | |
| 157 | + | ||
| 158 | + | func (r *killOrderTransport) snapshot() ([]string, bool) { | |
| 159 | + | r.mu.Lock() | |
| 160 | + | defer r.mu.Unlock() | |
| 161 | + | return append([]string(nil), r.order...), r.resultBeforeTeardown | |
| 162 | + | } | |
| 163 | + | ||
| 164 | + | func newKillOrderTransport(t *testing.T, teardown <-chan struct{}) (*mockServerConnection, *killOrderTransport) { | |
| 165 | + | t.Helper() | |
| 166 | + | recorder := &killOrderTransport{} | |
| 167 | + | tr := newMockServerConnection(t) | |
| 168 | + | tr.EXPECT().Send(mock.Anything).RunAndReturn(func(f wire.Envelope) error { | |
| 169 | + | name := envelopeMessageName(t, f.Payload) | |
| 170 | + | if name == "KillResult" { | |
| 171 | + | recorder.mu.Lock() | |
| 172 | + | recorder.order = append(recorder.order, "result") | |
| 173 | + | select { | |
| 174 | + | case <-teardown: | |
| 175 | + | default: | |
| 176 | + | recorder.resultBeforeTeardown = true | |
| 177 | + | } | |
| 178 | + | recorder.mu.Unlock() | |
| 179 | + | return nil | |
| 180 | + | } | |
| 181 | + | recorder.record(name) | |
| 182 | + | return nil | |
| 183 | + | }).Maybe() | |
| 184 | + | tr.EXPECT().Close().RunAndReturn(func() error { | |
| 185 | + | recorder.record("close") | |
| 186 | + | return nil | |
| 187 | + | }).Maybe() | |
| 188 | + | return tr, recorder | |
| 189 | + | } | |
| 190 | + | ||
| 191 | + | // TestHandleKillDaemonSendsResultBeforeTeardownAndClose proves the explicit | |
| 192 | + | // daemon-stop result is delivered before shutdown begins and before the control | |
| 193 | + | // connection closes, so a client can observe success instead of inferring it | |
| 194 | + | // from a close. | |
| 195 | + | func TestHandleKillDaemonSendsResultBeforeTeardownAndClose(t *testing.T) { | |
| 196 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 197 | + | ||
| 198 | + | tr, recorder := newKillOrderTransport(t, d.done) | |
| 199 | + | d.handleKill(tr, protocol.Kill{RequestID: 42, Scope: protocol.KillDaemon}) | |
| 200 | + | ||
| 201 | + | order, beforeTeardown := recorder.snapshot() | |
| 202 | + | require.Equal(t, []string{"result", "close"}, order) | |
| 203 | + | require.True(t, beforeTeardown, "the daemon-stop result must be sent before teardown begins") | |
| 204 | + | select { | |
| 205 | + | case <-d.done: | |
| 206 | + | default: | |
| 207 | + | t.Fatal("KillDaemon must end the daemon") | |
| 208 | + | } | |
| 209 | + | } | |
| 210 | + | ||
| 211 | + | // recordFailTransport records every sent envelope then fails, so a test can | |
| 212 | + | // inspect the exact result an undeliverable control send attempted. | |
| 213 | + | type recordFailTransport struct { | |
| 214 | + | mu sync.Mutex | |
| 215 | + | sent []wire.Envelope | |
| 216 | + | err error | |
| 217 | + | } | |
| 218 | + | ||
| 219 | + | func (t *recordFailTransport) Send(envelope wire.Envelope) error { | |
| 220 | + | t.mu.Lock() | |
| 221 | + | t.sent = append(t.sent, envelope) | |
| 222 | + | t.mu.Unlock() | |
| 223 | + | return t.err | |
| 224 | + | } | |
| 225 | + | ||
| 226 | + | func (*recordFailTransport) Recv() (wire.Envelope, error) { return wire.Envelope{}, io.EOF } | |
| 227 | + | func (*recordFailTransport) Close() error { return nil } | |
| 228 | + | ||
| 229 | + | func (t *recordFailTransport) payloads() []wire.Envelope { | |
| 230 | + | t.mu.Lock() | |
| 231 | + | defer t.mu.Unlock() | |
| 232 | + | return append([]wire.Envelope(nil), t.sent...) | |
| 233 | + | } | |
| 234 | + | ||
| 235 | + | // TestHandleKillAllSupersededByDaemonShutdownSendsOneUnknownOutcome proves a | |
| 236 | + | // KillAll that an explicit daemon shutdown superseded answers with exactly one | |
| 237 | + | // correlated KillOutcomeUnknown carrying ErrServerShutdown, records the outcome | |
| 238 | + | // and code in the send-failure log, and sends nothing further (no success and no | |
| 239 | + | // second result). | |
| 240 | + | func TestHandleKillAllSupersededByDaemonShutdownSendsOneUnknownOutcome(t *testing.T) { | |
| 241 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 242 | + | var logged syncedLogBuffer | |
| 243 | + | d.log = slog.New(slog.NewTextHandler(&logged, nil)) | |
| 244 | + | ||
| 245 | + | // An explicit stop already owns the lifecycle set, so the purge defers. | |
| 246 | + | d.mu.Lock() | |
| 247 | + | d.closing = true | |
| 248 | + | d.mu.Unlock() | |
| 249 | + | ||
| 250 | + | recorder := &recordFailTransport{err: errors.New("client gone")} | |
| 251 | + | d.handleKill(&rawServerConnection{raw: recorder}, protocol.Kill{RequestID: 5, Scope: protocol.KillAll}) | |
| 252 | + | ||
| 253 | + | payloads := recorder.payloads() | |
| 254 | + | require.Len(t, payloads, 1, "a superseded kill-all must attempt exactly one result") | |
| 255 | + | message, err := sessionwire.DecodeServerEnvelope(payloads[0].Payload) | |
| 256 | + | require.NoError(t, err) | |
| 257 | + | result, ok := message.(protocol.KillResult) | |
| 258 | + | require.True(t, ok, "expected KillResult, got %T", message) | |
| 259 | + | require.Equal(t, protocol.KillResult{RequestID: 5, Outcome: protocol.KillOutcomeUnknown, Code: protocol.ErrServerShutdown, Text: "daemon is shutting down"}, result) | |
| 260 | + | ||
| 261 | + | require.Contains(t, logged.String(), "kill result") | |
| 262 | + | require.Contains(t, logged.String(), "client gone") | |
| 263 | + | require.Contains(t, logged.String(), "outcome="+strconv.Itoa(int(protocol.KillOutcomeUnknown)), "the send-failure log must name the unobserved outcome") | |
| 264 | + | require.Contains(t, logged.String(), "code="+strconv.Itoa(int(protocol.ErrServerShutdown)), "the send-failure log must name the unobserved code") | |
| 265 | + | } | |
| 266 | + | ||
| 267 | + | // TestKillAllUndeliverableResultIsLogged proves an explicit partial-failure | |
| 268 | + | // result that cannot be delivered is logged (never silently closing as | |
| 269 | + | // success) and still carries the bounded structured failures. | |
| 270 | + | func TestKillAllUndeliverableResultIsLogged(t *testing.T) { | |
| 271 | + | repository := newSelectivePurgeRepository(map[domain.IncarnationID]error{{2}: errors.New("delete failed")}) | |
| 272 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 273 | + | var logged syncedLogBuffer | |
| 274 | + | d.log = slog.New(slog.NewTextHandler(&logged, nil)) | |
| 275 | + | WithSnapshotRepository(repository)(d) | |
| 276 | + | store, _ := newMockStore(t) | |
| 277 | + | WithStore(t, store)(d) | |
| 278 | + | ||
| 279 | + | record := domain.CatalogueRecord{Name: "stopped", IncarnationID: domain.IncarnationID{2}, CreatedAt: 7} | |
| 280 | + | require.NoError(t, d.catalogue.Create(record)) | |
| 281 | + | d.inactive["stopped"] = inactiveSessionFromRecord(record, protocol.SessionDown, nil) | |
| 282 | + | ||
| 283 | + | recorder := &recordFailTransport{err: errors.New("client gone")} | |
| 284 | + | d.handleKill(&rawServerConnection{raw: recorder}, protocol.Kill{RequestID: 1, Scope: protocol.KillAll}) | |
| 285 | + | ||
| 286 | + | require.Contains(t, logged.String(), "kill result") | |
| 287 | + | require.Contains(t, logged.String(), "client gone") | |
| 288 | + | ||
| 289 | + | payloads := recorder.payloads() | |
| 290 | + | require.Len(t, payloads, 1, "exactly one result is attempted") | |
| 291 | + | message, err := sessionwire.DecodeServerEnvelope(payloads[0].Payload) | |
| 292 | + | require.NoError(t, err) | |
| 293 | + | result, ok := message.(protocol.KillResult) | |
| 294 | + | require.True(t, ok, "expected KillResult, got %T", message) | |
| 295 | + | require.Equal(t, protocol.KillFailed, result.Outcome) | |
| 296 | + | require.Len(t, result.Failures, 1) | |
| 297 | + | require.Equal(t, "stopped", result.Failures[0].Name) | |
| 298 | + | } | |
| @@ -0,0 +1,298 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | // Explicit KillResult daemon contract. handleKill answers every normally | |||
| 4 | + | // decoded request with exactly one correlated KillResult before the control | |||
| 5 | + | // connection closes; a KillDaemon result is delivered before teardown begins. | |||
| 6 | + | // These tests are deterministic (no sleeps) and bounded by awaitFrame timeouts. | |||
| 7 | + | ||||
| 8 | + | import ( | |||
| 9 | + | "errors" | |||
| 10 | + | "fmt" | |||
| 11 | + | "io" | |||
| 12 | + | "log/slog" | |||
| 13 | + | "strconv" | |||
| 14 | + | "sync" | |||
| 15 | + | "testing" | |||
| 16 | + | ||||
| 17 | + | "github.com/stretchr/testify/mock" | |||
| 18 | + | "github.com/stretchr/testify/require" | |||
| 19 | + | ||||
| 20 | + | "github.com/bnema/vev/internal/adapters/sessionwire" | |||
| 21 | + | "github.com/bnema/vev/internal/domain" | |||
| 22 | + | "github.com/bnema/vev/internal/protocol" | |||
| 23 | + | "github.com/bnema/vev/internal/protocol/wire" | |||
| 24 | + | ) | |||
| 25 | + | ||||
| 26 | + | func awaitKillResult(t *testing.T, sends chan wire.Envelope, requestID uint64) protocol.KillResult { | |||
| 27 | + | t.Helper() | |||
| 28 | + | frame := awaitFrame(t, sends, "KillResult") | |||
| 29 | + | result, ok := decodeServerMessage(t, frame).(protocol.KillResult) | |||
| 30 | + | require.True(t, ok, "expected a KillResult, got %T", decodeServerMessage(t, frame)) | |||
| 31 | + | require.Equal(t, requestID, result.RequestID, "result must correlate to its request") | |||
| 32 | + | return result | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | // requireNoFurtherSends proves the handler produced exactly one result. | |||
| 36 | + | func requireNoFurtherSends(t *testing.T, sends chan wire.Envelope) { | |||
| 37 | + | t.Helper() | |||
| 38 | + | select { | |||
| 39 | + | case extra := <-sends: | |||
| 40 | + | t.Fatalf("unexpected extra send %s", envelopeMessageName(t, extra.Payload)) | |||
| 41 | + | default: | |||
| 42 | + | } | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | // TestHandleKillSessionSendsExactlyOneSucceededResult proves a direct kill of a | |||
| 46 | + | // live session answers with one KillSucceeded result correlated to the request | |||
| 47 | + | // and removes the session. | |||
| 48 | + | func TestHandleKillSessionSendsExactlyOneSucceededResult(t *testing.T) { | |||
| 49 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 50 | + | addControlSession(d, "work", "t_work", "p_work") | |||
| 51 | + | ||||
| 52 | + | tr, sends := newCapturingTransport(t) | |||
| 53 | + | d.handleKill(tr, protocol.Kill{RequestID: 7, Scope: protocol.KillSession, Name: "work"}) | |||
| 54 | + | ||||
| 55 | + | result := awaitKillResult(t, sends, 7) | |||
| 56 | + | require.Equal(t, protocol.KillResult{RequestID: 7, Outcome: protocol.KillSucceeded}, result) | |||
| 57 | + | require.Zero(t, sessionCount(d), "a succeeded kill must remove the session") | |||
| 58 | + | requireNoFurtherSends(t, sends) | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | // TestHandleKillSessionSendsDefiniteFailureResult proves a kill of a missing | |||
| 62 | + | // session answers with one definite KillFailed result, not a close-as-success, | |||
| 63 | + | // and leaves the registry untouched. | |||
| 64 | + | func TestHandleKillSessionSendsDefiniteFailureResult(t *testing.T) { | |||
| 65 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 66 | + | ||||
| 67 | + | tr, sends := newCapturingTransport(t) | |||
| 68 | + | d.handleKill(tr, protocol.Kill{RequestID: 9, Scope: protocol.KillSession, Name: "ghost"}) | |||
| 69 | + | ||||
| 70 | + | result := awaitKillResult(t, sends, 9) | |||
| 71 | + | require.Equal(t, protocol.KillFailed, result.Outcome) | |||
| 72 | + | require.Equal(t, protocol.ErrNoSuchSession, result.Code) | |||
| 73 | + | require.Equal(t, "no such session: ghost", result.Text) | |||
| 74 | + | requireNoFurtherSends(t, sends) | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | // TestHandleKillInvalidScopeSendsDefiniteFailureResult proves an invalid scope | |||
| 78 | + | // still receives exactly one definite failure instead of a silent close. | |||
| 79 | + | func TestHandleKillInvalidScopeSendsDefiniteFailureResult(t *testing.T) { | |||
| 80 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 81 | + | ||||
| 82 | + | tr, sends := newCapturingTransport(t) | |||
| 83 | + | d.handleKill(tr, protocol.Kill{RequestID: 3, Scope: protocol.KillScope(9)}) | |||
| 84 | + | ||||
| 85 | + | result := awaitKillResult(t, sends, 3) | |||
| 86 | + | require.Equal(t, protocol.KillFailed, result.Outcome) | |||
| 87 | + | require.Equal(t, protocol.ErrInternal, result.Code) | |||
| 88 | + | require.Equal(t, "invalid kill scope", result.Text) | |||
| 89 | + | } | |||
| 90 | + | ||||
| 91 | + | // TestHandleKillAllEmptyDaemonSendsOneSucceededResult proves a successful | |||
| 92 | + | // kill-all answers with one KillSucceeded result and leaves the daemon serving. | |||
| 93 | + | func TestHandleKillAllEmptyDaemonSendsOneSucceededResult(t *testing.T) { | |||
| 94 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 95 | + | ||||
| 96 | + | tr, sends := newCapturingTransport(t) | |||
| 97 | + | d.handleKill(tr, protocol.Kill{RequestID: 11, Scope: protocol.KillAll}) | |||
| 98 | + | ||||
| 99 | + | result := awaitKillResult(t, sends, 11) | |||
| 100 | + | require.Equal(t, protocol.KillSucceeded, result.Outcome) | |||
| 101 | + | select { | |||
| 102 | + | case <-d.done: | |||
| 103 | + | t.Fatal("kill-all must not end the daemon") | |||
| 104 | + | default: | |||
| 105 | + | } | |||
| 106 | + | requireNoFurtherSends(t, sends) | |||
| 107 | + | } | |||
| 108 | + | ||||
| 109 | + | // TestHandleKillAllReportsBoundedStructuredPartialFailures proves a kill-all | |||
| 110 | + | // whose stopped records cannot be purged answers with one definite KillFailed | |||
| 111 | + | // result carrying the total count plus at most purgeSummaryFailureLimit | |||
| 112 | + | // structured failures, so a control response stays bounded. | |||
| 113 | + | func TestHandleKillAllReportsBoundedStructuredPartialFailures(t *testing.T) { | |||
| 114 | + | const total = purgeSummaryFailureLimit + 3 | |||
| 115 | + | ||||
| 116 | + | fail := make(map[domain.IncarnationID]error, total) | |||
| 117 | + | repository := newSelectivePurgeRepository(fail) | |||
| 118 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 119 | + | WithSnapshotRepository(repository)(d) | |||
| 120 | + | store, _ := newMockStore(t) | |||
| 121 | + | WithStore(t, store)(d) | |||
| 122 | + | ||||
| 123 | + | for i := 0; i < total; i++ { | |||
| 124 | + | incarnation := domain.IncarnationID{byte(i + 1)} | |||
| 125 | + | fail[incarnation] = errors.New("delete failed") | |||
| 126 | + | record := domain.CatalogueRecord{Name: fmt.Sprintf("s%02d", i), IncarnationID: incarnation, CreatedAt: int64(i + 1)} | |||
| 127 | + | require.NoError(t, d.catalogue.Create(record)) | |||
| 128 | + | d.inactive[record.Name] = inactiveSessionFromRecord(record, protocol.SessionDown, nil) | |||
| 129 | + | } | |||
| 130 | + | ||||
| 131 | + | tr, sends := newCapturingTransport(t) | |||
| 132 | + | d.handleKill(tr, protocol.Kill{RequestID: 13, Scope: protocol.KillAll}) | |||
| 133 | + | ||||
| 134 | + | result := awaitKillResult(t, sends, 13) | |||
| 135 | + | require.Equal(t, protocol.KillFailed, result.Outcome) | |||
| 136 | + | require.Equal(t, protocol.ErrInternal, result.Code) | |||
| 137 | + | require.Len(t, result.Failures, purgeSummaryFailureLimit, "wire detail must be bounded") | |||
| 138 | + | require.Contains(t, result.Text, fmt.Sprintf("%d record(s) could not be purged", total)) | |||
| 139 | + | require.Contains(t, result.Text, "more omitted") | |||
| 140 | + | requireNoFurtherSends(t, sends) | |||
| 141 | + | } | |||
| 142 | + | ||||
| 143 | + | // killOrderTransport records the order of the KillResult send and the control | |||
| 144 | + | // close, and whether the daemon's shutdown signal was already closed when the | |||
| 145 | + | // result was written. | |||
| 146 | + | type killOrderTransport struct { | |||
| 147 | + | mu sync.Mutex | |||
| 148 | + | order []string | |||
| 149 | + | resultBeforeTeardown bool | |||
| 150 | + | } | |||
| 151 | + | ||||
| 152 | + | func (r *killOrderTransport) record(name string) { | |||
| 153 | + | r.mu.Lock() | |||
| 154 | + | r.order = append(r.order, name) | |||
| 155 | + | r.mu.Unlock() | |||
| 156 | + | } | |||
| 157 | + | ||||
| 158 | + | func (r *killOrderTransport) snapshot() ([]string, bool) { | |||
| 159 | + | r.mu.Lock() | |||
| 160 | + | defer r.mu.Unlock() | |||
| 161 | + | return append([]string(nil), r.order...), r.resultBeforeTeardown | |||
| 162 | + | } | |||
| 163 | + | ||||
| 164 | + | func newKillOrderTransport(t *testing.T, teardown <-chan struct{}) (*mockServerConnection, *killOrderTransport) { | |||
| 165 | + | t.Helper() | |||
| 166 | + | recorder := &killOrderTransport{} | |||
| 167 | + | tr := newMockServerConnection(t) | |||
| 168 | + | tr.EXPECT().Send(mock.Anything).RunAndReturn(func(f wire.Envelope) error { | |||
| 169 | + | name := envelopeMessageName(t, f.Payload) | |||
| 170 | + | if name == "KillResult" { | |||
| 171 | + | recorder.mu.Lock() | |||
| 172 | + | recorder.order = append(recorder.order, "result") | |||
| 173 | + | select { | |||
| 174 | + | case <-teardown: | |||
| 175 | + | default: | |||
| 176 | + | recorder.resultBeforeTeardown = true | |||
| 177 | + | } | |||
| 178 | + | recorder.mu.Unlock() | |||
| 179 | + | return nil | |||
| 180 | + | } | |||
| 181 | + | recorder.record(name) | |||
| 182 | + | return nil | |||
| 183 | + | }).Maybe() | |||
| 184 | + | tr.EXPECT().Close().RunAndReturn(func() error { | |||
| 185 | + | recorder.record("close") | |||
| 186 | + | return nil | |||
| 187 | + | }).Maybe() | |||
| 188 | + | return tr, recorder | |||
| 189 | + | } | |||
| 190 | + | ||||
| 191 | + | // TestHandleKillDaemonSendsResultBeforeTeardownAndClose proves the explicit | |||
| 192 | + | // daemon-stop result is delivered before shutdown begins and before the control | |||
| 193 | + | // connection closes, so a client can observe success instead of inferring it | |||
| 194 | + | // from a close. | |||
| 195 | + | func TestHandleKillDaemonSendsResultBeforeTeardownAndClose(t *testing.T) { | |||
| 196 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 197 | + | ||||
| 198 | + | tr, recorder := newKillOrderTransport(t, d.done) | |||
| 199 | + | d.handleKill(tr, protocol.Kill{RequestID: 42, Scope: protocol.KillDaemon}) | |||
| 200 | + | ||||
| 201 | + | order, beforeTeardown := recorder.snapshot() | |||
| 202 | + | require.Equal(t, []string{"result", "close"}, order) | |||
| 203 | + | require.True(t, beforeTeardown, "the daemon-stop result must be sent before teardown begins") | |||
| 204 | + | select { | |||
| 205 | + | case <-d.done: | |||
| 206 | + | default: | |||
| 207 | + | t.Fatal("KillDaemon must end the daemon") | |||
| 208 | + | } | |||
| 209 | + | } | |||
| 210 | + | ||||
| 211 | + | // recordFailTransport records every sent envelope then fails, so a test can | |||
| 212 | + | // inspect the exact result an undeliverable control send attempted. | |||
| 213 | + | type recordFailTransport struct { | |||
| 214 | + | mu sync.Mutex | |||
| 215 | + | sent []wire.Envelope | |||
| 216 | + | err error | |||
| 217 | + | } | |||
| 218 | + | ||||
| 219 | + | func (t *recordFailTransport) Send(envelope wire.Envelope) error { | |||
| 220 | + | t.mu.Lock() | |||
| 221 | + | t.sent = append(t.sent, envelope) | |||
| 222 | + | t.mu.Unlock() | |||
| 223 | + | return t.err | |||
| 224 | + | } | |||
| 225 | + | ||||
| 226 | + | func (*recordFailTransport) Recv() (wire.Envelope, error) { return wire.Envelope{}, io.EOF } | |||
| 227 | + | func (*recordFailTransport) Close() error { return nil } | |||
| 228 | + | ||||
| 229 | + | func (t *recordFailTransport) payloads() []wire.Envelope { | |||
| 230 | + | t.mu.Lock() | |||
| 231 | + | defer t.mu.Unlock() | |||
| 232 | + | return append([]wire.Envelope(nil), t.sent...) | |||
| 233 | + | } | |||
| 234 | + | ||||
| 235 | + | // TestHandleKillAllSupersededByDaemonShutdownSendsOneUnknownOutcome proves a | |||
| 236 | + | // KillAll that an explicit daemon shutdown superseded answers with exactly one | |||
| 237 | + | // correlated KillOutcomeUnknown carrying ErrServerShutdown, records the outcome | |||
| 238 | + | // and code in the send-failure log, and sends nothing further (no success and no | |||
| 239 | + | // second result). | |||
| 240 | + | func TestHandleKillAllSupersededByDaemonShutdownSendsOneUnknownOutcome(t *testing.T) { | |||
| 241 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 242 | + | var logged syncedLogBuffer | |||
| 243 | + | d.log = slog.New(slog.NewTextHandler(&logged, nil)) | |||
| 244 | + | ||||
| 245 | + | // An explicit stop already owns the lifecycle set, so the purge defers. | |||
| 246 | + | d.mu.Lock() | |||
| 247 | + | d.closing = true | |||
| 248 | + | d.mu.Unlock() | |||
| 249 | + | ||||
| 250 | + | recorder := &recordFailTransport{err: errors.New("client gone")} | |||
| 251 | + | d.handleKill(&rawServerConnection{raw: recorder}, protocol.Kill{RequestID: 5, Scope: protocol.KillAll}) | |||
| 252 | + | ||||
| 253 | + | payloads := recorder.payloads() | |||
| 254 | + | require.Len(t, payloads, 1, "a superseded kill-all must attempt exactly one result") | |||
| 255 | + | message, err := sessionwire.DecodeServerEnvelope(payloads[0].Payload) | |||
| 256 | + | require.NoError(t, err) | |||
| 257 | + | result, ok := message.(protocol.KillResult) | |||
| 258 | + | require.True(t, ok, "expected KillResult, got %T", message) | |||
| 259 | + | require.Equal(t, protocol.KillResult{RequestID: 5, Outcome: protocol.KillOutcomeUnknown, Code: protocol.ErrServerShutdown, Text: "daemon is shutting down"}, result) | |||
| 260 | + | ||||
| 261 | + | require.Contains(t, logged.String(), "kill result") | |||
| 262 | + | require.Contains(t, logged.String(), "client gone") | |||
| 263 | + | require.Contains(t, logged.String(), "outcome="+strconv.Itoa(int(protocol.KillOutcomeUnknown)), "the send-failure log must name the unobserved outcome") | |||
| 264 | + | require.Contains(t, logged.String(), "code="+strconv.Itoa(int(protocol.ErrServerShutdown)), "the send-failure log must name the unobserved code") | |||
| 265 | + | } | |||
| 266 | + | ||||
| 267 | + | // TestKillAllUndeliverableResultIsLogged proves an explicit partial-failure | |||
| 268 | + | // result that cannot be delivered is logged (never silently closing as | |||
| 269 | + | // success) and still carries the bounded structured failures. | |||
| 270 | + | func TestKillAllUndeliverableResultIsLogged(t *testing.T) { | |||
| 271 | + | repository := newSelectivePurgeRepository(map[domain.IncarnationID]error{{2}: errors.New("delete failed")}) | |||
| 272 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 273 | + | var logged syncedLogBuffer | |||
| 274 | + | d.log = slog.New(slog.NewTextHandler(&logged, nil)) | |||
| 275 | + | WithSnapshotRepository(repository)(d) | |||
| 276 | + | store, _ := newMockStore(t) | |||
| 277 | + | WithStore(t, store)(d) | |||
| 278 | + | ||||
| 279 | + | record := domain.CatalogueRecord{Name: "stopped", IncarnationID: domain.IncarnationID{2}, CreatedAt: 7} | |||
| 280 | + | require.NoError(t, d.catalogue.Create(record)) | |||
| 281 | + | d.inactive["stopped"] = inactiveSessionFromRecord(record, protocol.SessionDown, nil) | |||
| 282 | + | ||||
| 283 | + | recorder := &recordFailTransport{err: errors.New("client gone")} | |||
| 284 | + | d.handleKill(&rawServerConnection{raw: recorder}, protocol.Kill{RequestID: 1, Scope: protocol.KillAll}) | |||
| 285 | + | ||||
| 286 | + | require.Contains(t, logged.String(), "kill result") | |||
| 287 | + | require.Contains(t, logged.String(), "client gone") | |||
| 288 | + | ||||
| 289 | + | payloads := recorder.payloads() | |||
| 290 | + | require.Len(t, payloads, 1, "exactly one result is attempted") | |||
| 291 | + | message, err := sessionwire.DecodeServerEnvelope(payloads[0].Payload) | |||
| 292 | + | require.NoError(t, err) | |||
| 293 | + | result, ok := message.(protocol.KillResult) | |||
| 294 | + | require.True(t, ok, "expected KillResult, got %T", message) | |||
| 295 | + | require.Equal(t, protocol.KillFailed, result.Outcome) | |||
| 296 | + | require.Len(t, result.Failures, 1) | |||
| 297 | + | require.Equal(t, "stopped", result.Failures[0].Name) | |||
| 298 | + | } | |||
M internal/usecase/daemon/purge_test.go +8 -4
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "errors" | |
| 7 | 7 | "fmt" | |
| 8 | 8 | "log/slog" | |
| 9 | + | "strconv" | |
| 9 | 10 | "strings" | |
| 10 | 11 | "sync" | |
| 11 | 12 | "sync/atomic" | |
| @@ -791,8 +792,9 @@ func TestPurgeAllSupersededByExplicitShutdownPreservesStopAuthority(t *testing.T | |||
| 791 | 792 | d.waitNotifies() | |
| 792 | 793 | } | |
| 793 | 794 | ||
| 794 | - | // TestPurgeAllLogsFailedControlSend proves a purge failure that cannot be | |
| 795 | - | // delivered is logged, so a client observing EOF cannot silently infer success. | |
| 795 | + | // TestPurgeAllLogsFailedControlSend proves a purge failure result that cannot be | |
| 796 | + | // delivered is logged, so a client that never observes the explicit correlated | |
| 797 | + | // KillResult cannot silently infer success from a close. | |
| 796 | 798 | func TestPurgeAllLogsFailedControlSend(t *testing.T) { | |
| 797 | 799 | repository := newSelectivePurgeRepository(map[domain.IncarnationID]error{{2}: errors.New("delete failed")}) | |
| 798 | 800 | d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{}) | |
| @@ -809,10 +811,12 @@ func TestPurgeAllLogsFailedControlSend(t *testing.T) { | |||
| 809 | 811 | tr := newMockServerConnection(t) | |
| 810 | 812 | tr.EXPECT().Send(mock.Anything).Return(errors.New("client gone")).Maybe() | |
| 811 | 813 | tr.EXPECT().Close().Return(nil).Maybe() | |
| 812 | - | d.handleKill(tr, protocol.Kill{Scope: protocol.KillAll}) | |
| 814 | + | d.handleKill(tr, protocol.Kill{RequestID: 1, Scope: protocol.KillAll}) | |
| 813 | 815 | ||
| 814 | - | require.Contains(t, logged.String(), "kill all failure response") | |
| 816 | + | require.Contains(t, logged.String(), "kill result") | |
| 815 | 817 | require.Contains(t, logged.String(), "client gone") | |
| 818 | + | require.Contains(t, logged.String(), "outcome="+strconv.Itoa(int(protocol.KillFailed)), "the send-failure log must name the unobserved outcome") | |
| 819 | + | require.Contains(t, logged.String(), "code="+strconv.Itoa(int(protocol.ErrInternal)), "the send-failure log must name the unobserved code") | |
| 816 | 820 | } | |
| 817 | 821 | ||
| 818 | 822 | // TestPurgeResultSummaryBoundsWireDetail proves the partial-failure summary is | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "errors" | 6 | "errors" | ||
| 7 | "fmt" | 7 | "fmt" | ||
| 8 | "log/slog" | 8 | "log/slog" | ||
| 9 | + | "strconv" | |||
| 9 | "strings" | 10 | "strings" | ||
| 10 | "sync" | 11 | "sync" | ||
| 11 | "sync/atomic" | 12 | "sync/atomic" | ||
| @@ -791,8 +792,9 @@ func TestPurgeAllSupersededByExplicitShutdownPreservesStopAuthority(t *testing.T | |||||
| 791 | d.waitNotifies() | 792 | d.waitNotifies() | ||
| 792 | } | 793 | } | ||
| 793 | 794 | ||||
| 794 | - | // TestPurgeAllLogsFailedControlSend proves a purge failure that cannot be | 795 | + | // TestPurgeAllLogsFailedControlSend proves a purge failure result that cannot be |
| 795 | - | // delivered is logged, so a client observing EOF cannot silently infer success. | 796 | + | // delivered is logged, so a client that never observes the explicit correlated |
| 797 | + | // KillResult cannot silently infer success from a close. | |||
| 796 | func TestPurgeAllLogsFailedControlSend(t *testing.T) { | 798 | func TestPurgeAllLogsFailedControlSend(t *testing.T) { | ||
| 797 | repository := newSelectivePurgeRepository(map[domain.IncarnationID]error{{2}: errors.New("delete failed")}) | 799 | repository := newSelectivePurgeRepository(map[domain.IncarnationID]error{{2}: errors.New("delete failed")}) | ||
| 798 | d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{}) | 800 | d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{}) | ||
| @@ -809,10 +811,12 @@ func TestPurgeAllLogsFailedControlSend(t *testing.T) { | |||||
| 809 | tr := newMockServerConnection(t) | 811 | tr := newMockServerConnection(t) | ||
| 810 | tr.EXPECT().Send(mock.Anything).Return(errors.New("client gone")).Maybe() | 812 | tr.EXPECT().Send(mock.Anything).Return(errors.New("client gone")).Maybe() | ||
| 811 | tr.EXPECT().Close().Return(nil).Maybe() | 813 | tr.EXPECT().Close().Return(nil).Maybe() | ||
| 812 | - | d.handleKill(tr, protocol.Kill{Scope: protocol.KillAll}) | 814 | + | d.handleKill(tr, protocol.Kill{RequestID: 1, Scope: protocol.KillAll}) |
| 813 | 815 | ||||
| 814 | - | require.Contains(t, logged.String(), "kill all failure response") | 816 | + | require.Contains(t, logged.String(), "kill result") |
| 815 | require.Contains(t, logged.String(), "client gone") | 817 | require.Contains(t, logged.String(), "client gone") | ||
| 818 | + | require.Contains(t, logged.String(), "outcome="+strconv.Itoa(int(protocol.KillFailed)), "the send-failure log must name the unobserved outcome") | |||
| 819 | + | require.Contains(t, logged.String(), "code="+strconv.Itoa(int(protocol.ErrInternal)), "the send-failure log must name the unobserved code") | |||
| 816 | } | 820 | } | ||
| 817 | 821 | ||||
| 818 | // TestPurgeResultSummaryBoundsWireDetail proves the partial-failure summary is | 822 | // TestPurgeResultSummaryBoundsWireDetail proves the partial-failure summary is | ||
M internal/usecase/daemon/session_test.go +1 -1
| @@ -144,7 +144,7 @@ func TestKillAllEmptyDaemonKeepsServing(t *testing.T) { | |||
| 144 | 144 | pty, release := newBlockingPTY(t) | |
| 145 | 145 | defer release() | |
| 146 | 146 | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 147 | - | tr, _, _ := newConn(t, mustClientEnvelope(protocol.Kill{Scope: protocol.KillAll})) | |
| 147 | + | tr, _, _ := newConn(t, mustClientEnvelope(protocol.Kill{RequestID: 1, Scope: protocol.KillAll})) | |
| 148 | 148 | d.handleConn(tr) | |
| 149 | 149 | ||
| 150 | 150 | select { | |
| @@ -144,7 +144,7 @@ func TestKillAllEmptyDaemonKeepsServing(t *testing.T) { | |||||
| 144 | pty, release := newBlockingPTY(t) | 144 | pty, release := newBlockingPTY(t) | ||
| 145 | defer release() | 145 | defer release() | ||
| 146 | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | 146 | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | ||
| 147 | - | tr, _, _ := newConn(t, mustClientEnvelope(protocol.Kill{Scope: protocol.KillAll})) | 147 | + | tr, _, _ := newConn(t, mustClientEnvelope(protocol.Kill{RequestID: 1, Scope: protocol.KillAll})) |
| 148 | d.handleConn(tr) | 148 | d.handleConn(tr) | ||
| 149 | 149 | ||||
| 150 | select { | 150 | select { | ||
M internal/usecase/daemon/typed_connection_test.go +2 -0
| @@ -159,6 +159,8 @@ func serverMessageName(message protocol.ServerMessage) string { | |||
| 159 | 159 | return "Sessions" | |
| 160 | 160 | case protocol.CommandResult: | |
| 161 | 161 | return "CommandResult" | |
| 162 | + | case protocol.KillResult: | |
| 163 | + | return "KillResult" | |
| 162 | 164 | case protocol.AttachTarget: | |
| 163 | 165 | return "AttachTarget" | |
| 164 | 166 | case protocol.RemotePreview: | |
| @@ -159,6 +159,8 @@ func serverMessageName(message protocol.ServerMessage) string { | |||||
| 159 | return "Sessions" | 159 | return "Sessions" | ||
| 160 | case protocol.CommandResult: | 160 | case protocol.CommandResult: | ||
| 161 | return "CommandResult" | 161 | return "CommandResult" | ||
| 162 | + | case protocol.KillResult: | |||
| 163 | + | return "KillResult" | |||
| 162 | case protocol.AttachTarget: | 164 | case protocol.AttachTarget: | ||
| 163 | return "AttachTarget" | 165 | return "AttachTarget" | ||
| 164 | case protocol.RemotePreview: | 166 | case protocol.RemotePreview: | ||