[adalundhe]

Stacked on #3395.

Recover crashed runtimes during A2A sends when config.sessions.automaticRecovery: true. The setting defaults to false.

The flag controls joining an admitted recovery, retrying a conflicting send reservation, and reverting after a proven unaccepted send. With it off, sends keep their existing refusals. Explicit RevertSession calls remain available.

Missing or replaced runtimes are recorded as FAILED/NONE with reason RuntimeLost. These persistence changes apply with either flag value. The flag does not control migrations, schema, or stored data.

Chat reads recovery status from stored Session state. It retries KAGENT_SEND_NOT_ACCEPTED before the first stream frame, with the same message ID and a 30-second bound. Session reads do not query Substrate.

Validation:

• Gateway tests cover all three disabled paths and the existing enabled paths.
• Config tests cover the default, both values, Helm rendering, and the deployment checksum.
• PostgreSQL-backed Session, gRPC, and scheduled-run controller tests pass.
• Protobuf lint, generation, and compatibility checks pass.
• Go lint passes. Live crash-recovery E2E explicitly enables the flag in CI.

[adalundhe]

Agreed. The scan needed one interval per 100 READY Sessions and spent one GetActor on each healthy runtime. As of 94026a0, each interval lists the Actors in the atespaces that hold a READY Session, in pages of 1,000, and reads only the CRASHED ones individually. Revert re-observes the Actor before it admits anything, and PostgreSQL paging now covers admitted recoveries only. On a dev fleet with about 505 READY Sessions and a 15s interval, a worker loss took 69–73s to recover before and 0.4–11s after. An idle controller went from 507 GetActor calls a minute to 6 ListActors calls (crash-detection.txt, substrate-rpcs-idle.txt):

before  READY=506  recovered 1m13.1s after the worker deletion (4.9 intervals)
after   READY=507  recovered 11s after the worker deletion (0.7 intervals)
idle 60s, interval 10s, READY=507:  before 507 GetActor   after 6 ListActors

[adalundhe]

Review follow-up: a failed automatic recovery must still leave the user a way to revert or fork. The review thread that raised this is gone, so I'm answering here.

Agreed, and my first fix (02bd7b6) only half closed this. It added Revert to the alert in automatic mode, but disabled the button while the Session's operation was RESUME. An admitted recovery keeps RESUME through every failed attempt, so after the first automatic attempt failed, Revert spun until a later attempt succeeded. 5ae5c00 disables it only while the page's own request is in flight. A click joins the admitted recovery. After merging #3398 (11f9abb, 40e3702), Fork lives on each saved snapshot's line in the transcript, which renders in automatic mode too, so there is one place to fork.

On a dev fleet I held the automatic revert failing.

Before, at 3fc96de:

After, pending, forked, and reverted:

04:55:34 ERROR recover crashed session: lifecycle operation 9e0f62b5-… remains pending: … failed at step DiscardWorker
04:56:16 CheckpointService/ForkSession  OK  45ms
04:56:27 SessionService/RevertSession   OK  51ms
after UI step: recovered=true state=RUNTIME_STATE_READY needed=false

[EItanya]

Wouldn't this be covered by RuntimeState crashed?

[adalundhe]

Yes. I removed recovery_needed. A pending recovery shows as FAILED/REVERT with reason RuntimeCrashed, as before. A runtime that can't be recovered is now stored as FAILED/NONE with reason RuntimeLost. The failure field comment in the proto documents both.

[EItanya]

🤖 AI-generated review. Comment left by Claude on behalf of @EItanya.

Thanks for adding recovery on send, I think that's the right place for it. A crashed Actor can't take a send anyway, so reverting before answering KAGENT_SEND_NOT_ACCEPTED means API clients recover with the retry they already do. A few things now that it's in, inline.

[EItanya]

With send recovering, do we still need recovery_needed?

The chat still blocks sending when recoveryNeeded is true and asks the user to click Revert. Send now does the same Revert, so we have two recovery paths for one case. The chat could just send, and the existing KAGENT_SEND_NOT_ACCEPTED retry would land on the restored runtime.

If we drop the alert, recovery_needed, Observe, and RecoveryNeeded can go too. That also takes care of the polling cost: today GetSession calls Substrate GetActor on every 4s idle refresh and every 1s poll during a turn, for a Session that's almost always healthy, and the 5s timeout in RecoveryNeeded is longer than both intervals so a slow Substrate stacks up reads. An admitted-but-unfinished recovery already shows as FAILED/REVERT with RuntimeCrashed, so the UI can show "recovering" from stored state without asking Substrate.

[adalundhe]

Agreed. I removed the alert, recovery_needed, Observe, and RecoveryNeeded, along with the 4s Session re-read. No Session read calls Substrate now.

Chat sends normally. The UI's send path had no retry for KAGENT_SEND_NOT_ACCEPTED (only the CLI did), so I added one: same message ID, before any frame arrives, within a 30s window. Chat shows "recovering" from FAILED/REVERT and "runtime lost" from FAILED/NONE + RuntimeLost, both read from stored state.

One case send alone didn't cover: a crash during a turn leaves that task WORKING, and active work refuses every send, so no send ever reached the crashed Actor. A send refused for active work now runs Revert once and retries its reservation. A crashed runtime gets restored and the send goes through. A busy healthy runtime refuses it as before. That costs one Substrate read per send to a busy Session.

[EItanya]

An unrecoverable runtime isn't recorded anywhere

When the Actor is missing or replaced the send gets this error, but the Session stays READY/NONE. Every later send repeats the GetActor and gets the same answer, and GetSession has nothing to show (RecoveryNeeded returns nil for this case). This is what my comment on the proto was getting at: once we've seen it, can we write it down as state? E.g. FAILED/NONE with a failure reason, written in one transaction under the operation ID guard. Then sends fail fast from PostgreSQL, the UI can show "start a new conversation" from GetSession, and nothing needs a per-read observation.

[adalundhe]

Done. When Revert finds the Actor missing, replaced, or retemplated, it calls RecordLostSessionRuntime. In one transaction, guarded by the operation ID it observed, that sets FAILED/NONE with reason RuntimeLost, fails open tasks, and clears any pending dispatch. A newer generation returns a conflict and leaves the Session alone. This applies to an idle revert and to an admitted recovery whose Actor disappears.

After that, sends and RevertSession fail from PostgreSQL without asking Substrate. Delete still works.

[EItanya]

Send authorizes Revert with a weaker verb

Service.Revert requires VerbUpdate (service.go:306). A send to an existing context without a task ID resolves the Session through storedSession(ctx, auth.VerbCreate, ...) (interactions.go:353), and this calls workflow.Revert directly. So a caller who can send but not update the Session can now revert it. Is that intended? If send-implies-revert is the contract I think it should be explicit in the authorizer, otherwise can we check VerbUpdate before reverting?

[adalundhe]

Not intended. Recovery from a send now goes through Service.Revert, so it needs VerbUpdate, the same as RevertSession. A caller who can send but not update gets permission denied. On the busy-send path they get the original busy error instead.

[EItanya]

Tests don't exercise an actual revert

This stub always returns the Session unchanged, so the gateway tests only count calls, and TestSendRecoveryAnswersTheSend only maps errors. Nothing covers a send to a crashed Actor being reverted and the retry being accepted. Since this is runtime lifecycle behavior, can we add an e2e? session_revert_test.go could send instead of calling RevertSession.

[adalundhe]

Added. TestSessionRevertCrashedRuntime is now TestSessionSendRecoversCrashedRuntime. After the worker is deleted mid-turn, the test sends an ordinary message with retry. It then checks that the send is answered by the restored runtime, the Actor UID is unchanged, and the completed task is preserved. A RevertSession call after that returns READY.

Unit coverage also includes the lost-runtime transition against PostgreSQL and the busy-send probe (healthy and crashed).

[EItanya]

Small one: this maps any messageSession error to ErrSendNotAccepted, including not found and permission denied. Those become endless retries instead of the real error. Can we return the error as is?

[adalundhe]

Fixed. RecoverRejectedSend now returns the error as is, and there's a test for not found and permission denied.

[iplay88keys]

Can we make it so that automatic recovery is behind a feature flag that is off by default?

The flag only needs to cover the send path:

• the recovery in resolveSend
• the conflict retry in PrepareSend
• RecoverRejectedSend in finishSend

When the flag is off, a send to a crashed Session should keep the refusal it gets today.

Please keep the flag out of the migrations and the stored data. Schema and persistence changes should always apply. Then upgrades and rollbacks stay safe for each flag value, and each version can read the stored data. We cannot ship breaking changes now, so this is important.

[adalundhe]

Added config.sessions.automaticRecovery, off by default, in 9e269080. It gates all three send recovery paths. Explicit revert stays available, and migrations and persistence remain unconditional. Local checks pass; CI is running.

[codyhartsook]

🤖 AI-assisted review.

A failed recovery can leave chat stuck with the flag on

On kind, I took atelet down while chat's send ran the recovery, and the Session stayed FAILED/REVERT. It was still stuck three minutes after atelet came back. After a reload, chat says the runtime "is being restored", but the composer is disabled. Unless I missed a path, the user has no way to recover from chat.

Today canSend (AgentChatPage.tsx:345) is state === "ready", so a recovering Session gets a disabled composer. I'd recommend also allowing a pending recovery, with alert copy that says to send a message:

const canSend = state === "ready" || instance.data?.operation === "revert";

A running revert and a failed one look the same in the stored Session (FAILED/REVERT), and a send handles both. It rejoins a failed recovery (interactions.go:500), and while a revert runs it's refused as not accepted, so chat's existing resend lands once it finishes. I tried this on kind, and the next message restored the runtime and was answered.

What I checked

• Kind cluster at 9e26908 with automaticRecovery: true, chat driven in a browser.
• Worker pod deleted mid-turn, then a send from chat with atelet down.
• The same run with a local patch that enables the composer during revert.

[EItanya]

🤖 AI-generated review. Comment left by Claude on behalf of @EItanya.

Thanks for working through all of these, the stored RuntimeLost state and dropping the Substrate read from GetSession make this a lot simpler. A few things now that recovery on send is behind automaticRecovery, inline.

[EItanya]

With the flag off, chat has no way to recover a crashed conversation

The Revert alert is gone, and send only recovers when automaticRecovery is on, which is off by default. So in a default install, a crashed conversation looks READY in chat, every send comes back KAGENT_SEND_NOT_ACCEPTED, the new retry loop runs for 30s, and then the user gets an error with no hint that the runtime crashed or what to do. That's the same place we were before this PR, plus 30s of retries.

Can we keep an explicit Revert in the UI for this case? E.g. when the 30s window runs out on KAGENT_SEND_NOT_ACCEPTED, show "the runtime may have crashed" with a Revert button that calls RevertSession. Revert returns a healthy Session unchanged, so it's safe to offer on a guess, and it's an explicit user action, so it fits the flag being off. With the flag on, the user wouldn't see it unless recovery itself keeps failing.

[EItanya]

With the flag off, a lost runtime is never recorded from a send

This returns ErrSendNotAccepted before looking at anything when the flag is off, so a send to a missing or replaced Actor stays retryable forever and the Session stays READY/NONE. Only an explicit RevertSession records RuntimeLost. Recording the loss doesn't revert or discard anything, so I'm not sure it needs to be behind the flag. @iplay88keys, would you be ok with the rejected-send path recording RuntimeLost regardless of the flag, and only the revert being gated?

[EItanya]

With the gateway's retryAfterMs: 100, this window is up to ~300 sends per message, and each one goes through dispatch to the runtime (and with the flag on, a GetActor). That's fine for the busy-dispatch case that clears quickly, but for a crashed runtime with the flag off it's the whole window. Can we back off between retries, e.g. double from retryAfterMs up to a few seconds?

[EItanya]

lost sits next to actor with a comment saying one replaces the other, so a caller can build an observation with both, or with neither. Could observeCrash return the loss separately, e.g. a typed actorLostError{reason} that Revert checks with errors.As, so the observation only ever carries an Actor?

Small related one: lifecycleIdle is now defined in both database and service/session with the same body; can the session package use the database one?