Sign up
View stargazersSign up

fix(server): preserve reconnect replay progress#28404

open2
main
maria/takeover-pr-26422
opened18h agobyMmaria
7 files changed
View on GitHubTwo-way sync enabledPR last imported 8h ago

Problem

Large reconnect catch-up falls back to a snapshot that cannot advance the client's cursor until fully received. Repeated interruptions can discard all transfer progress.

Change

Replay existing-cursor catch-up in lazy pages bounded by event count and restored/wire payload bytes. Preserve ordering, unknown-event skipping, live handoff and recreated-thread snapshots without a wire change. Long absences can transfer more total bytes than a snapshot; pages remain bounded and keep their progress. Reuse Qard's byte-bounded event-store implementation and remove the duplicate legacy replay-size/fallback branch. Update the existing transfer-report assertion to expect replay.

Scope and approval

Maintainer-requested takeover of the focused reconnect fix, accepted by contribution triage under policy cb9629e689. No new workflow, setting or wire contract.

Verification

Passed: 79 focused event-store, WebSocket, thread-stream and client-sync tests, plus the real HTTP/WebSocket transfer integration check. Three interrupted catch-up cases fail on current main at snapshot fallback and pass here. Final Blacksmith vp run check:changed 54c5f25249961a1aa0b61932329092eadbd9d6fc passes, including nine package typechecks, lint, format, knip and 74 adjacent tests. Both independent whole-diff reviewers approved 7b5d2d7453 with no findings.

Real Linux web client with Replay (dev), four completed turns and the same initial IndexedDB cache: main requests cursor 62 twice when its snapshot frames are interrupted. This head delivers 128 events before interruption, reconnects from 190, receives the remaining events and the completion marker, and clears syncing. Browser interception drops whole RPC frames, representing an interrupted undecodable frame; this is controlled fault injection, not a real WAN reproduction. The baseline loads the exact main WebSocket/event-store modules from 54c5f2524996 through a temporary Node loader; other code and the database are identical. The follow-up commit changes only an integration assertion, so production code is unchanged across the captures.

Unverified: native iOS/Android, Electron, live paid providers and real remote/relay/tunnel networks. All 33 CI checks pass on 7b5d2d7453; the review-bot follow-up reports no actionable findings.

Evidence

Same thread, cache, data and 1280×800 viewport. The recording omits startup/loading frames and shows main followed by the replacement. Captures demonstrate behavior; no performance claim is made.

Before: interrupted snapshots keep the cached thread syncing

After: paged catch-up restores all four turns and clears syncing

Interrupted catch-up on main and the replacement

Takeover

Replaces #26422, with thanks to Qard for the original implementation.

Built with GPT-6.1-Sol in the Codex harness on T3 Code.

Checks

23 successful · 10 skipped
All checks passed
7b5d2d7

Added 1 commit

  • fix(server): preserve reconnect replay progress42289be
MmacroscopeappBot18h ago
Synced from GitHub

Approvability

Verdict: Approved at 7b5d2d7

Macroscope's review found this PR approvable — Large reconnect catch-ups now preserve incremental progress through lazy, bounded replay pages rather than restarting with a snapshot. The change is limited to read-only delivery and retains recreated-thread snapshot handling and live-stream handoff.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

Ggithub-actionsBot17h ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire5.0 KiB5.0 KiB+29 B (+0.6%)6.8 KiB✅
CodexThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
CodexLive turn WebSocket wire1.2 KiB1.2 KiB+29 B (+2.3%)2.0 KiB✅
CodexLive turn WebSocket decoded21.5 KiB21.6 KiB+41 B (+0.2%)24.4 KiB✅
CodexLive turn messages12+1 (+100.0%)8✅
ClaudeTotal thread wire5.0 KiB5.0 KiB0 B (0.0%)6.8 KiB✅
ClaudeThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
ClaudeLive turn WebSocket wire1.3 KiB1.3 KiB0 B (0.0%)2.0 KiB✅
ClaudeLive turn WebSocket decoded21.9 KiB21.9 KiB0 B (0.0%)24.4 KiB✅
ClaudeLive turn messages220 (0.0%)8✅

Baseline: 54c5f25 · PR result: 7b5d2d7 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Added 1 commit

  • test(server): expect incremental reconnect transfer7b5d2d7
CcoderabbitaiBot17h ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 42289be7fd0834479cfb99f3331e1e6b1a3aa5ec and 7b5d2d7453e81510cb0a437f25bf9e7ec6166814.

External image Files selected for processing: 1
  • apps/server/integration/transferBudgetV2.integration.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

The event store now supports byte-bounded replay pages and sequence-range event checks. WebSocket thread catch-up reads and coalesces replay pages as the stream is consumed. Tests cover payload handling, pagination, interrupted catch-up, and reconnecting during synchronization.

Changes: Thread replay and resume

Layer / File(s)Summary
Event store paging and event checks
apps/server/src/persistence/OrchestrationEventStore.ts, apps/server/src/persistence/OrchestrationEventStore.test.ts, apps/server/src/persistence/OrchestrationEventStore.sequence.test.ts		readAgentEvents accepts an optional byte limit and reads events in sequence order within that budget. The store adds hasAgentEvent for matching V2 thread events within an exclusive/inclusive sequence range. Tests cover paging, payload handling, and event matching.
Paginated WebSocket catch-up
apps/server/src/ws.ts, apps/server/src/ws.test.ts, packages/client-runtime/src/state/threads-sync.test.ts, apps/server/integration/transferBudgetV2.integration.test.ts		Thread replay uses byte-bounded pages, counts delivered events, and coalesces pages as they are consumed. Tests cover catch-up resumption, reconnecting during partial synchronization, and replay mode.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant ThreadClient
3 participant subscribeOrchestrationV2Thread
4 participant OrchestrationEventStore
5 ThreadClient->>subscribeOrchestrationV2Thread: Resume after last received sequence
6 subscribeOrchestrationV2Thread->>OrchestrationEventStore: Read replay page with byte limit
7 OrchestrationEventStore-->>subscribeOrchestrationV2Thread: Return events and sequences
8 subscribeOrchestrationV2Thread-->>ThreadClient: Deliver coalesced events
9 subscribeOrchestrationV2Thread->>OrchestrationEventStore: Read next page after last page sequence

Suggested labels: bugfix, takeover

Suggested reviewers: juliusmarminge, yash-singh1, t3dotgg

Merge Risk: External image Minimal · up to 7b5d2

The reconnect replay changes have no established failure requiring resolution before merge.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title is concise, specific, and accurately describes preserving reconnect replay progress.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Description checkPassedThe description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the reconnect failure, the implementation, approval context, focused and integration …
Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the reconnect failure, the implementation, approval context, focused and integration test results, evidence, limitations, and agent details. The scope section does not include a direct approval link or quoted approval comment, but it provides sufficient approval context for a mostly complete description.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
CcoderabbitaiBotreviewed17h ago
Synced from GitHub
External image Additional comments: 1
apps/server/src/ws.ts: 1

960-974: External image Trivial · Performance & Scalability

External image Unverified finding Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Confirm that unbounded total catch-up replay is intended.

The removed decideThreadResume branch sent a snapshot when the replay gap exceeded the event or byte limit. The new code has no total limit. THREAD_RESUME_MAX_REPLAY_EVENTS and THREAD_REPLAY_PAGE_BYTES now bound each page only. A client that reconnects after a long absence now receives every missed event over the socket. This matters for remote, relay, and tunnel connections. A bounded snapshot would be smaller in that case.

Interruption progress does not require dropping the total limit. The cursor advances with every delivered page, so the remaining gap shrinks. You can keep a hard limit on the remaining gap. You can compute it with getAgentReplayStats or a similar count. When the gap exceeds the limit, fall back to snapshotThenLive(). If the removal is intentional, add a comment that documents the decision.

Source: Coding guidelines

External image Review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 54c5f25249961a1aa0b61932329092eadbd9d6fc and 42289be7fd0834479cfb99f3331e1e6b1a3aa5ec.

External image Files selected for processing: 6
  • apps/server/src/persistence/OrchestrationEventStore.sequence.test.ts
  • apps/server/src/persistence/OrchestrationEventStore.test.ts
  • apps/server/src/persistence/OrchestrationEventStore.ts
  • apps/server/src/ws.test.ts
  • apps/server/src/ws.ts
  • packages/client-runtime/src/state/threads-sync.test.ts

Limit details: You’ve used all 10 included reviews currently available.

Mmaria17h ago

Note

Written by gpt-6.1-sol on behalf of Maria

The total catch-up gap is intentionally uncapped. Falling back to a snapshot before delivering the first page recreates this bug: an interrupted snapshot leaves the cursor unchanged, so the same remaining gap exceeds the cap on every retry. Main's real-client capture repeats cursor 62; paged replay advances to 190 and finishes after reconnection. Pages remain bounded in count and restored/wire bytes, with an indivisible-event exception. A long absence can therefore transfer more total bytes than a snapshot; that tradeoff preserves progress. No code change for the unverified trivial finding.

Mmaria13h ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 7b5d2d7453e81510cb0a437f25bf9e7ec6166814 on linux with the actual dev web client, repository Playwright and /usr/bin/google-chrome. created a real git repository and project through the app. real codex gpt-6.1-sol turns produced an initial reply and 28 separate shell tool calls. no replay provider, mocks, fixtures or fabricated event data.

reproduced on github/main at 5732bbee39 in a separate installed worktree first. using the same history and initial indexeddb thread cache, interrupted main's catch-up snapshot three times: each retry requested cursor 142 and the client remained on "syncing messages...". stopped main before running the exact pr head.

the pr delivered 128 events, advancing cursor 142 to 270. after dropping the next catch-up frame and disconnecting, the client requested cursor 270, received the remaining 56 events and synchronization marker, displayed the completed history, and cleared syncing. interruption was controlled browser websocket fault injection that drops an undecodable whole rpc frame, not a real wan outage. no performance claim.

neighboring behavior passed: a third real codex turn streamed after recovery and rendered LIVE_AFTER_RECONNECT_OK exactly once. a cold client loaded the bounded http snapshot (200), displayed all three replies, synchronized from cursor 360, and exposed all 28 commands in the expanded history.

72 focused event-store, websocket and client-sync tests passed through blacksmith this run. the existing head has 33 successful ci checks, zero failing or pending. no source changes or pushes were needed.

main: interrupted catch-up remains syncing

pr: interrupted catch-up restores completed codex history

main then pr: interrupted catch-up and recovery

short runtime transcript. the animated recording removes startup frames and labels main/pr. uploaded images rendered and uploaded video playback and seeking were checked.

Not verified

native electron, ios/android, macos/windows, other providers, real wan/relay/tunnel interruptions, and a live byte-limit/oversized-event scenario. this vm exercised linux chrome over the local dev server with real codex through the configured local proxy.