fix: use node.pair.approve for node command auto-approval - #382
Merged
shanselman merged 1 commit intoMay 14, 2026
Conversation
The auto-approval handler in GatewayConnectionManager was calling DevicePairApproveAsync (device.pair.approve) for node command upgrade pairing requests. Device and node pairing are separate systems with separate request stores, so the gateway returned 'unknown requestId' and the node stayed at 0 effective commands. Changes: - Fix: DevicePairApproveAsync -> NodePairApproveAsync in auto-approval - Race fix: Interlocked CAS guard prevents concurrent approvals - Clear approval guards on gateway switch in DisposeActiveClient - Testability: make approve methods and IsConnectedToGateway virtual - Diagnostics: log caps/commands in handshake and AttachClient - Make Registration internal, expose read-only count/sample accessors - Add 4 regression tests covering the approval method, scope checks, missing requestId, and concurrent duplicate guard (CAS path) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
indierawk2k2
pushed a commit
to indierawk2k2/openclaw-windows-node
that referenced
this pull request
May 16, 2026
…rayNode
Manual test 2026-05-16 hit a new failure mode in the easy-button setup:
setup-state.json:
Phase = 17 (Failed)
FailureCode = operator_pending_approval_failed
UserMessage:
Local gateway pending pairing approval CLI failed (preview stage).
stage1.attempt1.stderr=[openclaw] Could not start the CLI.
[openclaw] Reason: gateway closed (1006 abnormal closure (no close frame))
Trace from tray.log:
07:47:05 PairOperator completes; PairWindowsTrayNode starts
07:47:05 ConnectionManagerWindowsNodeConnector.ConnectAsync invoked
07:47:09 Node connect → NOT_PAIRED (role-upgrade, requestId=18f9be9e)
07:47:09 Manager auto-approve node.pair.approve(requestId) → 'unknown requestId'
(PR openclaw#382 fix doesn't cover this — role-upgrade requests live in the
device-pair queue, not the node-pair queue)
07:47:10 Manager retries node connect → same NOT_PAIRED
07:47:20 GATEWAY EMITS 1012 service-restart (~15s after operator pairing —
gateway flushes config and restarts the systemd service)
07:47:40 EnsureNodeConnectedAsync times out (35s window)
07:47:40 SettingsWindowsTrayNodeProvisioner catches → calls
WslGatewayCliPendingDeviceApprover.ApproveLatestAsync
07:47:40 Stage-1 attempt 1: gateway still restarting → exit 1 with
'1006 abnormal closure'
07:47:41 750ms retry → stage-1 attempt 2: still restarting → exit 1 same
07:47:52 Engine reports operator_pending_approval_failed (misleading —
the issue was a transient gateway-down, not a real pairing problem)
Fix: RunStage1WithRetryAsync now detects the gateway-down stderr pattern
('1006 abnormal closure' / 'gateway closed' / 'Gateway not yet ready' /
'Could not start the CLI') and, when BOTH initial attempts match it, keeps
retrying with exponential backoff (2s, 4s, 8s — ~15s additional patience).
Bails out early if a subsequent attempt fails with a DIFFERENT (non-gateway-
down) error so we surface real failures verbatim rather than burning all
retries.
Why this isn't a regression of our PR: the pre-PR NodeService.ConnectAsync
path hit the same root cause (gateway restart during PairWindowsTrayNode);
the WSL CLI approver fallback existed pre-PR too, with the same 750ms
single-retry budget. This patch fixes the fallback's resilience window for
both pre- and post-unification code paths.
Adds 4 regression tests in OperatorPairingApprovalTests:
- LooksLikeGatewayDown_RecognizesProductionStderrPattern (pattern match)
- GatewayDownInBothAttempts_RetriesUntilSuccess (extended retry happy path)
- PersistentGatewayDown_EventuallyFailsCleanly (graceful bail with stderr)
- GatewayDownThenDifferentError_StopsRetrying (don't mask real failures)
Tray.Tests 962/962 ✅
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The auto-approval handler in
GatewayConnectionManager.OnNodePairingStatusChangedwas callingDevicePairApproveAsync(device.pair.approve) for node command upgrade pairing requests. Device pairing and node command pairing are separate systems with separate request stores on the gateway, so the gateway returnedunknown requestIdand the auto-approval silently failed — leaving the node with 0 effective commands.Root Cause
Line 725 of
GatewayConnectionManager.cscalled the wrong approval method:operatorClient.DevicePairApproveAsync(e.RequestId)operatorClient.NodePairApproveAsync(e.RequestId)Changes
Bug Fix
DevicePairApproveAsync→NodePairApproveAsyncin the auto-approval pathRace Condition Fix
_autoApproveInFlightwithInterlocked.CompareExchange— atomic guard ensures only one approval runs at a timetry/finallywithInterlocked.Exchangefor cleanupGateway Switch Cleanup
_lastAutoApprovedRequestIdand_autoApproveInFlightinDisposeActiveClientso gateway switches start cleanTestability
NodePairApproveAsync,DevicePairApproveAsync,IsConnectedToGatewayvirtual onOpenClawGatewayClientDiagnostics
AttachClientcapability registration stateClientCreatedevent handler state in the diagnostics timelineRegisteredCapabilityCount,RegisteredCommandCount,RegisteredCommandsSample(read-only) onWindowsNodeClient;Registrationmade internalTests (4 new)
AutoApprove_CallsNodePairApprove_NotDevicePairApprove— core regression testAutoApprove_WithoutRequestId_DoesNotAttemptApprovalAutoApprove_WithoutAdminScope_DoesNotAttemptApprovalAutoApprove_SameRequestId_DoesNotApproveTwice— exercises CAS guard with blocked in-flight approvalValidation
./build.ps1✅dotnet test OpenClaw.Shared.Tests— 1,548 passed ✅dotnet test OpenClaw.Tray.Tests— 1,182 passed ✅openclaw nodes describe)