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>
This commit is contained in:
parent
536d436989
commit
eeaf823b21
@ -450,51 +450,52 @@ public class WindowsNodeClient : WebSocketClientBase
|
||||
_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);
|
||||
|
||||
@ -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 class WindowsNodeClientTests
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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).
|
||||
/// </summary>
|
||||
[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<PairingStatusEventArgs>();
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// When hello-ok has no token and no stored token, fires exactly one Pending event.
|
||||
/// </summary>
|
||||
[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<PairingStatusEventArgs>();
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user