From eeaf823b216035e95d49e08b0989e05b387200ff Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 26 Mar 2026 01:11:32 +0000 Subject: [PATCH] fix: prevent double PairingStatusChanged fire in hello-ok handler When hello-ok includes auth.deviceToken, HandleResponse was firing PairingStatusChanged twice: 1. From the auth block when wasWaiting (Paired + 'Pairing approved!') 2. From the DeviceToken fallback check below, which saw the newly-stored token and fired Paired again unconditionally. Fix: introduce a gotNewToken guard so the DeviceToken fallback check is skipped entirely when a token was received in the response body. This ensures exactly one PairingStatusChanged event fires per hello-ok regardless of whether the device was previously pending approval. Add two regression tests using reflection to invoke HandleResponse: - hello-ok WITH deviceToken while pending => exactly one Paired event - hello-ok WITHOUT deviceToken and no stored token => exactly one Pending event Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/OpenClaw.Shared/WindowsNodeClient.cs | 75 ++++++------ .../WindowsNodeClientTests.cs | 108 ++++++++++++++++++ 2 files changed, 146 insertions(+), 37 deletions(-) diff --git a/src/OpenClaw.Shared/WindowsNodeClient.cs b/src/OpenClaw.Shared/WindowsNodeClient.cs index 98df1e8..d05a12e 100644 --- a/src/OpenClaw.Shared/WindowsNodeClient.cs +++ b/src/OpenClaw.Shared/WindowsNodeClient.cs @@ -450,51 +450,52 @@ private void HandleResponse(JsonElement root) _nodeId = nodeIdProp.GetString(); } - // Check for device token in auth (means we're paired!) - if (payload.TryGetProperty("auth", out var authPayload)) + // Check for device token in auth — if present, pairing is confirmed in this response. + // Use gotNewToken to guard the fallback check below and avoid a double-fire of + // PairingStatusChanged when the gateway includes auth.deviceToken in hello-ok. + bool gotNewToken = false; + if (payload.TryGetProperty("auth", out var authPayload) && + authPayload.TryGetProperty("deviceToken", out var deviceTokenProp)) { - if (authPayload.TryGetProperty("deviceToken", out var deviceTokenProp)) + var deviceToken = deviceTokenProp.GetString(); + if (!string.IsNullOrEmpty(deviceToken)) { - var deviceToken = deviceTokenProp.GetString(); - if (!string.IsNullOrEmpty(deviceToken)) - { - var wasWaiting = _isPendingApproval; - _isPendingApproval = false; - _logger.Info("Received device token - we are now paired!"); - _deviceIdentity.StoreDeviceToken(deviceToken); - - // Fire pairing event if we were waiting - if (wasWaiting) - { - PairingStatusChanged?.Invoke(this, new PairingStatusEventArgs( - PairingStatus.Paired, - _deviceIdentity.DeviceId, - "Pairing approved!")); - } - } + gotNewToken = true; + var wasWaiting = _isPendingApproval; + _isPendingApproval = false; + _logger.Info("Received device token - we are now paired!"); + _deviceIdentity.StoreDeviceToken(deviceToken); + PairingStatusChanged?.Invoke(this, new PairingStatusEventArgs( + PairingStatus.Paired, + _deviceIdentity.DeviceId, + wasWaiting ? "Pairing approved!" : null)); } } _logger.Info($"Node registered successfully! ID: {_nodeId ?? _deviceIdentity.DeviceId.Substring(0, 16)}"); - // Pairing happens at connect time via device identity, no separate request needed - if (string.IsNullOrEmpty(_deviceIdentity.DeviceToken)) + // Pairing happens at connect time via device identity, no separate request needed. + // Skip this block if we already fired PairingStatusChanged above via gotNewToken. + if (!gotNewToken) { - _isPendingApproval = true; - _logger.Info("Not yet paired - check 'openclaw devices list' for pending approval"); - _logger.Info($"To approve, run: openclaw devices approve {_deviceIdentity.DeviceId}"); - PairingStatusChanged?.Invoke(this, new PairingStatusEventArgs( - PairingStatus.Pending, - _deviceIdentity.DeviceId, - $"Run: openclaw devices approve {ShortDeviceId}...")); - } - else - { - _isPendingApproval = false; - _logger.Info("Already paired with stored device token"); - PairingStatusChanged?.Invoke(this, new PairingStatusEventArgs( - PairingStatus.Paired, - _deviceIdentity.DeviceId)); + if (string.IsNullOrEmpty(_deviceIdentity.DeviceToken)) + { + _isPendingApproval = true; + _logger.Info("Not yet paired - check 'openclaw devices list' for pending approval"); + _logger.Info($"To approve, run: openclaw devices approve {_deviceIdentity.DeviceId}"); + PairingStatusChanged?.Invoke(this, new PairingStatusEventArgs( + PairingStatus.Pending, + _deviceIdentity.DeviceId, + $"Run: openclaw devices approve {ShortDeviceId}...")); + } + else + { + _isPendingApproval = false; + _logger.Info("Already paired with stored device token"); + PairingStatusChanged?.Invoke(this, new PairingStatusEventArgs( + PairingStatus.Paired, + _deviceIdentity.DeviceId)); + } } RaiseStatusChanged(ConnectionStatus.Connected); diff --git a/tests/OpenClaw.Shared.Tests/WindowsNodeClientTests.cs b/tests/OpenClaw.Shared.Tests/WindowsNodeClientTests.cs index 8e9f269..97765fd 100644 --- a/tests/OpenClaw.Shared.Tests/WindowsNodeClientTests.cs +++ b/tests/OpenClaw.Shared.Tests/WindowsNodeClientTests.cs @@ -1,5 +1,8 @@ using System; +using System.Collections.Generic; using System.IO; +using System.Reflection; +using System.Text.Json; using OpenClaw.Shared; using Xunit; @@ -34,4 +37,109 @@ public void Constructor_NormalizesGatewayUrl(string inputUrl, string expectedUrl } } } + + /// + /// Regression test: when hello-ok includes auth.deviceToken, PairingStatusChanged must + /// fire exactly once — not twice (once from the token block and again from the DeviceToken + /// fallback check that follows it). + /// + [Fact] + public void HandleResponse_HelloOkWithDeviceToken_FiresPairingChangedExactlyOnce() + { + var dataPath = Path.Combine(Path.GetTempPath(), $"openclaw-node-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(dataPath); + + try + { + using var client = new WindowsNodeClient("ws://localhost:18789", "test-token", dataPath); + + // Put client into pending-approval state (simulates first-connect, no stored token) + var isPendingField = typeof(WindowsNodeClient).GetField( + "_isPendingApproval", + BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(isPendingField); + isPendingField!.SetValue(client, true); + + var pairingEvents = new List(); + client.PairingStatusChanged += (_, e) => pairingEvents.Add(e); + + // Build a hello-ok payload that includes auth.deviceToken + var json = """ + { + "type": "res", + "ok": true, + "payload": { + "type": "hello-ok", + "nodeId": "test-node-id", + "auth": { + "deviceToken": "test-device-token-abc123" + } + } + } + """; + var root = JsonDocument.Parse(json).RootElement; + + var handleResponseMethod = typeof(WindowsNodeClient).GetMethod( + "HandleResponse", + BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(handleResponseMethod); + handleResponseMethod!.Invoke(client, [root]); + + Assert.Single(pairingEvents); + Assert.Equal(PairingStatus.Paired, pairingEvents[0].Status); + Assert.Equal("Pairing approved!", pairingEvents[0].Message); + } + finally + { + if (Directory.Exists(dataPath)) + { + Directory.Delete(dataPath, true); + } + } + } + + /// + /// When hello-ok has no token and no stored token, fires exactly one Pending event. + /// + [Fact] + public void HandleResponse_HelloOkNoToken_FiresPendingExactlyOnce() + { + var dataPath = Path.Combine(Path.GetTempPath(), $"openclaw-node-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(dataPath); + + try + { + using var client = new WindowsNodeClient("ws://localhost:18789", "test-token", dataPath); + + var pairingEvents = new List(); + client.PairingStatusChanged += (_, e) => pairingEvents.Add(e); + + var json = """ + { + "type": "res", + "ok": true, + "payload": { + "type": "hello-ok", + "nodeId": "test-node-id" + } + } + """; + var root = JsonDocument.Parse(json).RootElement; + + var handleResponseMethod = typeof(WindowsNodeClient).GetMethod( + "HandleResponse", + BindingFlags.NonPublic | BindingFlags.Instance); + handleResponseMethod!.Invoke(client, [root]); + + Assert.Single(pairingEvents); + Assert.Equal(PairingStatus.Pending, pairingEvents[0].Status); + } + finally + { + if (Directory.Exists(dataPath)) + { + Directory.Delete(dataPath, true); + } + } + } }