[adalundhe]

Stacked on #3397.

Story

When a conversation's runtime crashed, chat says so and offers to revert it to its last snapshot.

• Client: the API client maps GetSession's recovery into the conversation domain type and adds a revert operation that calls RevertSession (feat(session): revert crashed runtimes through RevertSession #3395). The mock transport passes recovery through and refuses reverts with FAILED_PRECONDITION, because its fixtures have no crashed runtime.
• Alert: when recovery.needed is set, chat shows an alert offering the revert. The existing fork action stays available for saved checkpoints.
• Crash detection: an idle conversation re-reads the Session on the existing four-second live refresh while the tab is visible, so a crash after a turn settles is found without keeping the one-second lifecycle poll running. The shared page does the same.
• Discarded inputs: inputs the restored runtime does not remember (failed_messages, feat(session): report crash recovery and discarded inputs in GetSession #3397) are marked "Not remembered after recovery" across their whole turn. The public transcript is kept.

UI only. No backend changes.

Testing

• vitest: SessionRecoveryAlert, ChatTranscript.recovery, AgentChatPage.recovery, operations.

🤖 Generated with Claude Code

[EItanya]

🤖 AI-generated review.

Reviewed with #3409 landing first and this stack rebased on top.

[EItanya]

Can we include Session metadata in the existing visible-conversation refresh? A task can settle and the page can read needed=false before the Actor crashes during automatic suspension. isBusy is then false, so Session polling stops, while useLiveTranscript refreshes only A2A history. Recovery controls stay hidden and the composer stays enabled until another trigger refreshes the Session. A page-level test that changes the backend from healthy to crashed after mounting would cover this; injecting new recovery props bypasses the discovery problem.

[jjamroga]

+1. I'd fold the Session read into the visible-conversation refresh useLiveTranscript already does rather than widening the lifecycle poll (see my comment on lines 452–453 about the poll itself).

[adalundhe]

The live refresh now reads the Session on the same 4-second tick as the transcript, while the tab is visible, as Jonathan suggested; the lifecycle poll is unchanged (24ce9f72). SharedAgentPage does the same, so its failure markers update after the owner recovers. AgentChatPage.recovery.test.tsx mounts the page healthy, switches the fake backend to crashed, and expects the banner and a disabled composer after one tick. That test fails if the refresh reads only the transcript. On kind, I killed the workers right after a turn finished. Before this change, no banner appeared in 90 s and the composer stayed enabled. After it, the banner appeared 3.8 s later.

before-idle: {"event":"no banner within 90s of the crash"} {"event":"composer","disabled":false}
after-idle:  {"event":"banner shown without reload","seconds":3.807} {"event":"composer","disabled":true}

[jjamroga]

Left comments inline. Most of them are about how the page finds out about a crash, and whether we need a second way to fork.

Tested locally: I stood this up on kind at 37bf46e, sent a long prompt, and deleted the worker pod mid-stream. The banner showed up, Revert brought the conversation back, and the composer re-enabled. The comments on ChatMessageItem.tsx, ChatTranscript.tsx and the banner copy come from what I saw on screen after that.

[jjamroga]

This one is on me, I suggested showing revert or fork when the runtime crashes. Seeing it built, every snapshot divider in the transcript already has a Fork button (ChatTranscript.tsx#L412), and it stays visible during recovery: readOnly only hides the pending-question alert (L434) and Retry (L469). So this Select + "Fork snapshot" ends up being a second control for the same thing.

I think the banner could just offer Revert and point people at the snapshots in the transcript. If the picker exists to hide non-READY snapshots, gating the divider's Fork on canForkFrom would cover both paths, since right now the divider doesn't check (CheckpointDivider.tsx#L239-L253).

Tested locally: an alert-only version (dropping the picker, isForking, and the onFork/checkpoints props) passes tsc, eslint, and yarn vitest run src/components/chat src/pages 83/83 (85 minus the two picker tests).

[adalundhe]

Agreed. The banner now offers only Revert, and the snapshot line's Fork button appears only when the snapshot's record is loaded and READY (60531473). The divider never checked, so before this change it also offered Fork on failed or deleting snapshots. On kind, the crashed banner went from Revert, a picker and Fork to Revert alone. The snapshot line in the transcript still shows Fork. New tests: CheckpointDivider.test.tsx covers missing, creating, failed, deleting and unknown snapshots, and SessionRecoveryAlert.test.tsx checks that the banner shows one button and no combobox.

before: {"buttons":["Revert to last snapshot","Fork snapshot"],"pickers":1,"dividerForks":1}
after:  {"buttons":["Revert to last snapshot"],"pickers":0,"dividerForks":1}

[jjamroga]

On kind with no saved snapshots, this text still says "or start a new conversation from a saved snapshot", but the picker below is hidden because ready is empty, so the banner offers something it doesn't show. Goes away if we drop the picker (my comment on L50-L63); otherwise I think the copy should depend on ready.length.

[adalundhe]

The copy now mentions forking only when at least one snapshot is READY (60531473). Unit tests cover the case with no snapshots and the case where none are READY. On kind, the idle-crash conversation had no saved snapshot, and its banner reads: "Revert to the last runtime snapshot to continue this conversation. The reverted agent does not remember messages sent after that snapshot." The conversation with a READY snapshot also gets "To start a new conversation instead, fork a saved snapshot from the transcript."

[jjamroga]

Separate from Eitan's thread on L453 (that one is about never noticing an idle crash): once a crash is noticed, recovery?.needed === true keeps isBusy true, so a crashed conversation left open re-reads the Session every second until someone acts (poll at L478-L489). With the Session READY and the Actor CRASHED, every GetSession also hits Substrate GetActor.

The comment just below (L491-L498) says an idle page shouldn't make requests, and a crashed page waiting on a human is pretty idle. If the open-ended poll is there to catch automatic recovery (#3399) or another tab, could it only poll while operation === "resume" or isReverting?

[adalundhe]

recovery.needed no longer keeps the one-second poll running (24ce9f72). The poll now runs only while a revert is in flight or the controller holds a resume operation. While the page waits on the reader, the 4-second live refresh from Eitan's thread catches a recovery made in another tab or by the controller. The page test expects no Session read before the next live tick, and it fails if recovery.needed goes back into isBusy. Controller log on kind, from the crash to Revert:

before: GetSession 19:43:21 19:43:22 19:43:23 ... 19:43:59 19:44:00   (one per second until Revert)
after:  GetSession ... 19:39:43 19:39:47 19:39:51 19:39:55 19:39:59 19:40:03 19:40:07   (every 4 s once crashed)
browser count in a 20 s crashed window: before 20, after 5

Full lines: before, after.

[jjamroga]

Nothing covers the page-level changes here.

Tested locally: I reverted all three together (this canSend check, recovery?.needed || isReverting in isBusy on L452-L453, and the instance.refresh() after revert on L440), and yarn vitest run src/pages src/components still passes 235/235.

So nothing checks that the composer is disabled while recovery is needed, or that a revert refreshes the Session and transcript. I think a small page test with a mocked revertSession is worth adding, maybe alongside the healthy → crashed test from Eitan's thread on L453.

[adalundhe]

Added ui/src/pages/AgentChatPage.recovery.test.tsx (24ce9f72). It renders the real page against a fake SessionService. It checks three things: a healthy page finds a crash that happens later, a crashed page waits for the live tick instead of polling every second, and Revert re-reads the Session and transcript, hides the banner and re-enables the composer. I checked each case by reverting its code. Reverting canSend, the live Session read, recovery.needed in isBusy, or refreshTranscript() after a revert each fails at least one case. Dropping only the instance.refresh() after a revert still passes. The page already re-reads the Session when it leaves the busy state, so the test pins the result rather than that one call. On Node 24, tsc, eslint (0 errors, 2 existing warnings), and vitest src/components/chat src/pages src/api (372 tests) pass.

[jjamroga]

Two things on this note:

• Wording. On kind, the message I got this on had actually sent, and the agent streamed half a story before I killed the worker. "Failed to send" reads as if it never left. Same point as my comment on feat(session): report crash recovery and discarded inputs in GetSession #3397, "the runtime doesn't remember this turn" (or similar) fits what actually happened.
• role="status" makes this a live region. It's persisted, so a screen reader announces every one each time the transcript loads. Plain text or aria-describedby on the message seems enough.

[adalundhe]

Both fixed in 56a72f5d. The note now reads "The runtime crashed during this turn. After recovery, the agent does not remember this message or the replies after it." It is plain text, not a live region. A test asserts that it has no role="status" or aria-live ancestor. On kind, after a revert, the discarded message had one live region before this change and none after:

before: {"role":"user","liveRegions":1,"text":"Tell me a long story Failed to send — the recovered agent did not retain this message."}
after:  {"role":"user","discarded":"message","liveRegions":0,"text":"Tell me a long story The runtime crashed during this turn. After recovery, the agent does not remember this message or the replies after it."}

The same wording is in docs/architecture/runtime-and-lifecycle.md and docs/lifecycle-retries.md. Those files belong to #3397, so they change there.

[jjamroga]

failed only marks the user message. After reverting on kind, the rest of the turn looked off:

• The crash note renders above the partial reply. "The runtime crashed before this turn completed…" (the failed task status written in session_recovery.go#L242-L245, from feat(session): report crash recovery and discarded inputs in GetSession #3397) shows up before the half-streamed answer, so it reads like the crash happened before the agent started. I'd expect it after the partial output. I didn't dig into whether that's history order or rendering order.
• The partial reply looks like a normal answer. Nothing marks it as something the reverted runtime doesn't remember. If the user message gets a note, I think the agent output from that same task needs one too, or the note should cover the whole turn.

[adalundhe]

Partial reply: fixed in 56a72f5d. Every later message in the same task as a discarded message is now dimmed and labelled "Not remembered after recovery". Marking starts at the first discarded message, not at the start of the task, so answers already captured in the snapshot stay unmarked.

Order: fixed by cd94db57 on #3397, which this branch now merges. The cause was the stored history, not the rendering. settleCrashedSessionTask wrote the crash note without kagent.dev/a2a/timeline-position, and a task with an unstamped entry falls back to the legacy interleave, which puts status messages before artifacts. cd94db57 stamps the note with its own timestamp. A controller with that change put the note after the partial reply on kind (order-reverted.png on evidence/3398-review):

after: user "Tell me a long story" → agent "The runtime crashed before…" → agent "Once upon a time…"
order: user "Tell me a long story" → agent "Once upon a time…" → agent "The runtime crashed before…"

[adalundhe]

Collapsed into #3399, which now carries this PR's commits with #3397's, stacked on #3395.