-
Notifications
You must be signed in to change notification settings - Fork 49
CloudAgentNext - broadcast cloud.status ready after terminalization and broaden wrapper terminal error detection #4515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
103accc
387f552
30a1891
0eb422c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ import { classifyAssistantFailureMessage } from './safe-failure-projection.js'; | |
| import { countPendingSessionMessages, type SessionQueueStorage } from './pending-messages.js'; | ||
| import type { SessionMessageQueue } from './session-message-queue.js'; | ||
| import { | ||
| getSessionMessageState, | ||
| isTerminalMessageState, | ||
| listMessagesForWrapperRun, | ||
| listNonTerminalAcceptedMessages, | ||
| type SessionMessageState, | ||
|
|
@@ -135,7 +137,7 @@ export type WrapperSupervisor = { | |
| ): Promise<void>; | ||
| observeFinalizing(wrapperRunId: string): Promise<void>; | ||
| onDisconnected(input: WrapperDisconnectedInput): Promise<void>; | ||
| onTerminalEvent(params: WrapperTerminalEvent): Promise<void>; | ||
| onTerminalEvent(params: WrapperTerminalEvent): Promise<boolean>; | ||
| requestPhysicalWrapperStop(reason: WrapperStopReason, target?: WrapperStopTarget): Promise<void>; | ||
| clearDisconnectGrace(): Promise<void>; | ||
| runMaintenance(now: number): Promise<void>; | ||
|
|
@@ -178,6 +180,13 @@ export type WrapperSupervisorDependencies = { | |
| recordSharedSandboxFailover: (routeKey: SandboxId) => Promise<void>; | ||
| requestAlarmAtOrBefore?: (deadline: number) => Promise<void>; | ||
| getSessionIdForLogs: () => string | undefined; | ||
| /** | ||
| * Invoked after a disconnect-grace or liveness path terminalizes accepted | ||
| * work, so the DO can broadcast a `cloud.status: ready` event and re-enable | ||
| * client input. Not invoked for the normal terminal-event path, which | ||
| * broadcasts readiness from {@link handleWrapperTerminalEvent} directly. | ||
| */ | ||
| onSessionTerminalized?: () => Promise<void>; | ||
| }; | ||
|
|
||
| function matchesDisconnectGraceFence( | ||
|
|
@@ -362,6 +371,7 @@ export function createWrapperSupervisor( | |
| recordSharedSandboxFailover, | ||
| requestAlarmAtOrBefore, | ||
| getSessionIdForLogs, | ||
| onSessionTerminalized, | ||
| } = dependencies; | ||
|
|
||
| async function readDisconnectGrace(): Promise<DisconnectGraceState | undefined> { | ||
|
|
@@ -633,15 +643,46 @@ export function createWrapperSupervisor( | |
| const acceptedMessages = await listNonTerminalAcceptedMessages(storage, state.wrapperRunId); | ||
| for (const message of acceptedMessages) { | ||
| const activityObserved = message.agentActivityObservedAt !== undefined; | ||
| await messageSettlementOutbox.terminalizeSessionMessageOnce(message.messageId, { | ||
| kind: 'failed', | ||
| reason: 'wrapper_failure', | ||
| error, | ||
| completionSource: 'wrapper_failure', | ||
| failureStage: activityObserved ? 'agent_activity' : 'post_dispatch_no_activity', | ||
| failureCode: activityObserved ? 'wrapper_error_after_activity' : failureCode, | ||
| }); | ||
| try { | ||
| await messageSettlementOutbox.terminalizeSessionMessageOnce(message.messageId, { | ||
| kind: 'failed', | ||
| reason: 'wrapper_failure', | ||
| error, | ||
| completionSource: 'wrapper_failure', | ||
| failureStage: activityObserved ? 'agent_activity' : 'post_dispatch_no_activity', | ||
| failureCode: activityObserved ? 'wrapper_error_after_activity' : failureCode, | ||
| }); | ||
| } catch (terminalizeError) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 Reply with |
||
| // terminalizeSessionMessageOnly only becomes fallible after the | ||
| // durable transition commits (event/callback effects). A throw while | ||
| // the message is still non-terminal means the durable write itself | ||
| // failed; re-throw so the wrapper runtime identity is preserved and | ||
| // maintenance can retry instead of orphaning still-accepted work. | ||
| let durableTransitionSucceeded = false; | ||
| try { | ||
| const persistedState = await getSessionMessageState(storage, message.messageId); | ||
| durableTransitionSucceeded = | ||
| persistedState !== undefined && isTerminalMessageState(persistedState); | ||
| } catch { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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 Reply with |
||
| // Re-read failed — assume the durable transition did not succeed. | ||
| } | ||
| if (!durableTransitionSucceeded) { | ||
| throw terminalizeError; | ||
| } | ||
| logger | ||
| .withFields({ | ||
| sessionId: getSessionIdForLogs(), | ||
| wrapperRunId: state.wrapperRunId, | ||
| messageId: message.messageId, | ||
| error: | ||
| terminalizeError instanceof Error | ||
| ? terminalizeError.message | ||
| : String(terminalizeError), | ||
| }) | ||
| .warn('Failed to apply terminal effects during unhealthy wrapper handling'); | ||
| } | ||
| } | ||
| await onSessionTerminalized?.(); | ||
| await messageSettlementOutbox.releaseWrapperTerminalWaitForIdleBatch(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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 Reply with |
||
| if (isWrapperRunFinalizing(state) && state.wrapperRunId) { | ||
| await messageSettlementOutbox.finalizeTerminalWrapperRunCallbackIfReady(state.wrapperRunId); | ||
|
|
@@ -721,6 +762,7 @@ export function createWrapperSupervisor( | |
| failureCode: activityObserved ? 'wrapper_error_after_activity' : 'wrapper_disconnected', | ||
| }); | ||
| } | ||
| await onSessionTerminalized?.(); | ||
| await clearWrapperRuntimeIdentity( | ||
| storage, | ||
| { | ||
|
|
@@ -1290,7 +1332,7 @@ export function createWrapperSupervisor( | |
| ); | ||
| } | ||
|
|
||
| async function onTerminalEvent(params: WrapperTerminalEvent): Promise<void> { | ||
| async function onTerminalEvent(params: WrapperTerminalEvent): Promise<boolean> { | ||
| const { | ||
| wrapperRunId, | ||
| status, | ||
|
|
@@ -1313,7 +1355,7 @@ export function createWrapperSupervisor( | |
| logger | ||
| .withFields({ sessionId, wrapperRunId, status }) | ||
| .warn('Ignoring non-current wrapper terminal event'); | ||
| return; | ||
| return false; | ||
| } | ||
|
|
||
| logger | ||
|
|
@@ -1461,6 +1503,7 @@ export function createWrapperSupervisor( | |
| ) { | ||
| await sessionMessageQueue.requestPendingDrainIfNeeded(); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| async function runMaintenance(now: number): Promise<void> { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.