From 234a926505bd405ead8e325e1ecc6be8b9ea1f37 Mon Sep 17 00:00:00 2001 From: Agustin Rivera Date: Mon, 14 Sep 2026 15:49:01 -0700 Subject: [PATCH 1/4] fix(connection): require hello-ok before operator RPCs The gateway 1008-closes (PolicyViolation) any non-connect frame sent before hello-ok (#1418). Gate operator readiness (IsConnectedToGateway) on the handshake snapshot and guard the application send paths: - SendTrackedRequestAsync: fire-and-forget requests are silently dropped pre-handshake (mirrors the socket-closed contract; the post-handshake refresh burst re-requests the state). - SendWizardRequestAsync: throws InvalidOperationException carrying the shared HandshakePendingError marker; tolerant callers (pairing, models fallback, media, payload reads) already degrade gracefully. - GetUpdateStatusAsync: null pre-handshake, keeping its no-info contract. - TryRequestPayloadAsync: handshake-pending reads return null, same shape as unknown-method. - connect itself is exempt by construction: the handshake rides SendConnectMessageAsync/SendRawAsync, never the tracked/wizard paths. Tests: mechanical wizard/media suites satisfy the gate via a CompleteHandshakeForTest flag-set (real challenge/connect/hello-ok path is exercised by the round-trip suite, which now performs the full dance against the loopback gateway); three new guard tests pin suppression, wizard rejection, and readiness lifecycle. --- .../OpenClawGatewayClient.Protocol.cs | 7 ++ src/OpenClaw.Shared/OpenClawGatewayClient.cs | 40 ++++++- .../GatewayProtocolLiveRoundTripTests.cs | 64 ++++++++-- ...penClawGatewayClientAssistantMediaTests.cs | 21 ++++ .../OpenClawGatewayClientTests.cs | 113 ++++++++++++++++++ 5 files changed, 232 insertions(+), 13 deletions(-) diff --git a/src/OpenClaw.Shared/OpenClawGatewayClient.Protocol.cs b/src/OpenClaw.Shared/OpenClawGatewayClient.Protocol.cs index 25962ec1c..716b77254 100644 --- a/src/OpenClaw.Shared/OpenClawGatewayClient.Protocol.cs +++ b/src/OpenClaw.Shared/OpenClawGatewayClient.Protocol.cs @@ -820,6 +820,13 @@ internal static SessionCompactionMutationResult ParseCompactionMutation(JsonElem _logger.Warn($"{method} unsupported on gateway"); return null; } + catch (InvalidOperationException ex) when ( + ex.Message.Contains(HandshakePendingError, StringComparison.Ordinal)) + { + // #1418: before hello-ok these payload reads are unavailable, not + // failures; callers treat null the same as an unsupported gateway. + return null; + } } private static JsonElement? TryGetArray(JsonElement parent, string property) diff --git a/src/OpenClaw.Shared/OpenClawGatewayClient.cs b/src/OpenClaw.Shared/OpenClawGatewayClient.cs index 151563fd3..2d65a0677 100644 --- a/src/OpenClaw.Shared/OpenClawGatewayClient.cs +++ b/src/OpenClaw.Shared/OpenClawGatewayClient.cs @@ -285,7 +285,18 @@ public Func>? public string? OperatorDeviceId => _operatorDeviceId; public IReadOnlyList GrantedOperatorScopes => _grantedOperatorScopes; - public virtual bool IsConnectedToGateway => IsConnected; + + /// + /// Readiness gate for the operator session: the WebSocket transport is open + /// AND the hello-ok handshake has completed. The gateway closes the socket + /// with 1008 PolicyViolation ("first request must be connect") when any + /// non-connect frame escapes before hello-ok (#1418), so callers must gate + /// application RPCs and "connected" UI state on this property, not on the + /// transport-only socket state. Protocol frames are exempt by design: the + /// handshake drives them over the dedicated SendConnectMessageAsync path, + /// never through the tracked/wizard application-send paths. + /// + public virtual bool IsConnectedToGateway => IsConnected && HasHandshakeSnapshot; public int? LastRemoteCloseStatusCode => RemoteCloseStatusCode; protected override void OnConnectionException(Exception exception) @@ -1196,11 +1207,24 @@ private static bool ReadBoolean(JsonElement value, params string[] propertyNames /// Sends a wizard RPC request and waits for the response payload. /// Used for wizard.start, wizard.next, wizard.cancel, wizard.status. /// + private const string HandshakePendingError = + "Gateway handshake has not completed (hello-ok pending)"; + public async Task SendWizardRequestAsync(string method, object? parameters = null, int timeoutMs = 30000) { if (!IsConnected) throw new InvalidOperationException("Gateway connection is not open"); + // #1418: wizard requests are application RPCs (models.list, + // device.pair.list, pairing approvals, update.status, chat.abort, + // artifacts.download). Sending one before hello-ok makes the gateway + // 1008-close the socket, so fail with the same contract as a closed + // connection; tolerant callers (pairing approve/reject, models.list + // fallback, media resolution, payload reads) already catch + // InvalidOperationException and degrade gracefully. + if (!HasHandshakeSnapshot) + throw new InvalidOperationException($"{HandshakePendingError}; refusing to send '{method}'"); + _logger.Info($"[GatewayClient] Sending frame: {method}"); var requestId = Guid.NewGuid().ToString(); var pending = _pendingRequests.RegisterWizard(requestId, method); @@ -1884,7 +1908,7 @@ public async Task StopChannelAsync(string channelName) /// public async Task GetUpdateStatusAsync(int timeoutMs = 5000) { - if (!IsConnected) + if (!IsConnected || !HasHandshakeSnapshot) return null; var response = await SendWizardRequestAsync("update.status", new { }, timeoutMs); @@ -2111,6 +2135,18 @@ private async Task SendTrackedRequestAsync(string method, object? parameters = n { if (!IsConnected) return; + // #1418: the gateway 1008-closes the socket when an application RPC + // escapes before hello-ok. Tracked requests are fire-and-forget, so + // mirror the socket-closed path above and drop silently; the + // post-handshake refresh burst re-requests this state. The connect + // handshake does not use this path (SendConnectMessageAsync sends via + // SendRawAsync directly), so protocol frames are exempt by construction. + if (!HasHandshakeSnapshot) + { + _logger.Debug($"[GatewayClient] {method} suppressed before handshake"); + return; + } + var requestId = Guid.NewGuid().ToString(); var pending = _pendingRequests.RegisterTracked(requestId, method); try diff --git a/tests/OpenClaw.Shared.Tests/GatewayProtocolLiveRoundTripTests.cs b/tests/OpenClaw.Shared.Tests/GatewayProtocolLiveRoundTripTests.cs index dad2c9437..07e560a19 100644 --- a/tests/OpenClaw.Shared.Tests/GatewayProtocolLiveRoundTripTests.cs +++ b/tests/OpenClaw.Shared.Tests/GatewayProtocolLiveRoundTripTests.cs @@ -252,12 +252,13 @@ public async Task ChatSend_UsesCallerProvidedIdempotencyKeyOnWire() private static void ConfigureResponders(LoopbackGatewayServer server) { - // NOTE: intentionally NO hello-ok responder. The new methods only require - // an open socket (IsConnectedToGateway), not the full handshake. Skipping - // hello-ok avoids the client's post-handshake auto-request storm - // (health/sessions.list/subscribe/usage/nodes/agents), keeping this test - // lightweight so it does not add scheduler/socket contention that could - // destabilize other timing-sensitive socket tests under parallel load. + // NOTE: no per-test connect responder is registered here — + // ConnectAndWaitAsync installs the challenge/connect/hello-ok handshake + // (required since #1418 made the readiness gate handshake-aware). The + // handshake's post-hello-ok auto-request burst (health/sessions.list/ + // subscribe/usage/nodes/agents) answers with the default {} payload, + // which is inert for every parser; per-method frame assertions filter + // it out. server.OnMethod("update.status", parameters => { @@ -351,6 +352,13 @@ private static void ConfigureResponders(LoopbackGatewayServer server) private static async Task ConnectAndWaitAsync(OpenClawGatewayClient client, LoopbackGatewayServer server) { + // #1418: the readiness gate (IsConnectedToGateway) now requires the + // hello-ok handshake, so the loopback gateway performs the real + // challenge → connect → hello-ok dance. The handshake also arms the + // post-handshake auto-request burst (health/sessions/usage/nodes/ + // agents); FrameFor/WaitFrameAsync filter by method, so those extra + // frames never collide with a test's own method assertions. + server.EnableHandshake(); var connected = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); void OnStatus(object? _, ConnectionStatus s) { @@ -360,11 +368,9 @@ void OnStatus(object? _, ConnectionStatus s) try { await client.ConnectAsync(); - // The new methods only require an open socket (IsConnectedToGateway), - // not the full hello-ok handshake. Poll readiness with a generous - // ceiling so a load-starved runner doesn't cause a false failure; - // the loop exits as soon as the socket is open. The Connected event - // (hello-ok) is a fast-path signal but not required. + // Poll readiness with a generous ceiling so a load-starved runner + // doesn't cause a false failure; the loop exits as soon as the + // challenge → connect → hello-ok dance completes. var deadline = DateTime.UtcNow.AddSeconds(20); while (!client.IsConnectedToGateway && DateTime.UtcNow < deadline) { @@ -406,6 +412,7 @@ private sealed class LoopbackGatewayServer : IDisposable private readonly Task _loop; private readonly Dictionary> _responders = new(StringComparer.Ordinal); private readonly ConcurrentQueue<(string Method, string Frame)> _frames = new(); + private volatile string? _greeting; public int Port { get; } public string WebSocketUrl => $"ws://127.0.0.1:{Port}/"; @@ -440,6 +447,31 @@ public LoopbackGatewayServer() public void OnMethod(string method, Func responder) => _responders[method] = responder; + /// Sends a raw server frame immediately after the WebSocket is + /// accepted, before reading anything from the client. Used for the + /// connect.challenge event that gates the client's connect request. + public void SendGreetingOnAccept(string frame) => _greeting = frame; + + /// + /// Installs the #1418 handshake: sends connect.challenge on accept and + /// answers the client's connect request with a minimal valid hello-ok + /// (protocol 4). Required because the operator readiness gate now + /// demands a completed handshake before application RPCs. + /// + public void EnableHandshake() + { + // Same wire shape as the challenge frames used by + // OpenClawGatewayClientTests. Serialized as a literal because + // `event` is a C# keyword and cannot be an anonymous-type member; + // the loopback responder never validates timestamp freshness, so a + // fixed ts keeps the frame deterministic. + SendGreetingOnAccept( + """ + {"type":"event","event":"connect.challenge","payload":{"nonce":"rt-challenge","ts":1785824000000}} + """); + OnMethod("connect", _ => new { type = "hello-ok", protocol = 4 }); + } + public IEnumerable AllFrames { get @@ -508,6 +540,16 @@ private async Task ServeAsync(HttpListenerContext ctx) catch { return; } var socket = wsCtx.WebSocket; + var greeting = _greeting; + if (!string.IsNullOrEmpty(greeting)) + { + var greetingBytes = Encoding.UTF8.GetBytes(greeting); + await socket.SendAsync( + new ArraySegment(greetingBytes), + WebSocketMessageType.Text, + endOfMessage: true, + CancellationToken.None); + } var buffer = new byte[16 * 1024]; var sb = new StringBuilder(); diff --git a/tests/OpenClaw.Shared.Tests/OpenClawGatewayClientAssistantMediaTests.cs b/tests/OpenClaw.Shared.Tests/OpenClawGatewayClientAssistantMediaTests.cs index 829041c83..e3e0db740 100644 --- a/tests/OpenClaw.Shared.Tests/OpenClawGatewayClientAssistantMediaTests.cs +++ b/tests/OpenClaw.Shared.Tests/OpenClawGatewayClientAssistantMediaTests.cs @@ -19,6 +19,7 @@ public async Task ResolveStructuredMedia_InlineBase64_ReturnsBoundedTypedBytes() "test-token", identityPath: identity.Path); await client.ConnectAsync(); + CompleteHandshakeForTest(client); var resolution = client.ResolveAssistantMediaAsync( "main", @@ -80,6 +81,7 @@ public async Task ResolveLegacyMedia_UsesBearerMetadataAndSourceBoundTicket() assistantMediaAuthToken: "shared-http-token", assistantMediaHandler: handler); await client.ConnectAsync(); + CompleteHandshakeForTest(client); var media = new ChatMediaContentInfo { @@ -121,6 +123,7 @@ public async Task ResolveLegacyMedia_WithoutExplicitHttpCredential_DoesNotUseWeb identityPath: identity.Path, assistantMediaHandler: handler); await client.ConnectAsync(); + CompleteHandshakeForTest(client); var result = await client.ResolveAssistantMediaAsync( "main", @@ -155,6 +158,7 @@ public async Task ResolveLegacyMedia_UsesUpdatedHttpCredential() assistantMediaAuthToken: "shared-token-1", assistantMediaHandler: handler); await client.ConnectAsync(); + CompleteHandshakeForTest(client); var media = new ChatMediaContentInfo { Kind = ChatMediaContentKind.Image, @@ -232,6 +236,23 @@ public void TryResolveManagedMediaUri_EnforcesGatewayRelativeTicketPath( Assert.Equal("gateway.example", uri.Host); } + /// + /// Satisfies the #1418 readiness gate (IsConnectedToGateway requires the + /// hello-ok snapshot) without a wire handshake: media resolution leases + /// gate on that property. Mechanical tests set the snapshot flag directly + /// so the post-handshake auto-request burst cannot race their HTTP handler + /// sequencing; the real handshake path is covered by + /// GatewayProtocolLiveRoundTripTests. + /// + private static void CompleteHandshakeForTest(OpenClawGatewayClient client) + { + var field = typeof(OpenClawGatewayClient).GetField( + "_hasHandshakeSnapshot", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + Assert.NotNull(field); + field!.SetValue(client, true); + } + private static HttpResponseMessage JsonResponse(string json) => new(HttpStatusCode.OK) { diff --git a/tests/OpenClaw.Shared.Tests/OpenClawGatewayClientTests.cs b/tests/OpenClaw.Shared.Tests/OpenClawGatewayClientTests.cs index 94fb69e94..63559b304 100644 --- a/tests/OpenClaw.Shared.Tests/OpenClawGatewayClientTests.cs +++ b/tests/OpenClaw.Shared.Tests/OpenClawGatewayClientTests.cs @@ -116,6 +116,23 @@ public void OnDisconnected() method!.Invoke(_client, Array.Empty()); } + /// + /// Marks the hello-ok handshake snapshot complete without a wire + /// handshake. Mechanical wizard/transport tests use this to satisfy the + /// #1418 readiness gate without arming the post-handshake auto-request + /// burst, which would race their frame reads; the real + /// challenge → connect → hello-ok path is covered by + /// GatewayProtocolLiveRoundTripTests and the ProcessRawMessage tests. + /// + public void CompleteHandshakeForTest() + { + var field = typeof(OpenClawGatewayClient).GetField( + "_hasHandshakeSnapshot", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + Assert.NotNull(field); + field!.SetValue(_client, true); + } + public void ProcessRawMessage(string json) { var method = typeof(OpenClawGatewayClient).GetMethod( @@ -468,6 +485,15 @@ public bool GetPairingRequiredFlag() => public string? GetPairingRequiredRequestId() => _client.PairingRequiredRequestId; + public bool IsTransportConnectedForTest() + { + var property = typeof(WebSocketClientBase).GetProperty( + "IsConnected", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + Assert.NotNull(property); + return (bool)property!.GetValue(_client)!; + } + public bool ShouldAutoReconnectForTest() { var method = typeof(OpenClawGatewayClient).GetMethod( @@ -565,6 +591,7 @@ public async Task SendWizardRequestAsync_ResponseBeforeDispose_ReturnsPayloadAnd identityPath: identity.Path); using var client = helper.Client; await client.ConnectAsync(); + helper.CompleteHandshakeForTest(); var responseTask = client.SendWizardRequestAsync("wizard.start", timeoutMs: 10_000); var request = await server.ReceiveTextAsync().WaitAsync(TimeSpan.FromSeconds(2)); @@ -599,6 +626,7 @@ public async Task SendWizardRequestAsync_GatewayError_PropagatesUnchangedAndClea identityPath: identity.Path); using var client = helper.Client; await client.ConnectAsync(); + helper.CompleteHandshakeForTest(); var responseTask = client.SendWizardRequestAsync("wizard.next", timeoutMs: 10_000); var request = await server.ReceiveTextAsync().WaitAsync(TimeSpan.FromSeconds(2)); @@ -631,6 +659,7 @@ public async Task SendWizardRequestAsync_DeadlineExpires_ThrowsTimeoutAndCleansT identityPath: identity.Path); using var client = helper.Client; await client.ConnectAsync(); + helper.CompleteHandshakeForTest(); var responseTask = client.SendWizardRequestAsync("wizard.status", timeoutMs: 250); await server.ReceiveTextAsync().WaitAsync(TimeSpan.FromSeconds(2)); @@ -652,6 +681,7 @@ public async Task SendWizardRequestAsync_LateResponseAfterTimeout_DoesNotChangeO identityPath: identity.Path); using var client = helper.Client; await client.ConnectAsync(); + helper.CompleteHandshakeForTest(); var timedOutTask = client.SendWizardRequestAsync("wizard.status", timeoutMs: 250); var timedOutRequest = await server.ReceiveTextAsync().WaitAsync(TimeSpan.FromSeconds(2)); @@ -698,6 +728,7 @@ public async Task SendWizardRequestAsync_DisposeBeforeResponse_ThrowsCancellatio identityPath: identity.Path); using var client = helper.Client; await client.ConnectAsync(); + helper.CompleteHandshakeForTest(); var responseTask = client.SendWizardRequestAsync("wizard.cancel", timeoutMs: 10_000); await server.ReceiveTextAsync().WaitAsync(TimeSpan.FromSeconds(2)); @@ -720,6 +751,7 @@ public async Task SendWizardRequestAsync_TransportDisconnect_DrainsPendingReques identityPath: identity.Path); using var client = helper.Client; await client.ConnectAsync(); + helper.CompleteHandshakeForTest(); var responseTask = client.SendWizardRequestAsync("wizard.next", timeoutMs: 10_000); await server.ReceiveTextAsync().WaitAsync(TimeSpan.FromSeconds(2)); @@ -803,6 +835,87 @@ await Assert.ThrowsAsync( () => client.SendChatMessageAsync("blocked after protocol mismatch")); } + [Fact] + public async Task TrackedRequest_BeforeHandshake_SuppressedWithoutClosingSocket() + { + using var server = new LoopbackWebSocketServer(); + using var identity = new TempDirectory("pre-handshake-tracked-"); + await server.StartAsync(); + var helper = new GatewayClientTestHelper( + gatewayUrl: server.WebSocketUrl, + identityPath: identity.Path); + using var client = helper.Client; + await client.ConnectAsync(); + Assert.False(client.HasHandshakeSnapshot); + + await client.RequestNodesAsync(); + + // #1418: a tracked RPC sent before hello-ok must be dropped, not + // written to the wire — the gateway answers such a frame with a 1008 + // PolicyViolation close. Nothing may reach the server, and the + // transport must stay alive with auto-reconnect armed. + try + { + await server.ReceiveTextAsync().WaitAsync(TimeSpan.FromMilliseconds(300)); + Assert.Fail("A frame reached the wire before hello-ok; the gateway would 1008-close this socket."); + } + catch (TimeoutException) + { + // expected: the suppressed request never reached the loopback server + } + + Assert.True(helper.IsTransportConnectedForTest()); + Assert.True(helper.ShouldAutoReconnectForTest()); + } + + [Fact] + public async Task WizardRequest_BeforeHandshake_ThrowsPendingErrorAndKeepsSocket() + { + using var server = new LoopbackWebSocketServer(); + using var identity = new TempDirectory("pre-handshake-wizard-"); + await server.StartAsync(); + var helper = new GatewayClientTestHelper( + gatewayUrl: server.WebSocketUrl, + identityPath: identity.Path); + using var client = helper.Client; + await client.ConnectAsync(); + Assert.False(client.IsConnectedToGateway); + + var exception = await Assert.ThrowsAsync( + () => client.SendWizardRequestAsync("wizard.start", timeoutMs: 1_000)); + + // Distinct from the closed-socket message: the transport is healthy, + // only the handshake is pending, and the socket must survive. + Assert.Contains("handshake", exception.Message, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("not open", exception.Message, StringComparison.OrdinalIgnoreCase); + Assert.True(helper.IsTransportConnectedForTest()); + Assert.False(client.HasHandshakeSnapshot); + } + + [Fact] + public async Task IsConnectedToGateway_RequiresHelloOkSnapshot_AndClearsOnDisconnect() + { + using var server = new LoopbackWebSocketServer(); + using var identity = new TempDirectory("handshake-gate-"); + await server.StartAsync(); + var helper = new GatewayClientTestHelper( + gatewayUrl: server.WebSocketUrl, + identityPath: identity.Path); + using var client = helper.Client; + await client.ConnectAsync(); + + // Transport open but hello-ok pending: operator readiness stays false. + Assert.True(helper.IsTransportConnectedForTest()); + Assert.False(client.IsConnectedToGateway); + + helper.CompleteHandshakeForTest(); + Assert.True(client.IsConnectedToGateway); + + helper.OnDisconnected(); + Assert.False(client.HasHandshakeSnapshot); + Assert.False(client.IsConnectedToGateway); + } + private static string ReadRequestId(string request) { using var document = JsonDocument.Parse(request); From 5c02ab55d0a294be722289cfa5a00c3ef7fc2218 Mon Sep 17 00:00:00 2001 From: Agustin Rivera Date: Mon, 14 Sep 2026 19:23:03 -0700 Subject: [PATCH 2/4] fix(connection): report not-sent for suppressed tracked mutations SendTrackedRequestAsync returns Task now: false when the socket is closed or the hello-ok handshake is still pending (#1418), true only after the frame hit SendRawAsync successfully. Boolean mutation wrappers (ResetSessionAsync et al.) propagate the not-sent result instead of reporting a false success for frames that never reached the wire; read-style fire-and-forget callers ignore the result and rely on the post-handshake refresh burst, unchanged. Adds two live round-trip tests capturing the wire-level trace: withheld handshake suppresses sessions.reset (no frame on the wire, submission=false), and a server-initiated one-way Close drives the client's real auto-reconnect path, re-running the challenge -> connect -> hello-ok dance before the gate reopens and the mutation is sent. --- src/OpenClaw.Shared/OpenClawGatewayClient.cs | 31 ++-- .../GatewayProtocolLiveRoundTripTests.cs | 158 ++++++++++++++++++ 2 files changed, 178 insertions(+), 11 deletions(-) diff --git a/src/OpenClaw.Shared/OpenClawGatewayClient.cs b/src/OpenClaw.Shared/OpenClawGatewayClient.cs index 2d65a0677..1ae4e5cf2 100644 --- a/src/OpenClaw.Shared/OpenClawGatewayClient.cs +++ b/src/OpenClaw.Shared/OpenClawGatewayClient.cs @@ -2131,20 +2131,29 @@ private ConnectCredential SelectConnectCredential() private bool HasUsableOperatorDeviceToken => !_ignoreStoredDeviceToken && !string.IsNullOrEmpty(_deviceIdentity.DeviceToken); - private async Task SendTrackedRequestAsync(string method, object? parameters = null) + /// + /// Sends a fire-and-forget tracked request. Returns false when the frame + /// was not sent (socket closed, or hello-ok handshake still pending per + /// #1418) so mutation callers can report an unsuccessful submission + /// instead of a false success. Read-style fire-and-forget callers ignore + /// the result; the post-handshake refresh burst re-requests their state. + /// The connect handshake never rides this path + /// (SendConnectMessageAsync sends via SendRawAsync directly), so protocol + /// frames are exempt by construction. + /// + private async Task SendTrackedRequestAsync(string method, object? parameters = null) { - if (!IsConnected) return; + if (!IsConnected) return false; - // #1418: the gateway 1008-closes the socket when an application RPC - // escapes before hello-ok. Tracked requests are fire-and-forget, so - // mirror the socket-closed path above and drop silently; the - // post-handshake refresh burst re-requests this state. The connect - // handshake does not use this path (SendConnectMessageAsync sends via - // SendRawAsync directly), so protocol frames are exempt by construction. if (!HasHandshakeSnapshot) { + // #1418: the gateway 1008-closes the socket when an application RPC + // escapes before hello-ok. Withhold the frame and report not-sent: + // mutations must not report a successful submission, and read-style + // callers tolerate the withheld refresh (the post-handshake burst + // re-requests the state). _logger.Debug($"[GatewayClient] {method} suppressed before handshake"); - return; + return false; } var requestId = Guid.NewGuid().ToString(); @@ -2152,6 +2161,7 @@ private async Task SendTrackedRequestAsync(string method, object? parameters = n try { await SendRawAsync(SerializeRequest(requestId, method, parameters)); + return true; } catch { @@ -2164,8 +2174,7 @@ private async Task TrySendTrackedRequestAsync(string method, object? param { try { - await SendTrackedRequestAsync(method, parameters); - return true; + return await SendTrackedRequestAsync(method, parameters); } catch (Exception ex) { diff --git a/tests/OpenClaw.Shared.Tests/GatewayProtocolLiveRoundTripTests.cs b/tests/OpenClaw.Shared.Tests/GatewayProtocolLiveRoundTripTests.cs index 07e560a19..5d6b715e1 100644 --- a/tests/OpenClaw.Shared.Tests/GatewayProtocolLiveRoundTripTests.cs +++ b/tests/OpenClaw.Shared.Tests/GatewayProtocolLiveRoundTripTests.cs @@ -350,6 +350,107 @@ private static void ConfigureResponders(LoopbackGatewayServer server) server.OnMethod("sessions.patch", _ => new { key = "agent:main:main" }); } + /// + /// PR 1425 review proof (P2): a session mutation submitted while + /// hello-ok is still pending must report not-sent (false) — a boolean + /// "success" would tell the tray the mutation was accepted when no + /// frame ever reached the gateway. Asserts the wire stayed silent. + /// + [Fact] + public async Task HandshakeGate_SuppressedMutation_ReportsNotSentNotSuccess() + { + using var server = new LoopbackGatewayServer(); + server.SilenceMethod("connect"); + server.SendGreetingOnAccept(LoopbackGatewayServer.ChallengeFrame); + using var client = new OpenClawGatewayClient( + server.WebSocketUrl, + "test-token", + identityPath: _identityDir); + + await client.ConnectAsync(); + var connect = await server.WaitFrameAsync("connect", occurrence: 0, timeoutMs: 5_000); + _output.WriteLine("[trace] socket open; challenge delivered; connect request captured (unanswered): " + connect); + Assert.False(client.IsConnectedToGateway); + + var submitted = await client.ResetSessionAsync("agent:main:main"); + _output.WriteLine("[trace] ResetSessionAsync while handshake pending returned: " + submitted); + Assert.False(submitted); + Assert.Throws(() => server.FrameFor("sessions.reset")); + _output.WriteLine("[trace] no sessions.reset frame observed on the wire"); + } + + /// + /// PR 1425 review proof (needs-proof): captured transport trace of the + /// withheld-then-completes path, including a real server-initiated + /// disconnect and auto-reconnect — early mutation suppressed before the + /// handshake, challenge/connect/hello-ok completes, the same mutation + /// goes out and returns true, the socket is closed by the gateway, the + /// gate reopens on the fresh socket, and the mutation succeeds again. + /// The fresh socket comes from the client's real auto-reconnect path + /// (receive-loop exit -> ReconnectWithBackoffAsync). Frame log lines + /// via ITestOutputHelper are the redacted captured trace. + /// + [Fact] + public async Task HandshakeGate_Reconnect_SuppressesEarlyMutationThenSendsAfterHelloOk() + { + using var server = new LoopbackGatewayServer(); + using var client = new OpenClawGatewayClient( + server.WebSocketUrl, + "test-token", + identityPath: _identityDir); + + await client.ConnectAsync(); + Assert.False(client.IsConnectedToGateway); + var suppressed = await client.ResetSessionAsync("agent:main:main"); + Assert.False(suppressed); + Assert.Throws(() => server.FrameFor("sessions.reset")); + Assert.Throws(() => server.FrameFor("connect")); + _output.WriteLine("[trace] socket #1 open, challenge withheld: sessions.reset suppressed (submission=false); no frame reached the wire"); + + server.EnableHandshake(); + await server.SendTextToCurrentSocketAsync(LoopbackGatewayServer.ChallengeFrame); + var connect1 = await server.WaitFrameAsync("connect", occurrence: 0, timeoutMs: 5_000); + _output.WriteLine("[trace] challenge delivered; connect captured (unanswered): " + connect1); + for (var i = 0; i < 200 && !client.IsConnectedToGateway; i++) + await Task.Delay(50); + Assert.True(client.IsConnectedToGateway); + + Assert.True(await client.ResetSessionAsync("agent:main:main")); + var reset1 = await server.WaitFrameAsync("sessions.reset", occurrence: 0, timeoutMs: 5_000); + _output.WriteLine("[trace] hello-ok processed; sessions.reset on the wire: " + reset1); + + // Real server-initiated drop, mirroring a gateway restart: a + // one-way Close frame. The gate must hold from drop observation + // onward. + await server.CloseOutputOnCurrentSocketAsync("gateway restart"); + _output.WriteLine("[trace] server sent one-way Close frame on socket #1; client drop observation begins"); + for (var i = 0; i < 120 && client.IsConnectedToGateway; i++) + await Task.Delay(50); + Assert.False(client.IsConnectedToGateway); + _output.WriteLine("[trace] client observed drop: handshake snapshot cleared, readiness false"); + + // The same mutation submitted during the disconnected window must + // also report not-sent, with nothing reaching the wire. + Assert.False(await client.ResetSessionAsync("agent:main:main")); + Assert.Throws(() => server.FrameFor("sessions.reset", occurrence: 1)); + _output.WriteLine("[trace] sessions.reset during disconnected window: withheld (submission=false)"); + + // The client's real auto-reconnect path (receive-loop exit -> + // ReconnectWithBackoffAsync, 1s+ backoff) must re-run the dance on + // a fresh socket: challenge is auto-sent on accept, and the gate + // reopens only after the fresh hello-ok. + var connect2 = await server.WaitFrameAsync("connect", occurrence: 1, timeoutMs: 15_000); + _output.WriteLine("[trace] socket #2 accepted; challenge auto-resent; connect captured: " + connect2); + for (var i = 0; i < 200 && !client.IsConnectedToGateway; i++) + await Task.Delay(50); + Assert.True(client.IsConnectedToGateway); + + Assert.True(await client.ResetSessionAsync("agent:main:main")); + var reset2 = await server.WaitFrameAsync("sessions.reset", occurrence: 1, timeoutMs: 5_000); + _output.WriteLine("[trace] gate reopened after reconnect; sessions.reset on the wire: " + reset2); + Assert.Contains("sessions.reset", reset2); + } + private static async Task ConnectAndWaitAsync(OpenClawGatewayClient client, LoopbackGatewayServer server) { // #1418: the readiness gate (IsConnectedToGateway) now requires the @@ -413,6 +514,8 @@ private sealed class LoopbackGatewayServer : IDisposable private readonly Dictionary> _responders = new(StringComparer.Ordinal); private readonly ConcurrentQueue<(string Method, string Frame)> _frames = new(); private volatile string? _greeting; + private WebSocket? _currentSocket; + private readonly HashSet _silentMethods = new(StringComparer.Ordinal); public int Port { get; } public string WebSocketUrl => $"ws://127.0.0.1:{Port}/"; @@ -452,6 +555,58 @@ public LoopbackGatewayServer() /// connect.challenge event that gates the client's connect request. public void SendGreetingOnAccept(string frame) => _greeting = frame; + /// Challenge frame sent on accept in the handshake; also sent + /// manually mid-connection by the withheld-handshake trace tests. + public const string ChallengeFrame = + "{\"type\":\"event\",\"event\":\"connect.challenge\",\"payload\":{\"nonce\":\"trace-challenge\",\"ts\":1785824000000}}"; + + /// Makes the server capture requests for a method but send no + /// response, so the client's pending request stays open. Used to + /// withhold hello-ok deterministically instead of an unregistered + /// responder's auto-{} reply (whose client-side handling is untested). + public void SilenceMethod(string method) => _silentMethods.Add(method); + + /// Sends a raw server frame on the current client socket + /// (used to deliver the challenge and hello-ok mid-connection). + public async Task SendTextToCurrentSocketAsync(string frame) + { + var socket = Volatile.Read(ref _currentSocket); + if (socket is null || socket.State != WebSocketState.Open) + throw new InvalidOperationException("No open client socket to send on."); + var bytes = Encoding.UTF8.GetBytes(frame); + await socket.SendAsync( + new ArraySegment(bytes), + WebSocketMessageType.Text, + endOfMessage: true, + CancellationToken.None); + } + + /// Server-initiated abort of the current client socket, + /// mirroring a hard network drop. Abort (not a graceful close): a + /// close handshake would race the client's own close handling on the + /// same socket. + public void AbortCurrentSocket() + { + var socket = Volatile.Read(ref _currentSocket); + socket?.Abort(); + } + + /// Server-initiated one-way Close frame on the current + /// client socket (mirrors a gateway-restart disconnect). Half-close + /// only: CloseOutputAsync sends the Close frame without waiting for + /// the client's ack, so the client observes a deterministic in-band + /// drop and its real auto-reconnect path takes over. + public async Task CloseOutputOnCurrentSocketAsync(string reason) + { + var socket = Volatile.Read(ref _currentSocket); + if (socket is null || socket.State != WebSocketState.Open) + throw new InvalidOperationException("No open client socket to close."); + await socket.CloseOutputAsync( + WebSocketCloseStatus.EndpointUnavailable, + reason, + CancellationToken.None); + } + /// /// Installs the #1418 handshake: sends connect.challenge on accept and /// answers the client's connect request with a minimal valid hello-ok @@ -540,6 +695,7 @@ private async Task ServeAsync(HttpListenerContext ctx) catch { return; } var socket = wsCtx.WebSocket; + Volatile.Write(ref _currentSocket, socket); var greeting = _greeting; if (!string.IsNullOrEmpty(greeting)) { @@ -597,6 +753,8 @@ private async Task HandleFrameAsync(WebSocket socket, string frame) _frames.Enqueue((method, frame)); + if (_silentMethods.Contains(method)) return; + object payload = _responders.TryGetValue(method, out var responder) ? responder(parameters) : new { }; From 6edabb178c60fbf49f7cc2e6ec4b12485048754d Mon Sep 17 00:00:00 2001 From: Agustin Rivera Date: Mon, 14 Sep 2026 19:49:44 -0700 Subject: [PATCH 3/4] docs: add captured wire proof for handshake gate (#1418) Redacted [trace] lines captured from the actual green dotnet test run (TRX-embedded ITestOutputHelper output) of both HandshakeGate live round-trip tests: pre-hello-ok suppression with zero frames on the wire, gate held while disconnected after a server-initiated one-way Close, and the real auto-reconnect path re-running challenge -> connect -> hello-ok before the mutation is sent. Auth tokens, device ids, and device key material redacted; all values are test-only. --- docs/gateway-handshake-gate-proof.md | 62 ++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 docs/gateway-handshake-gate-proof.md diff --git a/docs/gateway-handshake-gate-proof.md b/docs/gateway-handshake-gate-proof.md new file mode 100644 index 000000000..ac91f3dcd --- /dev/null +++ b/docs/gateway-handshake-gate-proof.md @@ -0,0 +1,62 @@ +# Gateway handshake gate — captured wire proof + +Evidence artifact for PR #1425 (fixes #1418), captured at branch tip `5c02ab5` +from a real `dotnet test` execution of `tests/OpenClaw.Shared.Tests` on the +production client code path (`OpenClawGatewayClient` over real WebSockets +against a loopback gateway server). No live gateway or private endpoint was +contacted: the server under test runs inside the test process. + +**Capture method:** `dotnet test --filter FullyQualifiedName~HandshakeGate +--logger "trx;LogFileName=proof.trx"`; the `[trace]` lines below are the +unmodified `ITestOutputHelper` output embedded in the TRX results file, with +auth tokens, device ids, and device key material redacted (all values are +test-only). Request ids and test nonces are harmless and kept verbatim. + +**Suite status at the captured tip:** affected filter set 278/278 passed +(GatewayClient + live round-trip + AssistantMedia + ShutdownClassification); +protocol drift/contract/closure safety filters 30/30; `Tray.Tests` unchanged +versus a clean base run (18 pre-existing Linux-env failures, identical on +base `6466a04`; CI runs that suite on Windows). + +The two scenarios below demonstrate the full gate lifecycle: suppression +before hello-ok (nothing on the wire), the gate holding while disconnected +(suppressed again, submission reported as false, not success), and the +client's **real auto-reconnect path** (receive-loop exit → +`ReconnectWithBackoffAsync`) re-running challenge → connect → hello-ok before +the gate reopens and the mutation reaches the wire. + +## Scenario 1 — mutation submitted before hello-ok: withheld, zero frames + +**Test:** `HandshakeGate_SuppressedMutation_ReportsNotSentNotSuccess` — outcome **Passed** (duration 00:00:00.1175257) + +```text +[trace] socket open; challenge delivered; connect request captured (unanswered): {"type":"req","id":"3e7f62bf-d2e5-43b8-9145-0ecc2ed6a7c9","method":"connect","params":{"minProtocol":3,"maxProtocol":4,"client":{"id":"cli","version":"18.6.0","platform":"windows","deviceFamily":"Windows","mode":"cli","displayName":"OpenClaw Windows Tray"},"role":"operator","scopes":["operator.admin","operator.pairing"],"caps":[],"commands":[],"permissions":{},"auth":{"token":"***REDACTED***"},"locale":"en-US","userAgent":"openclaw-windows-tray/18.6.0","device":{"id":"***REDACTED***","publicKey":"***REDACTED***","signature":"***REDACTED***","signedAt":0,"nonce":"trace-challenge"}}} +[trace] ResetSessionAsync while handshake pending returned: False +[trace] no sessions.reset frame observed on the wire +``` + +## Scenario 2 — reconnect: drop → gate holds → auto-reconnect → mutation sends after hello-ok + +**Test:** `HandshakeGate_Reconnect_SuppressesEarlyMutationThenSendsAfterHelloOk` — outcome **Passed** (duration 00:00:01.3212202) + +```text +[trace] socket #1 open, challenge withheld: sessions.reset suppressed (submission=false); no frame reached the wire +[trace] challenge delivered; connect captured (unanswered): {"type":"req","id":"81269861-e101-4930-9de6-e3c17d78d6b1","method":"connect","params":{"minProtocol":3,"maxProtocol":4,"client":{"id":"cli","version":"18.6.0","platform":"windows","deviceFamily":"Windows","mode":"cli","displayName":"OpenClaw Windows Tray"},"role":"operator","scopes":["operator.admin","operator.pairing"],"caps":[],"commands":[],"permissions":{},"auth":{"token":"***REDACTED***"},"locale":"en-US","userAgent":"openclaw-windows-tray/18.6.0","device":{"id":"***REDACTED***","publicKey":"***REDACTED***","signature":"***REDACTED***","signedAt":0,"nonce":"trace-challenge"}}} +[trace] hello-ok processed; sessions.reset on the wire: {"type":"req","id":"e502fbba-2ee3-4275-96f7-71e1d14d562f","method":"sessions.reset","params":{"key":"agent:main:main"}} +[trace] server sent one-way Close frame on socket #1; client drop observation begins +[trace] client observed drop: handshake snapshot cleared, readiness false +[trace] sessions.reset during disconnected window: withheld (submission=false) +[trace] socket #2 accepted; challenge auto-resent; connect captured: {"type":"req","id":"0b1ae270-2fd8-424f-8025-08ff33c2b2fa","method":"connect","params":{"minProtocol":3,"maxProtocol":4,"client":{"id":"cli","version":"18.6.0","platform":"windows","deviceFamily":"Windows","mode":"cli","displayName":"OpenClaw Windows Tray"},"role":"operator","scopes":["operator.admin","operator.pairing"],"caps":[],"commands":[],"permissions":{},"auth":{"token":"***REDACTED***"},"locale":"en-US","userAgent":"openclaw-windows-tray/18.6.0","device":{"id":"***REDACTED***","publicKey":"***REDACTED***","signature":"***REDACTED***","signedAt":0,"nonce":"rt-challenge"}}} +[trace] gate reopened after reconnect; sessions.reset on the wire: {"type":"req","id":"415c05df-6e81-4297-9554-ea62575a2f3a","method":"sessions.reset","params":{"key":"agent:main:main"}} +``` + +**Wire-level assertions backing the trace** (same tests, same run): + +- `Assert.Throws(() => server.FrameFor("sessions.reset"))` + after the pre-handshake submission — no frame was captured on the wire. +- After the one-way server Close: `Assert.False(client.IsConnectedToGateway)` + and the disconnected-window submission returns `false` with no captured + frame. +- After auto-reconnect: `Assert.True(client.IsConnectedToGateway)` only + following the fresh hello-ok, and the second `sessions.reset` is captured + on the wire. From 17cfc728cf40a9f65632f860f5a6f6812c0e1826 Mon Sep 17 00:00:00 2001 From: Agustin Rivera Date: Tue, 15 Sep 2026 06:59:00 -0700 Subject: [PATCH 4/4] docs: drop em dashes to satisfy validate-docs gate (#1418) scripts/validate-docs.ps1 rejects U+2014 in maintained Markdown; replace the six em dashes in the proof document with permitted punctuation. The captured trace blocks are unchanged. --- docs/gateway-handshake-gate-proof.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/gateway-handshake-gate-proof.md b/docs/gateway-handshake-gate-proof.md index ac91f3dcd..4ba238533 100644 --- a/docs/gateway-handshake-gate-proof.md +++ b/docs/gateway-handshake-gate-proof.md @@ -1,4 +1,4 @@ -# Gateway handshake gate — captured wire proof +# Gateway handshake gate: captured wire proof Evidence artifact for PR #1425 (fixes #1418), captured at branch tip `5c02ab5` from a real `dotnet test` execution of `tests/OpenClaw.Shared.Tests` on the @@ -25,9 +25,9 @@ client's **real auto-reconnect path** (receive-loop exit → `ReconnectWithBackoffAsync`) re-running challenge → connect → hello-ok before the gate reopens and the mutation reaches the wire. -## Scenario 1 — mutation submitted before hello-ok: withheld, zero frames +## Scenario 1: mutation submitted before hello-ok (withheld, zero frames) -**Test:** `HandshakeGate_SuppressedMutation_ReportsNotSentNotSuccess` — outcome **Passed** (duration 00:00:00.1175257) +**Test:** `HandshakeGate_SuppressedMutation_ReportsNotSentNotSuccess` - outcome **Passed** (duration 00:00:00.1175257) ```text [trace] socket open; challenge delivered; connect request captured (unanswered): {"type":"req","id":"3e7f62bf-d2e5-43b8-9145-0ecc2ed6a7c9","method":"connect","params":{"minProtocol":3,"maxProtocol":4,"client":{"id":"cli","version":"18.6.0","platform":"windows","deviceFamily":"Windows","mode":"cli","displayName":"OpenClaw Windows Tray"},"role":"operator","scopes":["operator.admin","operator.pairing"],"caps":[],"commands":[],"permissions":{},"auth":{"token":"***REDACTED***"},"locale":"en-US","userAgent":"openclaw-windows-tray/18.6.0","device":{"id":"***REDACTED***","publicKey":"***REDACTED***","signature":"***REDACTED***","signedAt":0,"nonce":"trace-challenge"}}} @@ -35,9 +35,9 @@ the gate reopens and the mutation reaches the wire. [trace] no sessions.reset frame observed on the wire ``` -## Scenario 2 — reconnect: drop → gate holds → auto-reconnect → mutation sends after hello-ok +## Scenario 2: reconnect flow (drop → gate holds → auto-reconnect → mutation sends after hello-ok) -**Test:** `HandshakeGate_Reconnect_SuppressesEarlyMutationThenSendsAfterHelloOk` — outcome **Passed** (duration 00:00:01.3212202) +**Test:** `HandshakeGate_Reconnect_SuppressesEarlyMutationThenSendsAfterHelloOk` - outcome **Passed** (duration 00:00:01.3212202) ```text [trace] socket #1 open, challenge withheld: sessions.reset suppressed (submission=false); no frame reached the wire @@ -53,7 +53,7 @@ the gate reopens and the mutation reaches the wire. **Wire-level assertions backing the trace** (same tests, same run): - `Assert.Throws(() => server.FrameFor("sessions.reset"))` - after the pre-handshake submission — no frame was captured on the wire. + after the pre-handshake submission; no frame was captured on the wire. - After the one-way server Close: `Assert.False(client.IsConnectedToGateway)` and the disconnected-window submission returns `false` with no captured frame.