[codex] Cancel Android gateway pending RPCs on close by NianJiuZst · Pull Request #98067 · openclaw/openclaw

NianJiuZst · 2026-06-30T06:52:42Z

What Problem This Solves

Android GatewaySession kept pending RPC waiters in a session-wide map, but an intentional disconnect/reconnect path closed the socket without cancelling pending requests. Those callers could wait until their request timeout even though the owning WebSocket had already been closed.

Why This Change Was Made

This gives each Connection its own pending request id set. RPC registration, response, timeout, cancellation, send failure, and close now remove ids consistently, and closeQuietly() cancels only the pending ids owned by that connection. The extra closed-connection check covers the race where a request registers against a connection that has just started closing.

User Impact

Android callers no longer sit on stale Gateway RPCs after an intentional disconnect or reconnect. Newer connections are not affected by cleanup from an older socket.

Evidence

cd apps/android && ./gradlew :app:testThirdPartyDebugUnitTest --tests ai.openclaw.app.gateway.GatewaySessionInvokeTest --tests ai.openclaw.app.gateway.GatewaySessionReconnectTest --tests ai.openclaw.app.gateway.GatewaySessionInvokeTimeoutTest
cd apps/android && ./gradlew :app:ktlintCheck fails only on untouched upstream files: apps/android/app/src/main/java/ai/openclaw/app/GatewayExecApprovals.kt and apps/android/app/src/main/java/ai/openclaw/app/ui/design/ClawSurfaces.kt.
git diff --check
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

clawsweeper · 2026-06-30T07:22:18Z

Codex review: needs maintainer review before merge. Reviewed July 2, 2026, 9:12 AM ET / 13:12 UTC.

Summary
The PR moves Android GatewaySession RPC waiters into each WebSocket Connection, cancels that connection's waiters on close, adds disconnect/reconnect regression tests, and updates native i18n line inventory.

PR surface: Other +152. Total +152 across 4 files.

Reproducibility: yes. source-reproducible: current main stores waiters in a session-wide map and closeQuietly closes sockets without draining pending RPCs, so a slow RPC can wait for timeout after disconnect. This read-only review did not run local tests, but the PR adds focused regression coverage for the failing path.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit ✨ media proof bonus
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Wait for the remaining required CI check runs to finish before merge.

Risk before merge

[P1] Live GitHub checks still showed QA Smoke CI and two compact whole-check jobs in progress during this review, so final merge should wait for required checks to complete.

Maintainer options:

Decide the mitigation before merge
Land the per-connection pending-RPC cleanup after required checks finish, keeping the disconnect-cancellation and replacement-connection regression tests.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No ClawSweeper repair is needed; this open PR already contains the focused fix and proof, with only normal maintainer/check completion remaining.

Security
Cleared: The diff changes Android gateway lifecycle code, tests, and line-only i18n inventory metadata without adding dependency, workflow, secret, permission, or package-resolution surface.

Review details

Best possible solution:

Land the per-connection pending-RPC cleanup after required checks finish, keeping the disconnect-cancellation and replacement-connection regression tests.

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

Yes, source-reproducible: current main stores waiters in a session-wide map and closeQuietly closes sockets without draining pending RPCs, so a slow RPC can wait for timeout after disconnect. This read-only review did not run local tests, but the PR adds focused regression coverage for the failing path.

Is this the best way to solve the issue?

Yes. Per-Connection waiter ownership with synchronized registration and drain fixes the lifecycle boundary; a session-wide close drain would risk canceling replacement-connection waiters, which the new reconnect test guards.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b60e8c48382d.

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the PR comments include Android 16 emulator before/after screenshots for the disconnect/restart path, and the only later head commit is non-runtime i18n inventory alignment.
add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Sufficient: the PR comments include Android 16 emulator before/after screenshots for the disconnect/restart path, and the only later head commit is non-runtime i18n inventory alignment.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): Sufficient: the PR comments include Android 16 emulator before/after screenshots for the disconnect/restart path, and the only later head commit is non-runtime i18n inventory alignment.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: This is a bounded Android gateway lifecycle bug fix with limited blast radius and no evidence of a core-wide outage.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): Sufficient: the PR comments include Android 16 emulator before/after screenshots for the disconnect/restart path, and the only later head commit is non-runtime i18n inventory alignment.
proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the PR comments include Android 16 emulator before/after screenshots for the disconnect/restart path, and the only later head commit is non-runtime i18n inventory alignment.
proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Sufficient: the PR comments include Android 16 emulator before/after screenshots for the disconnect/restart path, and the only later head commit is non-runtime i18n inventory alignment.

Evidence reviewed

PR surface:

Other +152. Total +152 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	0	0	0	0
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	4	178	26	+152
Total	4	178	26	+152

What I checked:

Live PR state: The PR is open at head 4f20fd8, targets main b60e8c4, has maintainerCanModify enabled, and has no linked closing issue. (4f20fd82b0cb)
Current main still has the bug shape: Current main keeps RPC waiters in a session-wide pending map and closeQuietly closes the socket without draining pending waiters, so a request owned by a closing connection can remain pending until timeout. (apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt:183, b60e8c48382d)
Latest release is not already fixed: v2026.6.11 points at e085fa1 and still has the session-wide pending map plus a closeQuietly path that does not call failPending, so this is not implemented in the latest release. (apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt:179, e085fa1a3ffd)
PR implementation owns waiters per connection: At PR head, Connection owns its own pending map, registerPending synchronizes registration with close draining, closeQuietly calls failPending, and failPending drains only that connection's waiters. (apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt:392, 4f20fd82b0cb)
Regression coverage covers both lifecycle invariants: The PR adds a disconnect test that expects a 30-second pending RPC to cancel within 2 seconds, and a reconnect-generation test proving an old connection drain cannot cancel a replacement connection's RPC. (apps/android/app/src/test/java/ai/openclaw/app/gateway/GatewaySessionInvokeTest.kt:132, 4f20fd82b0cb)
Caller and protocol boundary checked: NodeRuntime and TalkSpeakClient use GatewaySession request/requestDetailed/sendRequestFrame APIs, while the gateway protocol keeps the same req/res frame shape and node.invoke/node.event methods; the PR does not change protocol schema or versions. Public docs: docs/gateway/protocol.md. (docs/gateway/protocol.md:217, b60e8c48382d)

Likely related people:

vincentkoc: git blame on the current pending map, request registration, and closeQuietly paths points to 87178b8, whose GitHub commit metadata maps to @vincentkoc. (role: introduced current main behavior; confidence: high; commits: 87178b84668b; files: apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt)
obviyus: CONTRIBUTING.md lists @obviyus for the Android app, and git history shows Ayaan Zaidi among the most frequent contributors to GatewaySession and its tests, including the earlier Android app package move that carried this path. (role: Android app area owner and historical contributor; confidence: high; commits: 5568b393a86d, 98d593956493; files: CONTRIBUTING.md, apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt, apps/android/app/src/test/java/ai/openclaw/app/gateway/GatewaySessionInvokeTest.kt)
steipete: git history shows prior Android GatewaySession auth/connect work, and this PR's later commits refine the branch to per-connection waiter ownership and i18n alignment. (role: recent adjacent contributor and PR branch refiner; confidence: medium; commits: 589aca0e6d73, 29512c8a9084, 4f20fd82b0cb; files: apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewaySession.kt, apps/android/app/src/test/java/ai/openclaw/app/gateway/GatewaySessionReconnectTest.kt, apps/.i18n/native-source.json)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-07-02T13:00:07Z

Maintainer verification on exact head 726982e4d269427788d613e67ca0dc1fb0d262c3:

Refactored pending RPC ownership into each Connection; closing an old socket now drains only that generation's waiters.
Added deterministic disconnect cancellation and overlapping reconnect-generation regressions.
Passed focused GatewaySessionInvokeTest + GatewaySessionReconnectTest, Android ktlint, native i18n inventory, git diff --check, and fresh exact-tree autoreview (no actionable findings, confidence 0.94).
Hosted CI is green: 61 passed, 13 skipped.

Live Android 16 emulator proof: the exact-head APK moved from Not connected after the isolated Gateway stopped to Online and ready after restart. The deterministic overlap-generation test is the proof that an old connection's delayed close cannot cancel a replacement connection's RPC.

Gateway stopped	Gateway restarted

steipete · 2026-07-02T13:39:49Z

Merged via squash.

Prepared head SHA: 726982e4d269427788d613e67ca0dc1fb0d262c3
Landed commit: 0ccdef5dcf76ad816d2105f47b89da5c609fab45

* fix: cancel Android gateway pending RPCs on close * fix(android): isolate pending RPCs per connection * style(android): separate RPC waiter invariant * chore(android): align native i18n inventory --------- Co-authored-by: NianJiuZst <180004567+users.noreply.github.com> Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added app: android App: android size: S labels Jun 30, 2026

NianJiuZst marked this pull request as ready for review June 30, 2026 06:54

steipete self-assigned this Jul 2, 2026

steipete force-pushed the codex/android-gateway-pending-cleanup branch from b405058 to 8c6f818 Compare July 2, 2026 12:23

openclaw-barnacle Bot added size: M and removed size: S labels Jul 2, 2026

steipete force-pushed the codex/android-gateway-pending-cleanup branch from addd263 to 4f20fd8 Compare July 2, 2026 13:01

NianJiuZst and others added 4 commits July 2, 2026 14:28

fix: cancel Android gateway pending RPCs on close

a9b41e2

fix(android): isolate pending RPCs per connection

dce2251

style(android): separate RPC waiter invariant

8bf868a

chore(android): align native i18n inventory

726982e

steipete force-pushed the codex/android-gateway-pending-cleanup branch from 4f20fd8 to 726982e Compare July 2, 2026 13:28

steipete merged commit 0ccdef5 into openclaw:main Jul 2, 2026
74 checks passed

github-actions Bot mentioned this pull request Jul 2, 2026

📡 Upstream Digest — 2026-07-02 13:41 UTC curtismercier/openclaw-mods#1189

Open

This was referenced Jul 2, 2026

test: cover Android discovery and foreground startup gates #99206

Closed

test: cover Android main activity lifecycle gates #99215

Closed

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix(android): order disconnect after in-flight gateway work #103068

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[codex] Cancel Android gateway pending RPCs on close#98067

[codex] Cancel Android gateway pending RPCs on close#98067
steipete merged 4 commits into
openclaw:mainfrom
NianJiuZst:codex/android-gateway-pending-cleanup

NianJiuZst commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

NianJiuZst commented Jun 30, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

steipete commented Jul 2, 2026 •

edited

Loading