CloudAgentNext - broadcast cloud.status ready after terminalization and broaden wrapper terminal error detection#4515
Conversation
…nd broaden wrapper terminal error detection
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous Review Summaries (3 snapshots, latest commit 30a1891)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 30a1891)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 387f552)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous review (commit 103accc)Status: 6 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (11 files)
Reviewed by gpt-5.6-sol · Input: 126.7K · Output: 15.2K · Cached: 1M Review guidance: REVIEW.md from base branch |
…ast and provider error classification - onTerminalEvent returns boolean; only broadcast cloud.status: ready for accepted terminal events so stale events don't re-enable input mid-run - Tie onSessionTerminalized to the durable terminalization transition (before fallible downstream effects) and invoke unconditionally so finalizing runs with no accepted messages still publish readiness - Extend drain delay to 2s when aborted while disconnected so the buffered fatal error frame survives reconnect delivery before close() clears it - Remap provider_error to assistant_error in persistedFailureCode (matching the existing model_missing/payment_required pattern) so run-state reports validate against the queue-report schema without a DB migration
…mmediately on reconnect during aborted drain
| failureStage: activityObserved ? 'agent_activity' : 'post_dispatch_no_activity', | ||
| failureCode: activityObserved ? 'wrapper_error_after_activity' : failureCode, | ||
| }); | ||
| } catch (terminalizeError) { |
There was a problem hiding this comment.
WARNING: Terminalization persistence failures now orphan accepted work
This catch also handles failures from the durable state write itself, as the new test demonstrates. In that case the message remains accepted, but execution continues through readiness publication and clears the wrapper runtime identity, so later maintenance has neither a connection nor a liveness deadline with which to retry terminalization. Only effect failures after a successful durable transition can be safely swallowed here; a persistence failure must preserve or schedule supervision for the still-accepted message.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| allowWithoutObservedIdle: true, | ||
| }); | ||
| } | ||
| } catch (settlementError) { |
There was a problem hiding this comment.
WARNING: Swallowing settlement failures can permanently strand gate callbacks
If releaseWrapperTerminalWaitForIdleBatch() fails before persisting wrapperTerminalWaitReleasedAt, this catch continues and the runtime identity is cleared. No later maintenance path retries that release: terminal-effect repair only repairs message effects, while callback retry only sees already-finalized batches. A completed gate-waiting callback can therefore remain blocked forever. Keep this settlement retryable instead of consuming the failure.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…ure and defer close during in-progress reconnect
| const persistedState = await getSessionMessageState(storage, message.messageId); | ||
| durableTransitionSucceeded = | ||
| persistedState !== undefined && isTerminalMessageState(persistedState); | ||
| } catch { |
There was a problem hiding this comment.
[WARNING]: A failed terminal-state reread can permanently suppress readiness
If the terminal transition commits but a later terminal effect throws, this verification read can itself fail transiently. The catch then assumes the transition did not succeed and rethrows; on the next maintenance pass the message is already terminal, so handleUnhealthyWrapper() is not entered again and onSessionTerminalized never publishes cloud.status: ready. Terminal-effect repair does not perform that broadcast. Preserve a durable/retryable readiness obligation instead of treating a failed reread as proof that the transition failed.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
| } | ||
| await onSessionTerminalized?.(); | ||
| await messageSettlementOutbox.releaseWrapperTerminalWaitForIdleBatch(); |
There was a problem hiding this comment.
[WARNING]: Gate-release failures still have no retry path
Propagating this failure avoids clearing the runtime immediately, but it does not make the release retryable. The accepted messages are already terminal, so on the next alarm a non-finalizing run fails hasActiveWrapperWork(), clears its liveness fields, and never re-enters handleUnhealthyWrapper() to retry releaseWrapperTerminalWaitForIdleBatch(). A completed gate-waiting callback can therefore remain blocked permanently. Persist or reconcile this release independently of active-message liveness.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
1 participant
Problem
When a Cloud Agent execution ends abnormally, the web UI's chat input stays disabled (
canSend = false) because the DO never broadcasts acloud.status: readyevent to re-enable it. Users are forced to create a new session for every prompt instead of sending follow-ups.Two independent gaps cause this:
DO gap: When the wrapper disconnects without sending a
complete/error/interruptedevent, the DO terminalizes messages as failed but never broadcasts anycloud.statusevent. Even when the wrapper sends a fatalerrorevent, the ingest handler broadcastscloud.status: errorbut never follows withcloud.status: ready.Wrapper gap:
getTerminalFailure()only classifies payment/credit/quota/model-not-found errors as terminal. AI SDK errors likeAI_InvalidResponseDataError(e.g., when a BYOK model returns a malformed response) are NOT classified as terminal, so the wrapper never sends a fatalerrorevent — it disconnects without any terminal event, triggering gap 1.Fix 1: DO broadcasts
cloud.status: readyafter terminalizationhandleWrapperTerminalEventbroadcastscloud.status: readyafteronTerminalEventforfailed/interruptedstatus (skipscompletedsince the ingest handler already broadcasts it).onSessionTerminalizedcallback toWrapperSupervisorDependencies, called aftercheckDisconnectGraceandhandleUnhealthyWrapperterminalize accepted work, so the DO broadcastscloud.status: readyon the disconnect-grace and liveness-expiry paths that don't go through the normal terminal-event flow.Fix 2: Wrapper detects AI SDK response errors as terminal
provider_errortoWrapperTerminalFailureCodes.isAIProviderResponseErrorhelper detectingAI_InvalidResponseDataError,AI_JSONParseError, and theExpected '...' to be a ...message pattern.getTerminalFailureso these permanent failures send a fatalerrorevent to the DO, which then broadcasts the error and transitions to ready.Supporting changes
Added
provider_errorto the shared failure-code contract (CLOUD_AGENT_FAILURE_CODES, DB schema type, safe-failure-projection message map, and non-retryable set) for consistency across run-state reports, client error projection, and telemetry.Verification
pnpm run typecheck— passespnpm run lint— passespnpm run test— 2184 unit tests passpnpm run test:integration— 210 integration tests pass