[adalundhe]

Builds on #3395, now merged; rebased onto main.

Recover crashed runtimes during A2A sends when config.sessions.automaticRecovery: true. The setting defaults to false. The chart only mentions it in a comment, so a default install renders the same controller.yaml as before and a rollback to v1.0.0 can still read it.

The flag controls joining an admitted recovery, recovering a send refused for an open turn, and reverting after a proven unaccepted send. A send waits at most 10 seconds for recovery; a recovery still running is answered KAGENT_SEND_NOT_ACCEPTED with state: RECOVERING, and the client's retry continues it. If recovery can't run, the send keeps the answer it gives without recovery. With the flag 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. The wait doubles from retryAfterMs up to 2 seconds, within a 30-second window. Chat offers Recover, which calls RevertSession, in three cases: when the window runs out; when a send is refused for an open turn (KAGENT_TURN_ACTIVE), which is what a mid-turn crash produces with the flag off while the Session still reads READY; and while a recovery is pending. A recovery whose attempt failed stays FAILED/REVERT until a client retries it. Recover returns a healthy runtime unchanged and records a lost one. 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. A helm-unittest asserts the default render omits the flag.
• PostgreSQL-backed Session, database, gRPC, and scheduled-run controller tests pass.
• UI tests cover the backoff, both refusal tags, and every Recover path, including a revert that records a lost runtime.
• Vitest workers are capped at eight (min(cores - 1, 8)); the default oversubscribed large machines and made heavy page tests time out. Three consecutive full runs pass, 846/846.
• On kind, flag off: deleting the worker mid-turn reproduces Enable persistent sessions #394. The refused send offers Recover, Recover restores the runtime, and the next message is answered. Recover during a healthy running turn leaves it running. Screenshots and logs: evidence gist.
• Protobuf, Go and UI lint pass. Live crash-recovery E2E explicitly enables the flag in CI.

🤖 Generated with Claude Code

[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

[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.

[adalundhe]

Done. When the retry window runs out on KAGENT_SEND_NOT_ACCEPTED, the turn error now says the runtime may have crashed and offers Recover (calls RevertSession) next to Retry. A refusal that arrives after the first frame is not tagged, because that message was accepted. Recover also records RuntimeLost, and the page then shows the lost-runtime alert. (ed0fa109)

This missed the mid-turn crash case; see my correction below.

[adalundhe]

Correction to my reply above: it didn't cover the #394 case. I reproduced it on kind with the flag off. After a worker is deleted mid-turn, the next send isn't KAGENT_SEND_NOT_ACCEPTED. It's an outright FailedPrecondition: resource conflict, because the crashed turn stays open, and the Session still reads READY. Chat showed only Retry, which can't succeed.

456201b6 also offers Recover for A2A refusals that arrive before the first frame. On the same cluster, the refused send now offers Recover. Recover restored the runtime, and the next message was answered. Recover during a healthy running turn left it running, and the turn completed. The branch is also rebased onto main.

Screenshots and logs are in this gist: section 1 is before the fix, section 3 is after it, and section 4 is the healthy running turn.

[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?

[iplay88keys]

For #3395, we had discussed that rollback wouldn't be possible if RevertSession RPC had been called, since the prior version doesn't know the REVERT operation. Keeping automatic recovery generally behind a flag keeps that true. So we have two options that still are rollback safe:

1. Keep it behind the flag. With the flag off, sends behave as they did before this PR. A Revert action in chat then records RuntimeLost when sends keep failing.
2. Record RuntimeLost regardless of the flag. A send to a lost runtime then fails fast with a clear error. This stays rollback safe because FAILED and NOINE already exist in every 1.0 release candidate. However, it must only record the loss and never admit a recovery. This also needs the same update access as RevertSession. Otherwise a caller who can only send could end a Session.

[adalundhe]

I went with option 1. Sends keep their pre-PR behaviour with the flag off. Chat's Recover action calls RevertSession, which records RuntimeLost through the existing update-access check, so the send path never records a loss or admits a recovery. (ed0fa109)

[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?

[adalundhe]

Done. Waits now start at retryAfterMs and double up to 2s. A runtime that stays down costs 19 sends per 30s window, down from about 300. A larger retryAfterMs from the server still wins. (ed0fa109)

[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?

[adalundhe]

Done. observeCrash now returns *actorLostError{operationID, reason}, and Revert checks it with errors.AsType, so a crashObservation always carries an Actor. lifecycleIdle is now database.LifecycleIdle, and the session package uses it. (ed0fa109)

[iplay88keys]

🤖 Written with AI, raised by me.

@EItanya, this raises a question about feature flags as config keys in general.

The chart copies config into controller.yaml, so every key in values.yaml renders for every install. v1.0.0 rejects unknown config fields. So kubectl rollout undo or an image-only rollback to v1.0.0 would crash-loop here, even if the flag was never enabled. Each new flag with a default in values.yaml repeats this.

Two options:

1. Keep default-off flags out of values.yaml, or add them only as a comment. The proto default keeps them off, and a default install renders the same config as before.
2. Let the config decoder accept unknown fields and log them. This only helps rollbacks to releases that include the change. It also stops the decoder from catching typos.

What do you think?

[adalundhe]

For this PR I took option 1. automaticRecovery is only a comment in values.yaml, so a default install renders the same controller.yaml as before. A helm-unittest asserts the default render omits the key, and a second test asserts it renders when set. The CI fleet still enables it explicitly. The general decoder question is still open for you two. (ed0fa109)

On a fresh install from this branch, the rendered controller.yaml has no automaticRecovery key (gist, Setup).

[adalundhe]

@codyhartsook Thanks for reproducing it. I added a Recover button to the recovering alert in place of enabling the composer. Recover calls RevertSession, which retries a failed recovery or joins a running one. Unlike a send, it works with automaticRecovery off: with the flag off, a send to a FAILED/REVERT Session is refused as a lifecycle conflict. The composer enables once the Session is READY. (ed0fa109)

Evidence from kind with the flag off is in this gist, section 2. With your one-line change applied temporarily, the send to the FAILED/REVERT Session was refused with Session cannot accept work during a lifecycle operation, and the recovering alert with Recover disappeared. With atelet back, Recover from the alert finished the recovery, and the next message was answered. I didn't re-test the change with the flag on; your run covers that.

[iplay88keys]

🤖 AI-generated review

The values.yaml change and the helm-unittest guard look good. A default install no longer renders the key, so a rollback can still read the config. With the flag off, sends keep their old refusals, and TestSendRecoveryDisabled covers all three send paths. The comments below are small.

[iplay88keys]

Optional: this tags every A2A refusal that arrives before the first frame, so chat says the runtime may have crashed for any of them. The reason: "SOMETHING_ELSE" test case expects SEND_REFUSED too.

The case this targets is the open-turn conflict from interactionStoreError. Could the gateway name that refusal in ErrorInfo metadata, as it does for KAGENT_SEND_NOT_ACCEPTED? Then chat could tag only that reason, and other refusals would keep the plain error with Retry. Recover is harmless either way, so this only affects what the user reads.

[adalundhe]

Done in 208343c2, and narrowed to open turns in 9d9f5f98. The gateway now names the open-turn refusal KAGENT_TURN_ACTIVE in its ErrorInfo metadata; the code and message are the same as before. Chat offers Recover only for that reason and KAGENT_SEND_NOT_ACCEPTED. Other refusals keep the plain error with Retry.

[iplay88keys]

Small one: the two new props sit between onManageTaskPush and its comment about the mock push store. That comment now reads as if it describes onRecover. Could the new props move above it?

Related: the onRecover doc in ChatTranscript.tsx says Recover is offered when sends are refused as not accepted. It is now offered for outright refusals too.

[adalundhe]

Fixed in 208343c2. The new props now sit above that comment, and the onRecover doc lists both refusals that offer Recover.

[iplay88keys]

Optional: this is the only component that imports from the gRPC chat client. isNotFound and the other error predicates live in ApiError.ts. Could SEND_NOT_ACCEPTED, SEND_REFUSED, isSendNotAccepted, and mayNeedRecovery move there? Then the transcript depends only on ApiError.

[adalundhe]

Moved in 208343c2. The reasons and predicates live in ApiError.ts and are exported from @/api, so the transcript no longer imports the chat client.

[EItanya]

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

Thanks for the Recover action and the backoff, that covers the flag-off case I was worried about. A few more on the send path after another pass, inline.

[EItanya]

ErrConflict here also covers a checkpoint in CREATING and a Session that isn't READY, so healthy Sessions call Revert (and GetActor) too. And if that Revert fails with Unavailable or Aborted, the caller gets ErrSendNotAccepted in place of the busy refusal, so a Substrate outage turns a definitive "busy" into a retry loop.

Can we fall back to the original refusal when recovery isn't available, like the unauthorized case already does?

[adalundhe]

Fixed in 9d9f5f98. Only an open turn starts recovery now. The store reports it as ErrActiveTurn, which is still an ErrConflict, so a checkpoint in progress or a Session that isn't READY keeps its refusal without a revert. If the revert can't run (outage, contention, or no update access), the send gets the same busy refusal as before, not ErrSendNotAccepted. A gateway test covers the outage case.

[EItanya]

Small one: PrepareSend falls back to the original refusal on ErrUnauthorized, but this path and the admitted path in resolveSend return permission denied, even for a transient not-accepted send on a healthy runtime, because Service.Revert authorizes before it checks health. The built-in authorizer grants create and update together, so it needs a custom authorizer to show up, but can the three be consistent?

[adalundhe]

Done in 9d9f5f98. All three paths now share one helper and handle a failed revert the same way. If no recovery is admitted, the send gets the answer it would give without recovery. If one is admitted, it gets the new recovering answer. Missing update access is one of those cases, so none of the paths returns permission denied for it any more.

[EItanya]

finishSend bounds its context to 5s, but WithoutCancel drops that deadline, and workflow.Revert then runs under the 2 minute RuntimeOperationTimeout. So a send can wait ~2 minutes before it answers, on all three paths. Is that intended?

I think the send should either answer "retry" once recovery is admitted and let the revert finish on its own, or have an explicit bound.

[adalundhe]

Not intended. Fixed in 9d9f5f98: a send now waits at most 10 seconds for the recovery it starts. If the revert isn't done by then, the attempt releases its claim and the send answers that the runtime is recovering. The client's next retry continues the admitted recovery, so nothing keeps running after the request ends.

[EItanya]

During recovery the client gets KAGENT_SEND_NOT_ACCEPTED with retryAfterMs: 100, the same as an ordinary busy dispatch, so it can't tell "we're reverting, come back shortly" from "busy".

Can we return a separate reason for it, e.g. KAGENT_RUNTIME_RECOVERING, with a retryAfterMs in seconds? The UI could show "recovering" from that directly, and A2A clients get the signal without calling GetSession.

[adalundhe]

Added in 9d9f5f98, but as metadata rather than a new reason. reason stays KAGENT_SEND_NOT_ACCEPTED, and the error adds state: RECOVERING and retryAfterMs: 1000. The CLI, the e2e helpers and enterprise's SendMessageWithRetry only retry on that exact reason, so a new reason would make recovery a hard failure for clients that are already released. Chat already shows "recovering" from the Session while it retries, and now also says so if its retry window runs out.

[EItanya]

Question: one GetActor NotFound marks the Session FAILED for good and fails every open task, including ones waiting on input. Can Substrate ever return NotFound for an Actor that still exists, e.g. while it's restarting? I didn't find a case, but since there's no way back from RuntimeLost I want to make sure a single observation is enough.

[adalundhe]

One observation is enough. In Substrate v1.0.0, GetActor is a primary-key read of ate-api's own PostgreSQL actors table (cmd/ateapi/internal/store/atepg/actor.go), with no cache in between. Crashes, restarts, suspends and reverts all update that row. Only DeleteActor removes it, and it refuses while a worker still holds the Actor. The atespace foreign key is ON DELETE RESTRICT, so no cascade removes it either. A database error comes back as Internal, not NotFound.

[EItanya]

This crashes mid-turn and sends, so it only exercises the conflict path in PrepareSend. RecoverRejectedSend through finishSend has no e2e.

The rename also dropped the e2e for an explicit RevertSession on a crashed runtime, which is what the new Recover button calls; it's now only called on a healthy runtime. Can we keep that case as its own test?

[adalundhe]

Restored in 9d9f5f98. TestSessionRevertCrashedRuntime is back as its own test. It calls RevertSession on the crashed runtime, including a retry after a lost response, and sits beside the send-recovery test; they share a crash helper. Both kagent subtests pass on kind with the flag on.

I couldn't add a reliable e2e for RecoverRejectedSend. It needs a crash after the dispatch is reserved but before the runtime saves the input, and a test can't time that. Gateway unit tests cover it.