Skip to content

fix: handle node pairing events on current master#144

Open
andyeskridge wants to merge 1 commit intoopenclaw:masterfrom
andyeskridge:fix/pairing-issues
Open

fix: handle node pairing events on current master#144
andyeskridge wants to merge 1 commit intoopenclaw:masterfrom
andyeskridge:fix/pairing-issues

Conversation

@andyeskridge
Copy link
Copy Markdown

Summary

Small follow-up to #80 that ports the pairing-specific fixes onto current master.

This PR adds:

  • NOT_PAIRED handling during node registration
  • support for node/device.pair.requested
  • support for node/device.pair.resolved
  • device-target filtering via PayloadTargetsCurrentDevice()
  • correct handling of hello-ok when current gateways omit auth.deviceToken

Testing

Added regression tests for:

  • hello-ok with and without auth.deviceToken
  • NOT_PAIRED responses
  • pairing requested/resolved events
  • ignoring pairing events for other devices

Validated locally with:

  • dotnet test tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj
  • dotnet test tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 4, 2026

🤖 This is an automated response from Repo Assist.

Nice work @andyeskridge! This is a solid, well-structured fix for the pairing edge cases. The test coverage is thorough and the state machine changes are clearly reasoned. A few observations that might be worth discussing before merge:


Logic review: hello-ok always sets _isPaired = true

The current master treated hello-ok without a device token as a pending state:

// old behaviour
_isPendingApproval = true;  // no token → still waiting

This PR changes that to always treat hello-ok acceptance as paired:

// new behaviour
_isPendingApproval = false;
_isPaired = true;  // hello-ok with no token → treated as paired

The comment explains the rationale: "Current gateways only send hello-ok for approved/accepted nodes, even when auth.deviceToken is omitted." If this is confirmed gateway behaviour, the change is correct — but it's worth noting that it flips the meaning of a tokenless hello-ok. Any older gateway version that sends hello-ok before pairing approval would now appear fully paired to the client.

Minor: HandleRequestError early-return on NOT_PAIRED when already pending

if (_isPendingApproval)
{
    return;  // suppress duplicate NOT_PAIRED events
}

This silently suppresses subsequent NOT_PAIRED responses when already pending. That's fine for the normal case, but if a user revokes pairing and reconnects, the _isPendingApproval flag might already be true from the first cycle, suppressing the new pending event. Could be worth a brief comment explaining the intent.

Test: HandleEvent_NodePairResolvedApproved uses SetField via reflection

The test sets _isPendingApproval = true via reflection to simulate the pre-condition. This is consistent with the rest of the test file's approach, so no concern — just flagging it for the reviewer's awareness.


Overall: The fix is a meaningful improvement over the current handling of NOT_PAIRED, pair.requested, and pair.resolved, and the test suite comprehensively covers the added paths. The key question is whether the gateway always sends hello-ok only for approved nodes — if confirmed, the simplification is well-justified.

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@cbb46ab386962aa371045839fc9998ee4e97ca64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant