Skip to content

Commit ea203a5

Browse files
fix(cli): preserve state after interrupted runs
1 parent 366311e commit ea203a5

3 files changed

Lines changed: 88 additions & 30 deletions

File tree

cli/src/hooks/helpers/__tests__/send-message.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,32 @@ describe('setupStreamingContext', () => {
289289
expect(abortController).toBe(ownedAbortController)
290290
})
291291

292+
test('invokes the owner checkpoint callback before abort cleanup', () => {
293+
let messages = createBaseMessages()
294+
const streamRefs = createStreamController()
295+
const timerController = createMockTimerController()
296+
const onAbort = mock(() => {})
297+
298+
const { abortController } = setupStreamingContext({
299+
aiMessageId: 'ai-1',
300+
timerController,
301+
setMessages: (fn: any) => {
302+
messages = fn(messages)
303+
},
304+
streamRefs,
305+
onAbort,
306+
setStreamStatus: () => {},
307+
setCanProcessQueue: () => {},
308+
updateChainInProgress: () => {},
309+
setIsRetrying: () => {},
310+
setStreamingAgents: () => {},
311+
})
312+
313+
abortController.abort()
314+
315+
expect(onAbort).toHaveBeenCalledTimes(1)
316+
})
317+
292318
test('setupStreamingContext resets streamRefs and starts timer', () => {
293319
let messages = createBaseMessages()
294320
const streamRefs = createStreamController()

cli/src/hooks/helpers/send-message.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ export const setupStreamingContext = (params: {
272272
setMessages: (updater: (messages: ChatMessage[]) => ChatMessage[]) => void
273273
streamRefs: StreamController
274274
abortController?: AbortController
275+
onAbort?: () => void
275276
setStreamStatus: (status: StreamStatus) => void
276277
setCanProcessQueue: (can: boolean) => void
277278
isQueuePausedRef?: MutableRefObject<boolean>
@@ -303,13 +304,16 @@ export const setupStreamingContext = (params: {
303304
const abortController = params.abortController ?? new AbortController()
304305

305306
abortController.signal.addEventListener('abort', () => {
307+
try {
308+
params.onAbort?.()
309+
} catch {
310+
// Checkpoint callbacks are best-effort; never skip abort cleanup.
311+
}
312+
306313
// Abort means the user stopped streaming; update UI with an interruption notice.
307-
// Release the chain lock immediately so new messages can be sent directly instead
308-
// of being queued. The minor trade-off is that if the user sends a new message
309-
// before client.run() resolves, it may use stale previousRunStateRef. This is
310-
// acceptable because: (1) the user explicitly cancelled, and (2) client.run()
311-
// will update previousRunStateRef when it eventually resolves, so subsequent
312-
// runs will have the full state.
314+
// The owner checkpoints its latest SDK snapshot synchronously through onAbort,
315+
// while generation guards prevent late results from an older run from replacing
316+
// state selected by a newer run.
313317
streamRefs.setters.setWasAbortedByUser(true)
314318
setIsRetrying(false)
315319
timerController.stop('aborted')

cli/src/hooks/use-send-message.ts

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ export const useSendMessage = ({
158158
const previousRunStateRef = useRef<RunState | null>(
159159
useChatStore.getState().runState,
160160
)
161+
// Incremented for every send so a late result from an interrupted run cannot
162+
// overwrite the state selected by the newer run that replaced it.
163+
const runGenerationRef = useRef(0)
161164
// Memoize stream controller to maintain referential stability across renders
162165
const streamRefsRef = useRef<ReturnType<
163166
typeof createStreamController
@@ -277,6 +280,10 @@ export const useSendMessage = ({
277280
return
278281
}
279282

283+
// Assign a generation only after the request is admitted as a real run.
284+
// A session-ended message that is requeued must not supersede an active run.
285+
const runGeneration = ++runGenerationRef.current
286+
280287
if (agentMode !== 'PLAN') {
281288
setHasReceivedPlanResponse(false)
282289
}
@@ -296,6 +303,8 @@ export const useSendMessage = ({
296303
const abortController = new AbortController()
297304
const runChatDir = resolveCurrentChatDir()
298305
const runChatIsCurrent = () => resolveCurrentChatDir() === runChatDir
306+
const runIsCurrent = () =>
307+
runGenerationRef.current === runGeneration && runChatIsCurrent()
299308
let latestRunStateSnapshot: RunState = previousRunStateRef.current ?? {
300309
traceSessionId: randomUUID(),
301310
output: {
@@ -506,6 +515,11 @@ export const useSendMessage = ({
506515
setMessages,
507516
streamRefs,
508517
abortController,
518+
onAbort: () => {
519+
if (runGenerationRef.current !== runGeneration) return
520+
previousRunStateRef.current = latestRunStateSnapshot
521+
setRunState(latestRunStateSnapshot)
522+
},
509523
setStreamStatus,
510524
setCanProcessQueue,
511525
isQueuePausedRef,
@@ -551,7 +565,7 @@ export const useSendMessage = ({
551565
)
552566

553567
const eventHandlerState = createEventHandlerState({
554-
isActive: () => !abortController.signal.aborted && runChatIsCurrent(),
568+
isActive: () => !abortController.signal.aborted && runIsCurrent(),
555569
streamRefs,
556570
setStreamingAgents,
557571
setStreamStatus,
@@ -596,7 +610,7 @@ export const useSendMessage = ({
596610
// conversation, and checkpointing them into this run's directory
597611
// would overwrite that chat's transcript with foreign (possibly
598612
// empty) state — the chat would then be hidden from /history.
599-
if (abortController.signal.aborted || !runChatIsCurrent()) {
613+
if (abortController.signal.aborted || !runIsCurrent()) {
600614
return
601615
}
602616
// Persist asynchronously and coalescing: the periodic snapshot
@@ -642,7 +656,7 @@ export const useSendMessage = ({
642656
// context, and previousRunStateRef/setRunState would leak this run's
643657
// agent state into the other chat. (A plain Esc interrupt keeps the
644658
// same chat, so the interrupted turn is still saved as before.)
645-
if (runChatIsCurrent()) {
659+
if (runIsCurrent()) {
646660
// Finalize: persist state and mark complete
647661
previousRunStateRef.current = runState
648662
setRunState(runState)
@@ -657,29 +671,31 @@ export const useSendMessage = ({
657671
// traps is several times slower.
658672
saveChatState(runState, useChatStore.getState().messages, runChatDir)
659673
}
660-
handleRunCompletion({
661-
runState,
662-
actualCredits,
663-
agentMode,
664-
timerController,
665-
updater,
666-
aiMessageId,
667-
wasAbortedByUser: abortController.signal.aborted,
668-
hasReceivedContent: hasReceivedContentRef.current,
669-
setStreamStatus,
670-
setCanProcessQueue,
671-
updateChainInProgress,
672-
setHasReceivedPlanResponse,
673-
resumeQueue,
674-
isProcessingQueueRef,
675-
isQueuePausedRef,
676-
})
674+
if (runIsCurrent()) {
675+
handleRunCompletion({
676+
runState,
677+
actualCredits,
678+
agentMode,
679+
timerController,
680+
updater,
681+
aiMessageId,
682+
wasAbortedByUser: abortController.signal.aborted,
683+
hasReceivedContent: hasReceivedContentRef.current,
684+
setStreamStatus,
685+
setCanProcessQueue,
686+
updateChainInProgress,
687+
setHasReceivedPlanResponse,
688+
resumeQueue,
689+
isProcessingQueueRef,
690+
isQueuePausedRef,
691+
})
692+
}
677693
} catch (error) {
678694
// If this run was aborted, the abort handler already handled cleanup.
679695
// Don't run error handling to avoid interfering with any new run that
680696
// may have started. Uses per-run abortController.signal (not shared
681697
// streamRefs) so a newer run's reset() can't clear this flag.
682-
if (!abortController.signal.aborted) {
698+
if (!abortController.signal.aborted && runIsCurrent()) {
683699
handleRunError({
684700
error,
685701
timerController,
@@ -692,20 +708,32 @@ export const useSendMessage = ({
692708
isQueuePausedRef,
693709
hasReceivedContent: hasReceivedContentRef.current,
694710
})
711+
// Keep the latest successful SDK snapshot available to the next
712+
// message in this process, not only on disk. Without this, a failed
713+
// or expired turn is followed by a fresh run with stale history.
714+
if (runIsCurrent()) {
715+
previousRunStateRef.current = latestRunStateSnapshot
716+
setRunState(latestRunStateSnapshot)
717+
}
695718
// Persist the last checkpoint plus the error banner so a restart
696719
// after a failed run still shows this turn. Settle async checkpoints
697720
// first so a stale write can't clobber this one. Skipped after a
698721
// mid-run chat switch — the store's messages belong to the new chat.
699-
if (runChatIsCurrent()) {
722+
if (runIsCurrent()) {
700723
await settleCheckpointSave()
701724
saveChatState(
702725
latestRunStateSnapshot,
703726
useChatStore.getState().messages,
704727
runChatDir,
705728
)
706729
}
707-
} else {
730+
} else if (abortController.signal.aborted) {
708731
logger.debug({ error }, '[send-message] Ignoring error after abort')
732+
} else {
733+
logger.debug(
734+
{ error },
735+
'[send-message] Ignoring error after run superseded',
736+
)
709737
}
710738
} finally {
711739
// Stop exit-flushing this run's checkpoint; the final state (or last
@@ -717,7 +745,7 @@ export const useSendMessage = ({
717745
// interfering with any new run that may have started after the abort.
718746
// Uses per-run abortController.signal (not shared streamRefs) so a newer
719747
// run's reset() can't clear this flag.
720-
if (!abortController.signal.aborted) {
748+
if (!abortController.signal.aborted && runIsCurrent()) {
721749
if (isChainInProgressRef.current) {
722750
logger.warn(
723751
{},

0 commit comments

Comments
 (0)