From 6012aa46e6b3569f7c95b23803821b60e24ed3e5 Mon Sep 17 00:00:00 2001 From: user Date: Sat, 29 Aug 2026 10:09:16 +0800 Subject: [PATCH] fix(flow-chat): settle dialog turn completion unconditionally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit handleDialogTurnComplete gates settlement on eventOwnsLatestSessionTurn, which requires the completed turn to be the latest dialog turn. When an optimistic follow-up turn exists, the completion event for the older turn is dropped and the session stays PROCESSING forever. Ownership matching now prefers the machine's currentDialogTurnId, the failure path settles with a single FINISHING_SETTLED transition instead of ERROR_OCCURRED→RESET (which clears currentDialogTurnId and desynchronizes later ownership checks), and completion events settle unconditionally (BACKEND_STREAM_COMPLETED while PROCESSING plus an unconditional beginTurnCompletion). The ownsSessionSettlement mechanism stays in place at all other consumption points. Test: pnpm vitest run src/flow_chat/services/flow-chat-manager/EventHandlerModule.test.ts (48 passed, 0 failed) + pnpm run type-check (0 errors) AI: This change was assisted by AI and lightly tested. --- .../EventHandlerModule.test.ts | 50 +++++++++++++++++++ .../flow-chat-manager/EventHandlerModule.ts | 33 ++++++++---- 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.test.ts b/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.test.ts index 5971db3fa2..d1818b6364 100644 --- a/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.test.ts +++ b/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.test.ts @@ -369,6 +369,56 @@ describe('interrupted turn lifecycle', () => { SessionExecutionState.FINISHING, ); }); + + it('settles the state machine when completion arrives for a non-latest turn', async () => { + // The machine is PROCESSING for turn-1 while dialogTurns already contains + // a newer optimistic follow-up turn-2. Gating settlement on + // eventOwnsLatestSessionTurn would drop the completion event and leave the + // machine in PROCESSING forever; completion must settle unconditionally. + vi.useFakeTimers(); + const turn1: DialogTurn = { + id: 'turn-1', + sessionId: 'session-1', + agentType: 'agentic', + userMessage: { id: 'user-1', content: 'first', timestamp: 1 }, + modelRounds: [], + status: 'processing', + startTime: 1, + }; + const turn2: DialogTurn = { + id: 'turn-2', + sessionId: 'session-1', + agentType: 'agentic', + userMessage: { id: 'user-2', content: 'follow-up', timestamp: 2 }, + modelRounds: [], + status: 'processing', + startTime: 2, + }; + createSessionWithTurn(turn1); + FlowChatStore.getInstance().setState(state => { + const sessions = new Map(state.sessions); + sessions.set('session-1', { ...sessions.get('session-1')!, dialogTurns: [turn1, turn2] }); + return { ...state, sessions }; + }); + await stateMachineManager.transition('session-1', SessionExecutionEvent.START, { + taskId: 'session-1', + dialogTurnId: 'turn-1', + }); + const context = createFlowChatContext(); + + handleDialogTurnComplete(context, { + sessionId: 'session-1', + turnId: 'turn-1', + success: true, + finishReason: 'complete', + }, vi.fn()); + await vi.advanceTimersByTimeAsync(500); + + expect(stateMachineManager.getCurrentState('session-1')).toBe(SessionExecutionState.IDLE); + expect(FlowChatStore.getInstance().getState().sessions + .get('session-1')!.dialogTurns[0]).toMatchObject({ id: 'turn-1', status: 'completed' }); + vi.useRealTimers(); + }); }); describe('resolveDialogTurnDisplayContent', () => { diff --git a/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts b/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts index 813eb12b4d..534f6b86c8 100644 --- a/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts +++ b/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts @@ -185,9 +185,13 @@ function eventOwnsLatestSessionTurn( sessionId: string, turnId: string, ): boolean { - if (session.dialogTurns.at(-1)?.id !== turnId) return false; const machine = stateMachineManager.get(sessionId); const currentTurnId = machine?.getContext().currentDialogTurnId; + // Prefer the state machine's current turn id: an optimistically created + // follow-up turn can make dialogTurns.at(-1) differ from the turn that is + // actually completing, and its completion event must not be dropped. + if (currentTurnId && currentTurnId === turnId) return true; + if (session.dialogTurns.at(-1)?.id !== turnId) return false; return !currentTurnId || currentTurnId === turnId; } @@ -2645,7 +2649,14 @@ export function handleDialogTurnComplete( reconcileBackgroundSubagentSession(sessionId); const currentState = stateMachineManager.getCurrentState(sessionId); - if (ownsSessionSettlement && currentState === SessionExecutionState.PROCESSING) { + // A DialogTurnCompleted event means a turn of this session finished + // normally, so settle unconditionally while the machine is PROCESSING. + // Gating on eventOwnsLatestSessionTurn drops the event when an optimistic + // follow-up turn exists, leaving the machine in PROCESSING forever. The + // backend emits Completed only once, right after the turn ends and before + // any new turn starts, so a late completion for an old turn cannot break a + // newer one (Cancelled events arrive late and have their own handler). + if (currentState === SessionExecutionState.PROCESSING) { void stateMachineManager .transition(sessionId, SessionExecutionEvent.BACKEND_STREAM_COMPLETED) .catch(error => { @@ -2655,9 +2666,10 @@ export function handleDialogTurnComplete( log.debug('Skipping BACKEND_STREAM_COMPLETED transition', { currentState, sessionId, turnId }); } - if (ownsSessionSettlement) { - beginTurnCompletion(context, sessionId, turnId, partialRecoveryReason); - } + // Settle unconditionally once the completion event arrives; finalize is + // idempotent about the machine's FINISHING_SETTLED and the UI turn update + // already happened above. + beginTurnCompletion(context, sessionId, turnId, partialRecoveryReason); } function normalizeDialogErrorDetail(event: any): AiErrorDetail { @@ -2745,14 +2757,15 @@ function handleDialogTurnFailed(context: FlowChatContext, event: any): void { reconcileBackgroundSubagentSession(sessionId); const currentState = stateMachineManager.getCurrentState(sessionId); + // Settle the failure with a single FINISHING_SETTLED transition. The old + // ERROR_OCCURRED→RESET pair cleared context.currentDialogTurnId, which made + // later ownership checks lose track of the turn and left the machine in + // PROCESSING forever. if (ownsSessionSettlement && isStreamingExecutionState(currentState)) { - stateMachineManager.transition(sessionId, SessionExecutionEvent.ERROR_OCCURRED, { + stateMachineManager.transition(sessionId, SessionExecutionEvent.FINISHING_SETTLED, { error: error || 'Execution failed' }).catch(err => { - log.error('State machine transition failed on error occurred', { sessionId, error: err }); - }); - stateMachineManager.transition(sessionId, SessionExecutionEvent.RESET).catch(err => { - log.error('State machine transition failed on reset', { sessionId, error: err }); + log.error('State machine transition failed on error settled', { sessionId, error: err }); }); }