feat(gateway): optional afterSeq catch-up cursor on chat.history#100267
feat(gateway): optional afterSeq catch-up cursor on chat.history#100267steipete wants to merge 6 commits into
Conversation
openclaw-barnacle
Bot
added
docs
![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: 6d7fff6ebf
ℹ️ 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".
| const readPage = await readSessionMessagesAfterSeqWithStatsAsync(readScope, { | ||
| afterSeq, | ||
| maxMessages: max, | ||
| allowResetArchiveFallback: true, |
There was a problem hiding this comment.
When an afterSeq catch-up page ends between a stale subagent_announce user row and its adjacent assistant reply (for example limit: 1), this reads exactly max rows and nextAfterSeq advances before dropPreSessionStartAnnouncePairs has the paired assistant context. The first page drops only the announce row, then the next page starts at the stale assistant and renders it; the existing recent/offset history paths overread one row specifically to avoid this boundary leak, so the cursor path needs the same context or must avoid advancing past an uninspected pair.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 8:59 AM ET / 12:59 UTC. Summary PR surface: Source +245, Tests +635, Docs +3, Other +4. Total +887 across 13 files. Reproducibility: yes. Source inspection of the PR head shows the CLI-bound offset guard and the full-transcript cursor read/projection path; I did not run tests because this review is read-only. 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:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Keep the additive cursor direction only if maintainers accept the protocol contract, while preserving existing offset pagination, making afterSeq catch-up bounded with projection-safe context, removing release-owned changelog edits, and adding real reconnect proof before merge. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection of the PR head shows the CLI-bound offset guard and the full-transcript cursor read/projection path; I did not run tests because this review is read-only. Is this the best way to solve the issue? No. The additive cursor is a plausible layer, but this is not the best mergeable implementation until it preserves current offset behavior, bounds catch-up work, and includes real reconnect proof. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a5f0a37373a9. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +245, Tests +635, Docs +3, Other +4. Total +887 across 13 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 (2 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05c305a8b4
ℹ️ 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".
| messages: augmentChatHistoryWithCanvasBlocks( | ||
| projectChatDisplayMessages(recencyFilteredMessages, { maxChars: effectiveMaxChars }), |
There was a problem hiding this comment.
When an afterSeq page boundary splits a visible message tool send, e.g. the page contains only the assistant toolCall and the next page contains the successful toolResult/NO_REPLY, this projects each slice independently so mirrorMessageToolVisibleReplies never sees both sides and no page returns the user-visible sent text. This can happen whenever reconnect catch-up misses more than the requested limit (or a budget-trimmed page cuts there), so cursor paging should overread/carry projection context or slice after projection at safe transcript boundaries.
Useful? React with 👍 / 👎.
| const localReadPage = await readRecentSessionMessagesWithStatsAsync(readScope, { | ||
| ...localHistoryReadOptions, | ||
| maxBytes: Math.max(maxHistoryBytes * 2, 1024 * 1024), | ||
| allowResetArchiveFallback: true, | ||
| }); |
There was a problem hiding this comment.
For ordinary chat.history loads with no offset or afterSeq, this now calls readRecentSessionMessagesWithStatsAsync; that helper obtains totalMessages via readSessionMessageCountFromPathAsync, which builds readSessionTranscriptIndex over the full JSONL whenever the transcript stat changes. On large active sessions, every reload after an append loses the previous bounded tail-read behavior and scans/parses the whole transcript before returning the recent window, so the hot reconnect/load path can stall as history grows.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
05c305a to
d522cc6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d522cc66d1
ℹ️ 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".
| totalMessages: readPage.totalMessages, | ||
| }; | ||
| } | ||
| if (offset !== undefined && !hasImportedCliHistory) { |
There was a problem hiding this comment.
When a session has a Claude CLI binding, this guard skips the existing offset branch, so chat.history requests that explicitly pass offset fall through to the recent/full path and return no offset/nextOffset metadata. That breaks existing offset consumers: the sibling embedded gateway still treats params.offset !== undefined as a paged request and deliberately avoids merging full CLI imports at src/agents/tools/embedded-gateway-stub.ts:280-339, so load-more/history paging for CLI-bound sessions repeats the tail instead of advancing.
Useful? React with 👍 / 👎.
d522cc6 to
c9a5ce2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9a5ce2ac7
ℹ️ 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".
| const readPage = await readSessionMessagesWithStatsAsync(readScope, { | ||
| allowResetArchiveFallback: true, | ||
| }); |
There was a problem hiding this comment.
When a client reconnects with afterSeq on a large active session, this reads the entire transcript and then projects the whole message array before filtering down to the requested cursor window. The transcript index cache is keyed by file mtime/size, so every append invalidates it; a normal reconnect that missed only one new row still reparses/projects all prior history, which can stall the new mobile catch-up path that is meant to fetch only missed rows. The cursor path should read a bounded indexed window plus the needed projection context instead of making each catch-up proportional to full session size.
Useful? React with 👍 / 👎.
c9a5ce2 to
eb669c2
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.
eb669c2 to
262de5b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 262de5ba0f
ℹ️ 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".
|
|
||
| ### Changes | ||
|
|
||
| - **Gateway chat reconnect recovery:** add session-scoped `chat.history` sequence cursors so mobile clients can fetch only missed projected transcript rows after reconnecting, with lossless pagination and older-Gateway fallback. (#100197, #100267) |
There was a problem hiding this comment.
Root AGENTS.md says CHANGELOG.md is release-only and that release generation owns it, so this normal feature commit should not add an Unreleased entry directly. Leaving this in can conflict with the generated release notes or produce duplicate/manual release content; put the release-note context in the PR body or squash message instead.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Land-ready on exact head
Native iOS and Android consumers remain the two stacked follow-ups; both have clean client-specific autoreviews against this exact Gateway contract. |
|
Superseded by #100277. The Gateway already ships the canonical reconnect snapshot needed here through #100277 was rewritten against the existing contract. It restores active-run ownership client-side across bootstrap, foreground resume, sequence gaps, post-send retries, completion refreshes, session changes, and agent/global-session routing. The rewritten exact head has 128 focused Swift tests passing, a macOS package build, clean SwiftFormat, direct Gateway/Codex contract review, and a clean final autoreview. Closing this alternate protocol path so the cluster has one canonical fix. |
1 participant
Part of #100197
What Problem This Solves
Resolves a problem where reconnecting Gateway clients (Control UI, WebChat, native apps) had to refetch the entire
chat.historypayload after a WebSocket drop, even when they only missed a handful of messages. Offset pagination pages from the newest end and cannot express "give me only what I missed since point X".Why This Change Was Made
chat.historymessages already carry a stable, monotonic transcript sequence (__openclaw.seq, the 1-based visible-entry index over the active transcript branch). This change adds an optionalafterSeqcursor that returns only entries newer than that sequence, oldest-first, withnextAfterSeq/hasMorefor follow-up pages. Catch-up is lossless by design: when the history byte budget would trim a cursor page, it trims from the newest end at transcript-seq group boundaries — always delivering at least one complete seq group so mirror rows of a single raw entry are never split across the cursor — andnextAfterSeqonly advances past rows actually delivered, so trimmed rows arrive on the next page; untrimmed pages advance over the full raw range consumed so projection-filtered entries can never stall the loop. The change is additive: no protocol version bump, the parameter is optional,offsetpagination and its tail-preserving budget behavior are unchanged, and combiningafterSeqwithoffsetis rejected as invalid. The generated Swift protocol model gains the field with a defaulted initializer parameter so existing native call sites keep compiling.User Impact
API and app clients can resume a chat session after a reconnect by fetching only the messages they missed — losslessly, even for oversized backlogs that exceed the per-response byte budget. Clients that never send
afterSeqsee byte-identical behavior. Companion client PRs (iOS, Android) adopt the cursor and are stacked on this branch.Evidence
node scripts/run-vitest.mjs src/gateway/server.chat.gateway-server-chat-b.test.ts -t "afterSeq"— 5 passed: cursor returns only newer messages oldest-first; budget-constrained pages keep the oldest rows and iterating with the returned cursor recovers all messages exactly once in order; a page of only projection-filtered rows still advances the cursor; absent param keeps the existing response shape; invalid cursors rejected cleanly.node scripts/run-vitest.mjs src/gateway/server-methods/chat-history-budget.test.ts— 18 passed: head-preserving cursor cap keeps oldest rows, delivers an over-budget first seq group whole instead of splitting it, trims later groups at the prior boundary; tail-preserving budget helpers untouched.node scripts/run-vitest.mjs src/gateway/server.chat.gateway-server-chat-b.test.ts -t "pagination"— 2 passed: existing offset pagination (including the final-budgeted-page case) unchanged.oxfmt --checkclean on touched files;scripts/protocol-gen-swift.tsregeneration diff is exactly theafterSeqfield (4 lines).Documented degradation (unchanged class from offset pagination): a single seq group whose bytes exceed the entire budget ships as one over-budget page — deliberate, protects losslessness.