Repository navigation
fix(cloud-agent-sdk): keep the live stream when a snapshot read fails - #8539
Conversation
The initial connect awaited the stream ticket and the snapshot page in one `Promise.all`, so a rejected page read discarded an already-available ticket and no websocket opened. The reconnect catch-up reads attached no-op rejection handlers, so a failed catch-up left a transcript gap with no error, no retry and no signal. The ticket and the snapshot read now start together and only a failed ticket stops the connect. A failed read is reported once per episode through the transport error callback and retried with bounded exponential backoff (three retries, 500 ms base, 4 s cap) while the socket keeps serving live events. A retried page replays when it lands, records the initial page exactly once, and never regresses a replay cursor that live events already advanced. The initial bounded read also retries a typed `retryable_failure`, so a transient history read no longer leaves the transcript empty until the next reconnect.
iscekic
added
drain
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up commit keeps terminal typed history failures off the reconnect error path while still reporting them once on the initial open, and the added test covers the reconnect catch-up case. Assumptions: Files Reviewed (2 files)
Previous Review Summaries (2 snapshots, latest commit 724669e)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 724669e)Status: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up commit closes both earlier findings: Assumptions: Files Reviewed (2 files)
Notes: the guard reset in Previous review (commit a28ce8d)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe snapshot-read degradation fix is sound and well tested, but the initial retry chain reuses Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Notes: no new retry timer leak found — Reviewed by deepseek-flash-latest · Input: 54.1K · Output: 8.5K · Cached: 556.2K Review guidance: REVIEW.md from base branch |
|
bot: Android E2E on the local dev stack (nextjs + Metro + cloud-agent-next worker + deterministic fake model), emulator
Each frame was captured in the same command as its
|
…e dead retry cap A reconnect catch-up read can land while an initial retry is still in flight, and that retry then handed the manager a second page for the same connect, resetting the older-pages cursor back to the newest page. `replayInitialPage` now records the page only when nothing has seeded the session yet, so the first page wins whichever chain delivers it, and the reconnect path no longer needs its own copy of that decision. The 4 s delay cap was unreachable with three retries (500 ms, 1 s, 2 s), so the delay is the plain bounded exponential and the constant is gone. Adds a test where a reconnect catch-up wins the race against a late initial retry.
…t path `applySnapshotRead` reported every non-page outcome, so a reconnect catch-up that came back `too_large` or `invalid_data` raised the session error on every reconnect for history that can never be loaded. Reporting now covers only the transient outcomes the retry exists for; the terminal typed failure is still surfaced once by the initial read, exactly where it was before. Adds a test where a reconnect catch-up returns `too_large` and must not reach the error channel.
iscekic
added
the
merge:bot
…nsport-snapshot-read-retry # Conflicts: # packages/cloud-agent-sdk/src/cloud-agent-transport.ts
iscekic
added
merge:bot
Evidence
Evidence: demonstrated — the initial session exercised forced snapshot failures on the local Android app and observed continued live delivery. After merging main, a fresh loopback HTTP/WebSocket smoke exercised the merged SDK's initial-read and reconnect recovery paths.
pnpm --filter @kilocode/cloud-agent-sdk test --runInBand— 35 suites, 1,790 tests passed. Jest printed its delayed-exit warning and the command exited successfully.pnpm --filter @kilocode/cloud-agent-sdk typecheck, formatting, targeted oxlint, andgit diff --checkpassed.pnpm install --frozen-lockfilepassed.Change
retryable_failureoutcomes report once per failure episode and retry three times at 500 ms, 1 s, and 2 s. Missing sessions and terminal typed outcomes stop retries. Disconnect, destroy, and lifecycle changes cancel pending work.replaySnapshotPage,snapshotPageFromSnapshot, andsessionSnapshotFailureMessagehelpers, removing the transport-local duplicates.Coverage and limitations
Eight recovery tests cover initial rejection, bounded retries, both reconnect read seams, cursor non-regression, the initial/reconnect race, retryable typed failure, and terminal reconnect outcomes. The existing suite also covers replayed message parts and settled tool timing.
The original Android run used the real local web tRPC route, stream worker, and session ingest with deterministic inference. Its temporary read-failure patch was removed before commit. The fresh follow-up smoke uses a loopback fixture server, not the full application stack. No CLI relay protocol changed; this is the app-facing SDK transport.
Proxy answers
The owner's proxy gave these answers in the owner's place.
retryable_failurereconnect outcomes? Answer: Yes, with the same bounded backoff and one report. Confidence: 0.85. Precedent: Fix the class of failure, not only one symptom.getSessionMessagesPageroute. Confidence: 0.70. Precedent: Authorized local E2E fault injection; remove the patch before committing.retryable_failureoutcomes on the initial read too? Answer: Yes, with the same bounded backoff. Confidence: 0.85. Precedent: Apply the same recovery rule to the whole defect class and satisfy the initial-read requirement.