fix(android): set currentConnection after conn.connect() to prevent node.event before handshake (#79552) by hclsys · Pull Request #79927 · openclaw/openclaw

hclsys · 2026-05-09T18:38:37Z

Summary

Android notifications sent during a WebSocket reconnect window are lost because sendNodeEvent was allowed to run against a connection that had not yet completed the gateway handshake. The gateway rejected events sent before the connect ACK with "invalid handshake: first request must be connect".

Fix: add Connection.isHandshakeComplete (a @Volatile Boolean set to true inside handleConnectSuccess before onConnected fires) and gate sendNodeEvent / sendNodeEventDetailed on this flag. currentConnection is still assigned before conn.connect() so the reference is available during reconnect loops, but the handshake guard prevents traffic through the pre-ACK window.

// Connection — new field
@Volatile var isHandshakeComplete = false

// handleConnectSuccess — set before onConnected
isHandshakeComplete = true
onConnected(serverName, remoteAddress, mainSessionKey)

// sendNodeEvent — add guard
val conn = currentConnection?.takeIf { it.isHandshakeComplete } ?: return false

Test plan

New test sendNodeEvent_returnsFalseWhenCalledBeforeConnectHandshakeCompletes: server delays the connect ACK; sendNodeEvent called during that window returns false, then connection completes normally
Existing test sendNodeEventDetailed_sendsPresenceAlivePayloadAndReturnsStructuredResponse still passes

Fixes #79552.

🤖 Generated with Claude Code

Real behavior proof

Behavior or issue addressed: Android sendNodeEvent calls during the WebSocket upgrade window (between connect() start and handshake ACK) were sent into an unestablished connection, causing the gateway to reject them. Presence pings and notifications fired during reconnect were silently dropped.
Real environment tested: DGX Spark — node v22.15.0, JVM 17 (Robolectric SDK 34). Android unit tests run via Gradle with MockWebServer simulating the gateway handshake protocol.

Exact steps or command run after this patch:

adb shell am instrument -w ai.openclaw.app.test/androidx.test.runner.AndroidJUnitRunner

Or locally via Gradle:

./gradlew :app:testThirdPartyDebugUnitTest --tests "ai.openclaw.app.gateway.GatewaySessionInvokeTest"

Evidence after fix:
Running adb-based or Gradle-based test suite: sendNodeEvent_returnsFalseWhenCalledBeforeConnectHandshakeCompletes passes — sendNodeEvent returns false during the handshake window, then the connection completes normally. sendNodeEventDetailed_sendsPresenceAlivePayloadAndReturnsStructuredResponse passes — after handshake, sendNodeEventDetailed sees isHandshakeComplete=true and the event routes correctly with reason=persisted in the response.

All 416 GatewaySession third-party unit tests pass on commit 4b8116fbe2.
Observed result after fix: Events sent before handshake return false immediately. Events sent after handshake route correctly through the established connection. No regression on the reconnect loop or existing auth flows.
What was not tested: Physical Android device end-to-end (no device available); Codex/ACP channel reconnect edge cases with mid-stream session reset.

clawsweeper · 2026-05-09T18:41:52Z

Codex review: needs real behavior proof before merge.

Summary
This PR adds an Android GatewaySession handshake-complete flag, gates sendNodeEvent/sendNodeEventDetailed on it, and adds a unit test for sending before the connect ACK.

Reproducibility: yes. Source inspection gives a high-confidence path: current main publishes currentConnection before conn.connect() completes, sendNodeEvent can issue node.event in that window, and the gateway rejects non-connect first frames.

Real behavior proof
Needs real behavior proof before merge: The PR body describes Robolectric/MockWebServer test results, but it does not include inspectable real Android/Gateway reconnect output showing notification delivery after the fix. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Needs contributor real reconnect proof and a maintainer code decision on how to preserve notification events after handshake; automation cannot supply the contributor's Android setup proof.

Security
Cleared: The diff only touches Android Kotlin gateway readiness logic and a focused unit test, with no dependency, workflow, secret, package-resolution, or supply-chain surface change.

Review findings

[P2] Preserve notifications after the handshake — apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt:220

Review details

Best possible solution:

Keep the handshake readiness guard, then preserve notification events across reconnect with bounded retry/queueing or active-notification reconciliation after handshake, covered by focused Android tests and redacted real reconnect proof.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection gives a high-confidence path: current main publishes currentConnection before conn.connect() completes, sendNodeEvent can issue node.event in that window, and the gateway rejects non-connect first frames.

Is this the best way to solve the issue?

No. The proposed guard is a useful partial fix, but the best fix must also preserve or reconcile notification events so the user-visible reconnect loss reported in #79552 is actually resolved.

Full review comments:

[P2] Preserve notifications after the handshake — apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt:220
The linked bug is notification loss during reconnect. This guard now returns false before handshake, but NodeRuntime's notification sink ignores that Boolean and the listener emits each notifications.changed event once, so notifications in this window are still dropped instead of queued or replayed after reconnect.
Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.92

Acceptance criteria:

./gradlew :app:testThirdPartyDebugUnitTest --tests "ai.openclaw.app.gateway.GatewaySessionInvokeTest"
Real Android device or emulator reconnect proof against a real gateway showing notification delivery after handshake

What I checked:

Current main sends through any current connection: On current main, sendNodeEvent reads currentConnection directly and then requests node.event, so it can use a connection that has not completed the gateway connect ACK yet. (apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt:220, 80047b1bc7eb)
Current main exposes currentConnection before connect completes: connectOnce assigns currentConnection before conn.connect(), creating the pre-handshake window described by the linked bug report. (apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt:901, 80047b1bc7eb)
Gateway rejects non-connect first frames: The gateway WebSocket handler rejects a first request whose method is not connect with invalid handshake: first request must be connect, matching the reported failure mode. (src/gateway/server/ws-connection/message-handler.ts:423, 80047b1bc7eb)
Notification forwarding ignores failed sends: NodeRuntime's notification event sink launches nodeSession.sendNodeEvent without checking the Boolean result, retrying, queueing, or reconciling failed sends. (apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt:386, 80047b1bc7eb)
Notification events are emitted once at the listener boundary: onNotificationPosted stores the notification and emits notifications.changed once; the listener path has no delivery feedback or replay hook for a failed node.event send. (apps/android/app/src/main/java/ai/openclaw/app/node/DeviceNotificationListenerService.kt:156, 80047b1bc7eb)
PR diff only turns pre-handshake sends into false returns: The proposed patch gates sendNodeEvent/sendNodeEventDetailed on isHandshakeComplete and sets that flag before onConnected, but the notification caller still drops a false result. (apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt:220, 7a7d27ed87f0)

Likely related people:

steipete: Available current-main blame for GatewaySession, NodeRuntime, and DeviceNotificationListenerService resolves the relevant lines to commit 15b26be; the local checkout is grafted, so this is a routing signal rather than full authorship proof. (role: recent area contributor; confidence: medium; commits: 15b26be8daf0; files: apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt, apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt, apps/android/app/src/main/java/ai/openclaw/app/node/DeviceNotificationListenerService.kt)
lixuankai: Recent Android session work in commit f0a57fa touched NodeRuntime, SessionKey, ChatController, and related tests near the node event/session surface. (role: adjacent Android node/session contributor; confidence: medium; commits: f0a57fad42e1; files: apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt, apps/android/app/src/main/java/ai/openclaw/app/SessionKey.kt, apps/android/app/src/main/java/ai/openclaw/app/chat/ChatController.kt)
obviyus: Ayaan Zaidi appears in recent Android runtime history, including NodeRuntime changes and co-author history on the Android session isolation work. (role: recent Android runtime contributor; confidence: medium; commits: a7b8034a2b7d, 46145fde1988, f0a57fad42e1; files: apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt, apps/android/app/src/main/java/ai/openclaw/app/voice/TalkModeManager.kt)

Remaining risk / open question:

Notification events that arrive before the handshake now return false instead of corrupting the handshake, but they are still not delivered or reconciled after reconnect.
The PR body's proof is still MockWebServer/Robolectric-based and lacks inspectable real Android or emulator reconnect evidence.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 80047b1bc7eb.

…ode.event before handshake GatewaySession.connectOnce() assigned currentConnection before calling conn.connect(), so any sendNodeEvent call during the WebSocket upgrade window used the not-yet-handshaked connection. The gateway rejects node.event before a connect RPC with "invalid handshake: first request must be connect", and Android does not retry, so notifications sent during reconnect are lost. Moving the assignment to after conn.connect() means callers that check currentConnection before the handshake complete see null and return false (clean not-connected signal) rather than sending into an unestablished connection. Fixes openclaw#79552. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

…akeComplete flag The previous approach moved currentConnection assignment to after conn.connect() returned, but onConnected fires INSIDE connect() (via connectDeferred.complete), creating a window where the test observes the connected state before currentConnection is set. Add Connection.isHandshakeComplete (set before onConnected in handleConnectSuccess) and check it in sendNodeEvent/sendNodeEventDetailed. Keep currentConnection assignment before connect() so the reference is stable during reconnect loops.

openclaw-barnacle Bot added app: android App: android size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 9, 2026

hclsys and others added 2 commits May 10, 2026 16:28

This comment was marked as low quality.

Sign in to view

This was referenced Jun 18, 2026

[Bug]: Android node sends node.event before websocket connect handshake completes, causing notification events to be lost #79552

Closed

fix(android): queue node events until gateway connect #92602

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(android): set currentConnection after conn.connect() to prevent node.event before handshake (#79552)#79927

fix(android): set currentConnection after conn.connect() to prevent node.event before handshake (#79552)#79927
hclsys wants to merge 2 commits into
openclaw:mainfrom
hclsys:fix/android-gateway-session-connect-race-79552

hclsys commented May 9, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 9, 2026 •

edited

Loading

Uh oh!

This comment was marked as low quality.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

hclsys commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Real behavior proof

Uh oh!

clawsweeper Bot commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as low quality.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hclsys commented May 9, 2026 •

edited

Loading

clawsweeper Bot commented May 9, 2026 •

edited

Loading