Repository navigation
fix(cloud-agent-sdk): keep failed sends scoped to the chat that sent them - #8387
Conversation
…them A send that fails after the user switched chats now removes the optimistic row it inserted from the transcript that received it, and records the prompt as that chat's draft. Returning to the chat therefore shows the text to retry and no unconfirmed row the server never received. The record is keyed by the pre-send session id: the active chat's failed-prompt state, billing state and send-failure callback stay untouched by the other chat's failure. The cached first page of the chat that sent is repaired too, so reopening it does not repaint the row, and an accepted send from a chat clears that chat's recorded draft.
iscekic
added
drain
|
bot: Visual proof from the local dev stack (real web app, Worker, Durable Object and sandbox with the deterministic fake model; owned fake test account). Flow exercised in one chat: type a prompt, hold the send request, switch to another chat while it is pending, abort the request, then return to the chat that sent.
A page reload after step 3 showed the same transcript, so the row never became server history.
|
![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 revision moves delivery detection to the page cached on the way out of a chat, which is the only surface that still holds a pre-switch confirmation after Overview
Files Reviewed (2 files)
Verification not performed in this read-only review: I did not run the package jest suite, typecheck, or lint; findings are from reading the incremental diff ( Previous Review Summaries (5 snapshots, latest commit 65e0aca)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 65e0aca)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe delivered detection added in this revision reads the send-time storage, which Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 7ba9ff4)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe confirmed-record guard only fires while the user is still on the sending chat, so a server-accepted send that settles after a switch is still restored as a draft for that chat and can be sent twice. Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit fe09a62)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe new guard correctly preserves a confirmed record in a reopened transcript, but the failure path still restores that already-accepted prompt as a draft, inviting a duplicate send. Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit be8389c)Status: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up commit fixes the previously reported ghost row on a reopened chat: cleanup now targets the mounted storage, the cached page is always repaired, and the draft/error writes key on the chat id instead of the session object; the new test covers the switch-away-then-back path. Files Reviewed (2 files)
Previous review (commit a2695e9)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe failed-send cleanup is scoped by the session's object identity, so a chat reopened before the send settles still shows the ghost optimistic row and receives no failure state. Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Reviewed by deepseek-v4.1-flash-1017 · Input: 79.8K · Output: 21.3K · Cached: 1.2M Review guidance: REVIEW.md from base branch |
A chat reopened while its send is still pending repaints the optimistic row from the page cached on the way out. The failure path now also clears the mounted transcript when it belongs to the chat that sent, keeps the prompt as that chat's draft, and repairs the cached page so a later switch away cannot store the row again. The failure state is keyed by the chat id rather than the session object, so a reopened chat still shows the retry and the error; other chats are unaffected.
|
bot: Visual proof for the reopened-chat path raised in review, from the same local dev stack and fake test account. Flow: type a prompt, hold the send, switch to another chat and come back while it is still pending, then abort.
|
…lands A send whose response is lost after the server accepted it can leave the confirmed record in a reopened transcript under the client's message id. The failure path now removes only the unconfirmed row from the mounted transcript, matching how a retried delivery failure is handled, so server history stays visible.
When a confirmed record for the client's message id is already in the chat, the server accepted the prompt and only the response was lost. The failure path now leaves that record in the transcript and records no draft, so the same prompt is not shown twice and cannot be sent again.
…witch The confirmed record for a prompt lands in the storage that sent it, so it is visible even when the user has already switched away. The failure path now reads that record too: a delivered prompt records no draft, and any draft a previous failure left for the chat is consumed, so returning to the chat does not offer a prompt the server already has.
Leaving a chat caches its page and then destroys the session, which clears the storage the prompt was sent from, so the confirmation is gone from that storage by the time the failure lands. The failure path now reads the chat's cached first page for the confirmed record of the message id, so a prompt the server accepted is not recorded as a draft to send again.
iscekic
added
the
merge:bot
Change
packages/cloud-agent-sdk/src/session-manager.ts: a send that fails after the user switched chats used to leave its optimistic user row in the transcript that received it, and to lose the prompt text, because the failure path returned as soon as the active session had changed.The failure path now does the chat-scoped work first and only then applies the active-chat guard:
remoteOptimisticIdsentry is dropped either way.failedPromptBySession, bounded to 20 sessions like the existing delivery-failure memory).switchSessionseedsfailedPromptAtomfrom that record, so returning to the chat shows the text to retry.readCachedSnapshotPage/writeCachedSnapshotPagehooks, so a chat reopened before the send settles cannot repaint the row, and a later switch away cannot store it again. Without those hooks (mobile, tests) there is no cached page to repair.failedPromptAtom,billingFailureAtom, the status indicator,config.onSendFailed) is keyed by the chat id, not the session object, so a chat reopened mid-send still gets its draft and error, while the chat the user is looking at never inherits another chat's failure.Tests in
session-manager.test.tscover the mid-send switch on the real manager and jotai storage: the row is gone from the sending storage and its cached page, returning to the chat paints no such row, the draft is restored for the sending chat only, an accepted send consumes it, a chat reopened before the send settles drops the repainted row while keeping the draft, a confirmed record under the same id survives a lost response with no duplicate draft, and a prompt the server confirmed after a switch records no draft.Verification
Package (this VM):
Each new test fails on the previous behaviour and passes with the change.
End to end on this VM against the local dev stack (
pnpm dev:start app agents fake-llm): real Next.js app, real cloud-agent-next Worker, Durable Object and sandbox, deterministic fake model, browser driven through the app's own UI:cloudAgentNext.sendMessagerequest so it stayed pending. Observed: the prompt renders in the transcript and the composer is cleared.Worker-side check during step 1: no file under
services/cloud-agent-next/.wrangler/state/v3/docontains the prompt text while the request is held or after the abort, so the aborted send never became server history and the missing row is the correct end state.Screenshots of steps 1–3 and of step 5 are attached in follow-up comments.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Storage + owning chat's cached page (Recommended)
Confidence: 0.80 · Precedent: Fix the root cause and the class of the issue on every affected surface: session 7f3470e0ba6f, "fix class of issue preventing progress"; the measured ghost repaint is the same defect on the cache surface.
Answer: Per-session map, bounded to 20 sessions (Recommended)
Confidence: 0.65 · Precedent: Performance reviewer asks what happens at 10x or 100x the data: session 55c9c80332a7, owner defined this reviewer. The bounded map follows the existing delivery-failure memory pattern.
Answer: On a successful send from that chat (Recommended)
Confidence: 0.75 · Precedent: UX must be best-in-class: session 00eb2a1d3e4b, "design and UX MUST BE BEST-IN-CLASS". Proposing already-delivered text would be a defect.