feat(android): resume chat from the afterSeq cursor after reconnect#100301
feat(android): resume chat from the afterSeq cursor after reconnect#100301steipete wants to merge 8 commits into
Conversation
openclaw-barnacle
Bot
added
app: android
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a671d9f89
ℹ️ 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".
| val retained = _messages.value.filterNot { messageIdentityKey(it) in deltaKeys } | ||
| val merged = retained + delta | ||
| prunePersistedOptimisticMessages(merged) | ||
| _messages.value = mergeOptimisticMessages(incoming = merged, optimistic = optimisticMessagesByRunId.values) |
There was a problem hiding this comment.
When reconnect catch-up succeeds on the cursor path, onDisconnected() has already cleared _sessionId, but this delta apply path only updates _messages. After a disconnect/reconnect with missed rows, the transcript looks recovered while startNewChatAwait() still sees hasLoadedParentSession == false and omits parentSessionKey/emitCommandHooks, so creating a new chat from the recovered session loses the parent linkage until the user performs a full refresh. Please mirror the wholesale path's _sessionId = history.sessionId update for cursor pages.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 9:17 AM ET / 13:17 UTC. Summary PR surface: Source +245, Tests +635, Docs +3, Other +520. Total +1403 across 16 files. Reproducibility: yes. from source inspection: disconnect clears Review metrics: 1 noteworthy metric.
Stored data model 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:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Repair the Android delta-merge state bugs, keep older-gateway full-history fallback, and land this only after the Gateway afterSeq contract is approved and merged. Do we have a high-confidence way to reproduce the issue? Yes, from source inspection: disconnect clears Is this the best way to solve the issue? No. The afterSeq owner boundary is plausible, but this patch is incomplete until successful cursor pages preserve the same session identity and optimistic-send bookkeeping guarantees as full-history refreshes. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f9e194e4a122. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +245, Tests +635, Docs +3, Other +520. Total +1403 across 16 files. View PR surface stats
Acceptance criteria:
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)
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
d522cc6 to
c9a5ce2
Compare
… pages hit the history byte budget
…budget trims a catch-up page
… pages The default (no offset/afterSeq) chat.history path numbered __openclaw.seq within its byte-bounded tail window, so clients seeding the afterSeq reconnect cursor from a plain full fetch would start from a window-local value and re-walk or duplicate transcript rows. Reuse the stats reader so recent-tail rows carry the same transcript-global seq as cursor and offset pages; the response envelope is unchanged.
c9a5ce2 to
eb669c2
Compare
…after reconnect ChatController now tracks the highest applied __openclaw.seq for the active session and, on the first healthy transition after a disconnect, pages chat.history with afterSeq/nextAfterSeq/hasMore to append only the rows missed while offline. Catch-up pages dedupe by the existing message identity keys and reuse the generation-tracked reconciliation; the seq baseline resets wherever messages reset (session switch, wholesale history replace). Gateways that reject or ignore afterSeq fall back to the existing full-history fetch. The duplicated full-history refetch blocks collapse into one fetch-and-apply helper. Part of #100197
8a671d9 to
5921f31
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5921f31d46
ℹ️ 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".
| prunePersistedOptimisticMessages(merged) | ||
| _messages.value = mergeOptimisticMessages(incoming = merged, optimistic = optimisticMessagesByRunId.values) |
There was a problem hiding this comment.
When a user sends a new message after healthOk flips true but before the reconnect catch-up finishes, _messages.value already contains that optimistic row. Passing merged into prunePersistedOptimisticMessages makes the optimistic row match itself, so its optimisticMessagesByRunId entry is removed before the gateway has confirmed it; later chat.send failures or terminal-error acks can no longer remove the unsent echo, leaving a stale user message in the transcript. Prune only against gateway-delivered delta rows before re-merging the still-pending optimistic messages.
Useful? React with 👍 / 👎.
eb669c2 to
262de5b
Compare
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Closing with the rejected cursor stack. #100267 is closed and #100277 was rewritten to use the existing Gateway-owned This branch cannot be repaired as an Android-only follow-up: it carries the closed Gateway protocol stack, and its delta path also leaves If Android needs active-run ownership recovery, the follow-up should consume the same existing run snapshot contract at the current |
1 participant
Stacked PR: based on the gateway branch
feat/gateway-chat-history-after-seq(#100267) and must land after it.Part of #100197
What Problem This Solves
After a gateway disconnect, the Android client refetched nothing automatically and any manual/terminal-event refresh re-downloaded the entire recent history page. On flaky connections this wastes bandwidth, delays the transcript, and re-renders rows the client already has.
Why This Change Was Made
The gateway now exposes an
afterSeqcatch-up cursor onchat.history(#100267, including the recent-tail fix that stamps transcript-global__openclaw.seqon default pages so a plain full fetch can seed the cursor baseline — a gap found while building this adoption). The client tracks the highest applied transcript seq per session and, on the first healthy transition after a disconnect, fetches only the missed rows and appends them through the existing identity/dedup and generation-tracked reconciliation, pagingnextAfterSeq/hasMorewith an advance-only guard. Version skew is the single required fallback: gateways that reject the unknown param (schemaadditionalProperties: false→ error) or ignore it (no cursor echo) get today's full-history fetch with wholesale replace and a baseline reset. Fresh sessions and session switches reset the baseline. The three duplicated full-refetch blocks collapsed into one helper pair (−55 lines of duplication).User Impact
Reconnects on the Android app resume the chat transcript by fetching only messages that arrived while offline, in order and without duplicates. No behavior change for fresh sessions, manual refresh, or older gateways.
Evidence
cd apps/android && ./gradlew :app:testPlayDebugUnitTest --tests 'ai.openclaw.app.chat.*'— 42 tests pass, including 6 newChatControllerReconnectCatchUpTestcases: delta append without duplicates, multi-page cursor loop, legacy no-echo fallback with baseline reset, legacy param-rejection fallback, seq baseline reset on session switch, fresh-session full fetch unchanged.server.chat.gateway-server-chat-b.test.ts116/116 (including the new transcript-global recent-tail seq test, now carried by feat(gateway): optional afterSeq catch-up cursor on chat.history #100267);chat-history-budget.test.ts18/18 post-rebase.Known gap: no live-device E2E; one CI/Testbox run of the gateway lane recommended before landing the stack (a final local rerun was blocked by a sibling session's repo-wide heavy-check lock — coverage documented in the PR discussion above holds).