fix(android): preserve chat sends across reconnect recovery#100551
Conversation
f6299ab to
09893c7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6299ab100
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **Android microphone capture:** treat negative `AudioRecord.read` results as fatal shared-session errors so both transcription and Talk capture stop cleanly after device loss. (#100028) Thanks @NianJiuZst. | ||
| - **Lean local-model tools:** trim media generation, TTS, and PDF tools from lean agent surfaces while preserving explicit config and runtime opt-ins. (#88881) Thanks @vincentkoc. | ||
| - **iOS development app identity:** keep the development app labeled OpenClaw while using its distinct debug icon to differentiate it from release builds. | ||
| - **Android chat recovery:** preserve optimistic user messages and locally owned runs while reconnect and sequence-gap history snapshots catch up, preventing sent messages from disappearing or stale runs from taking ownership. (#100197) |
There was a problem hiding this comment.
Remove the normal-fix changelog entry
This Android fix adds a CHANGELOG.md entry even though the root AGENTS.md Docs / Changelog rule says CHANGELOG.md is release-only and must not be edited for normal PRs or direct fixes because release generation owns it. Leaving this manual entry can make generated release notes duplicate or conflict with the release workflow; keep the release-note context in the commit/PR text instead and let the release process update the changelog.
Useful? React with 👍 / 👎.
09893c7 to
a3d183d
Compare
|
Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 10:48 PM ET / 02:48 UTC. Summary PR surface: Docs +1, Other +1716. Total +1717 across 14 files. Reproducibility: yes. for the source path: current main clears pending run state on disconnect and seqGap, while the PR adds focused tests for reconnect recovery, lost acknowledgements, and terminal/history reconciliation. I did not run a live Android repro in this read-only review. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Land the Android snapshot/lost-ACK recovery only after fixing the raw-prompt ownership bug, removing the manual changelog entry, and adding exact-head Android emulator/device or redacted ADB proof. Do we have a high-confidence way to reproduce the issue? Yes for the source path: current main clears pending run state on disconnect and seqGap, while the PR adds focused tests for reconnect recovery, lost acknowledgements, and terminal/history reconciliation. I did not run a live Android repro in this read-only review. Is this the best way to solve the issue? No as submitted: the Android snapshot/lost-ACK direction is a plausible maintainable fix, but the raw assistant auto-send prompt claim and release-owned changelog edit need repair before this is the best mergeable version. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 75c8753a2cb5. Label changesLabel justifications:
Evidence reviewedPR surface: Docs +1, Other +1716. Total +1717 across 14 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (1 earlier review cycle)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3d183dffa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (_pendingAssistantAutoSend.value != pendingPrompt) { | ||
| _assistantAutoSendInFlight.value = false | ||
| return |
There was a problem hiding this comment.
Claim the raw assistant prompt
When an assistant auto-send prompt contains leading/trailing whitespace, the Compose callers pass the trimmed value returned by resolvePendingAssistantAutoSend, while _pendingAssistantAutoSend still holds the raw value from handleAssistantLaunch. This comparison then fails, flips assistantAutoSendInFlight back to false, and returns before either sending or clearing the pending prompt, leaving the auto-send stuck and the chat UI treating it as still queued.
Useful? React with 👍 / 👎.
a3d183d to
0a74e1d
Compare
|
Land-ready on What was rewritten:
Proof:
Proof gap: live Android UI validation remains unavailable because the only attached ADB target was unauthorized. Deterministic controller tests cover reconnect, lost-ACK, terminal, timeout, and duplicate-send state transitions. |
|
Merged via squash.
|
…#100551) * fix(android): preserve chat recovery ownership * fix(android): retry reconnect recovery on ticks * chore(android): refresh native i18n inventory
…#100551) * fix(android): preserve chat recovery ownership * fix(android): retry reconnect recovery on ticks * chore(android): refresh native i18n inventory
Closes #100197
Follow-up to #100384.
What Problem This Solves
Fixes an issue where Android chat users could lose a sent-message row, duplicate it, or have a stale run take over the composer after reconnects, sequence gaps, lost acknowledgements, or delayed transcript persistence.
Why This Change Was Made
The controller now keeps one bounded ownership path from optimistic send through gateway acknowledgement, reconnect recovery, terminal delivery, and authoritative history reconciliation. It distinguishes definitive request rejection from unknown transport outcome, orders same-generation history responses, preserves the gateway's persisted idempotency identity, and keeps assistant auto-send ownership in the view model so UI effect restarts cannot dispatch twice.
User Impact
Sent messages remain visible while recovery catches up, active replies resume after reconnect, stale or cross-client terminal events cannot steal local streaming UI, and chat stays disabled until reconnect history has safely applied. Unknown-outcome sends retry reconciliation without creating duplicate turns.
Evidence
tbx_01kwtcpdgbex21fsw4mdv2jk2f(coral-crab), warmup run: https://github.com/openclaw/openclaw/actions/runs/28760001708./gradlew :app:testPlayDebugUnitTest --tests ai.openclaw.app.chat.* --tests ai.openclaw.app.ui.chat.ChatSheetContentTest --tests ai.openclaw.app.gateway.GatewaySessionInvokeTest.disconnectFailsPendingRpcWithUnknownOutcomeWithoutWaitingForTimeout --tests ai.openclaw.app.gateway.GatewaySessionInvokeTest.disconnectReportsUnknownOutcomeForFireAndForgetRpc --rerun-tasksktlintMainSourceSetCheckand every touched test file are clean. The repository-wide test-source ktlint task still reports only pre-existing violations inChatControllerCommandControlsTest,InvokeErrorParserTest, andGatewayConfigResolverTest.