Skip to content

Commit 6072b48

Browse files
committed
Fix blank-session restarts after interrupt or failed run
Fixes #1054. Esc-aborting a session and sending a follow-up could start a brand new, blank conversation because the continuation state (previousRunStateRef) was only synced when run() settled, while the abort listener released the input lock immediately. - SetupStreamingContext gains an onAbort callback invoked synchronously at the top of the abort listener, before the lock is released, so the run owner can checkpoint its latest SDK snapshot. - useSendMessage passes syncRunState(latestRunStateSnapshot) as onAbort, and syncRunState now guards with a generation token so a superseded run settling late can never adopt state, persist a checkpoint, or touch shared queue state over the run that replaced it. - The catch path (failed/expired runs) now also syncs the ref, not just disk, so the next prompt resumes from the last snapshot instead of stale or null history. - loadMostRecentChatState stops adopting/persisting sessionState-less run states, which made the SDK build a blank session on restart. Adds hook-level regression tests through the real createRunConfig / client.run wiring: abort then follow-up carries full history, late superseded runs cannot clobber, the rejected-run path resumes from the last snapshot, and sessionState-less states are never adopted. Helper and storage suites updated.
1 parent 731bb5b commit 6072b48

7 files changed

Lines changed: 728 additions & 55 deletions

File tree

Lines changed: 398 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,398 @@
1+
import { describe, expect, test, beforeEach, afterEach } from 'bun:test'
2+
import * as fs from 'fs'
3+
import * as os from 'os'
4+
import * as path from 'path'
5+
import { createTestRenderer } from '@opentui/core/testing'
6+
import { createRoot, flushSync } from '@opentui/react'
7+
import React, { useEffect, useRef } from 'react'
8+
9+
import { useSendMessage } from '../use-send-message'
10+
import { setClientFactoryOverrideForTesting } from '../../utils/codebuff-client'
11+
import { stopActiveRun } from '../../utils/active-run'
12+
import { setChatDirOverrideForTesting } from '../../utils/run-state-storage'
13+
import { setProjectRoot } from '../../project-files'
14+
import { useChatStore } from '../../state/chat-store'
15+
16+
import type { RunState } from '@codebuff/sdk'
17+
import type { SendMessageFn } from '../../types/contracts/send-message'
18+
import type { ElapsedTimeTracker } from '../use-elapsed-time'
19+
20+
// Ensure required env vars exist so logger/env parsing succeeds in tests.
21+
const ensureEnv = () => {
22+
process.env.NEXT_PUBLIC_CB_ENVIRONMENT =
23+
process.env.NEXT_PUBLIC_CB_ENVIRONMENT || 'test'
24+
process.env.NEXT_PUBLIC_CODEBUFF_APP_URL =
25+
process.env.NEXT_PUBLIC_CODEBUFF_APP_URL || 'https://app.codebuff.test'
26+
process.env.NEXT_PUBLIC_SUPPORT_EMAIL =
27+
process.env.NEXT_PUBLIC_SUPPORT_EMAIL || 'support@codebuff.test'
28+
process.env.NEXT_PUBLIC_POSTHOG_API_KEY =
29+
process.env.NEXT_PUBLIC_POSTHOG_API_KEY || 'phc_test_key'
30+
process.env.NEXT_PUBLIC_POSTHOG_HOST_URL =
31+
process.env.NEXT_PUBLIC_POSTHOG_HOST_URL || 'https://posthog.codebuff.test'
32+
process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY =
33+
process.env.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY || 'pk_test_123'
34+
process.env.NEXT_PUBLIC_STRIPE_CUSTOMER_PORTAL =
35+
process.env.NEXT_PUBLIC_STRIPE_CUSTOMER_PORTAL ||
36+
'https://stripe.codebuff.test'
37+
process.env.NEXT_PUBLIC_WEB_PORT = process.env.NEXT_PUBLIC_WEB_PORT || '3000'
38+
}
39+
ensureEnv()
40+
41+
let api: { sendMessage: SendMessageFn; clearMessages: () => void } | null = null
42+
43+
const makeTimer = (): ElapsedTimeTracker => ({
44+
start: () => {},
45+
stop: () => {},
46+
pause: () => {},
47+
resume: () => {},
48+
elapsedSeconds: 0,
49+
startTime: null,
50+
isPaused: false,
51+
})
52+
53+
const Host = () => {
54+
const inputRef = useRef<any>(null)
55+
const activeSubagentsRef = useRef<Set<string>>(new Set())
56+
const isChainInProgressRef = useRef(false)
57+
const isQueuePausedRef = useRef(false)
58+
const isProcessingQueueRef = useRef(false)
59+
const mainAgentTimer = useRef<ElapsedTimeTracker>(makeTimer()).current
60+
61+
const { sendMessage, clearMessages } = useSendMessage({
62+
inputRef,
63+
activeSubagentsRef,
64+
isChainInProgressRef,
65+
setStreamStatus: () => {},
66+
setCanProcessQueue: () => {},
67+
agentId: undefined,
68+
onBeforeMessageSend: async () => ({ success: true, errors: [] }),
69+
mainAgentTimer,
70+
scrollToLatest: () => {},
71+
onTimerEvent: () => {},
72+
isQueuePausedRef,
73+
isProcessingQueueRef,
74+
resumeQueue: () => {},
75+
requeueMessageAtFront: () => {},
76+
continueChat: false,
77+
subscriptionData: null,
78+
})
79+
80+
useEffect(() => {
81+
api = { sendMessage, clearMessages }
82+
}, [sendMessage, clearMessages])
83+
84+
return <text>host</text>
85+
}
86+
87+
type RunCall = {
88+
runConfig: any
89+
resolve: (state: RunState) => void
90+
reject: (error: unknown) => void
91+
settled: boolean
92+
}
93+
94+
let runCalls: RunCall[] = []
95+
96+
const fakeClient = {
97+
run: (runConfig: any) =>
98+
new Promise<RunState>((resolve, reject) => {
99+
runCalls.push({ runConfig, resolve, reject, settled: false })
100+
}),
101+
}
102+
103+
const TEST_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), 'use-send-message-'))
104+
105+
// Timer-based poll, not setImmediate: the sendMessage preflight does real
106+
// async work (checkpoint drain, filesystem, yieldToEventLoop), and a tight
107+
// setImmediate loop starves the timer phase before the next run registers.
108+
const waitFor = async (label: string, fn: () => boolean, timeoutMs = 5000) => {
109+
const deadline = Date.now() + timeoutMs
110+
while (Date.now() < deadline) {
111+
if (fn()) return
112+
await new Promise((r) => setTimeout(r, 10))
113+
}
114+
throw new Error(`Timed out waiting for ${label}`)
115+
}
116+
117+
/** History shape a real in-flight SDK snapshot carries: a user prompt, an
118+
* assistant response, and a completed tool call. */
119+
const threeMessageHistory = () => [
120+
{
121+
role: 'user',
122+
content: [{ type: 'text', text: 'implement authentication' }],
123+
},
124+
{
125+
role: 'assistant',
126+
content: [{ type: 'text', text: 'reading project files' }],
127+
},
128+
{
129+
role: 'tool',
130+
toolName: 'read_files',
131+
content: [{ type: 'text', value: { files: ['src/auth.ts'] } }],
132+
},
133+
]
134+
135+
const makeSnapshot = (marker: string, history: any[]): RunState =>
136+
({
137+
traceSessionId: `trace-${marker}`,
138+
sessionState: {
139+
mainAgentState: {
140+
messageHistory: history,
141+
},
142+
},
143+
output: {
144+
type: 'error',
145+
message: 'Session ended before this response completed.',
146+
},
147+
}) as unknown as RunState
148+
149+
const makeCompleted = (marker: string, history: any[]): RunState => ({
150+
traceSessionId: `trace-${marker}-done`,
151+
sessionState: {
152+
mainAgentState: {
153+
messageHistory: history,
154+
},
155+
} as any,
156+
output: { type: 'lastMessage', value: [] },
157+
})
158+
159+
const resolveAllPending = () => {
160+
for (const call of runCalls) {
161+
if (!call.settled) {
162+
call.settled = true
163+
call.resolve(
164+
makeCompleted(call.runConfig.previousRun?.traceSessionId ?? 'x', []),
165+
)
166+
}
167+
}
168+
}
169+
170+
beforeEach(() => {
171+
runCalls = []
172+
api = null
173+
setProjectRoot(process.cwd())
174+
setChatDirOverrideForTesting(TEST_ROOT)
175+
setClientFactoryOverrideForTesting(async () => fakeClient as any)
176+
useChatStore.getState().reset()
177+
})
178+
179+
afterEach(() => {
180+
stopActiveRun('process-exit')
181+
setClientFactoryOverrideForTesting(undefined)
182+
setChatDirOverrideForTesting(undefined)
183+
})
184+
185+
describe('useSendMessage session-state preservation (freebuff #1054)', () => {
186+
test('Esc abort: an immediate follow-up run inherits the interrupted snapshot', async () => {
187+
const setup = await createTestRenderer({ width: 80, height: 3 })
188+
const root = createRoot(setup.renderer)
189+
flushSync(() => {
190+
root.render(<Host />)
191+
})
192+
await setup.renderOnce()
193+
194+
let run1: Promise<void> | null = null
195+
let run2: Promise<void> | null = null
196+
try {
197+
flushSync(() => {
198+
run1 = api!.sendMessage({ content: 'first', agentMode: 'DEFAULT' })
199+
})
200+
await waitFor('first run to start', () => runCalls.length === 1)
201+
202+
// The SDK emits an in-flight snapshot that includes real history.
203+
const richSnapshot = makeSnapshot('A', threeMessageHistory())
204+
runCalls[0].runConfig.onStateSnapshot(richSnapshot)
205+
206+
// User hits Esc. The abort listener must synchronously adopt the
207+
// latest snapshot into continuation state before releasing input.
208+
flushSync(() => {
209+
stopActiveRun('user-interrupt')
210+
})
211+
212+
flushSync(() => {
213+
run2 = api!.sendMessage({
214+
content: 'also verify tests',
215+
agentMode: 'DEFAULT',
216+
})
217+
})
218+
await waitFor('second run to start', () => runCalls.length === 2)
219+
220+
// Regression: the follow-up must carry the interrupted run's full
221+
// history instead of an empty previousRun that makes the SDK start a
222+
// blank session.
223+
const previousRun = runCalls[1].runConfig.previousRun as RunState
224+
expect(previousRun).toBeDefined()
225+
expect(
226+
(previousRun.sessionState as any).mainAgentState.messageHistory,
227+
).toHaveLength(3)
228+
expect(
229+
(previousRun.sessionState as any).mainAgentState.messageHistory[0]
230+
.content[0].text,
231+
).toBe('implement authentication')
232+
expect(
233+
(previousRun.sessionState as any).mainAgentState.messageHistory[2]
234+
.toolName,
235+
).toBe('read_files')
236+
} finally {
237+
resolveAllPending()
238+
await Promise.all([run1, run2])
239+
flushSync(() => root.unmount())
240+
setup.renderer.destroy()
241+
}
242+
})
243+
244+
test('rejected run: the next prompt resumes from the last snapshot', async () => {
245+
const setup = await createTestRenderer({ width: 80, height: 3 })
246+
const root = createRoot(setup.renderer)
247+
flushSync(() => {
248+
root.render(<Host />)
249+
})
250+
await setup.renderOnce()
251+
252+
let run1: Promise<void> | null = null
253+
let run2: Promise<void> | null = null
254+
try {
255+
flushSync(() => {
256+
run1 = api!.sendMessage({ content: 'first', agentMode: 'DEFAULT' })
257+
})
258+
await waitFor('first run to start', () => runCalls.length === 1)
259+
260+
const richSnapshot = makeSnapshot('A', threeMessageHistory())
261+
runCalls[0].runConfig.onStateSnapshot(richSnapshot)
262+
263+
// The SDK run throws (network/auth/session gate). The catch block
264+
// must keep the latest snapshot available in memory for the next
265+
// message, not only on disk.
266+
runCalls[0].reject(new Error('boom'))
267+
await run1
268+
269+
flushSync(() => {
270+
run2 = api!.sendMessage({ content: 'continue', agentMode: 'DEFAULT' })
271+
})
272+
await waitFor('second run to start', () => runCalls.length === 2)
273+
274+
const previousRun = runCalls[1].runConfig.previousRun as RunState
275+
expect(previousRun).toBeDefined()
276+
expect(previousRun.traceSessionId).toBe('trace-A')
277+
expect(
278+
(previousRun.sessionState as any).mainAgentState.messageHistory,
279+
).toHaveLength(3)
280+
} finally {
281+
resolveAllPending()
282+
await Promise.all([run1, run2])
283+
flushSync(() => root.unmount())
284+
setup.renderer.destroy()
285+
}
286+
})
287+
288+
test('superseded run settling late cannot clobber the newer run state', async () => {
289+
const setup = await createTestRenderer({ width: 80, height: 3 })
290+
const root = createRoot(setup.renderer)
291+
flushSync(() => {
292+
root.render(<Host />)
293+
})
294+
await setup.renderOnce()
295+
296+
let run1: Promise<void> | null = null
297+
let run2: Promise<void> | null = null
298+
let run3: Promise<void> | null = null
299+
try {
300+
flushSync(() => {
301+
run1 = api!.sendMessage({ content: 'first', agentMode: 'DEFAULT' })
302+
})
303+
await waitFor('first run to start', () => runCalls.length === 1)
304+
const snapshotA = makeSnapshot('A', threeMessageHistory())
305+
runCalls[0].runConfig.onStateSnapshot(snapshotA)
306+
307+
// Abort run A, then start run B before A's SDK promise settles.
308+
flushSync(() => {
309+
stopActiveRun('user-interrupt')
310+
run2 = api!.sendMessage({ content: 'second', agentMode: 'DEFAULT' })
311+
})
312+
await waitFor('second run to start', () => runCalls.length === 2)
313+
314+
// B settles first with its own final state.
315+
runCalls[1].resolve(
316+
makeCompleted('B', [
317+
{ role: 'user', content: [{ type: 'text', text: 'B final' }] },
318+
]),
319+
)
320+
await run2
321+
322+
// Now A settles late. Its state must NOT replace B's: B is the
323+
// current run.
324+
runCalls[0].resolve(
325+
makeCompleted('A-late', [
326+
{ role: 'user', content: [{ type: 'text', text: 'A late result' }] },
327+
]),
328+
)
329+
await run1
330+
331+
flushSync(() => {
332+
run3 = api!.sendMessage({ content: 'third', agentMode: 'DEFAULT' })
333+
})
334+
await waitFor('third run to start', () => runCalls.length === 3)
335+
336+
const previousRun = runCalls[2].runConfig.previousRun as RunState
337+
expect(previousRun.traceSessionId).toBe('trace-B-done')
338+
expect(
339+
(previousRun.sessionState as any).mainAgentState.messageHistory[0]
340+
.content[0].text,
341+
).toBe('B final')
342+
} finally {
343+
resolveAllPending()
344+
await Promise.all([run1, run2, run3])
345+
flushSync(() => root.unmount())
346+
setup.renderer.destroy()
347+
}
348+
})
349+
350+
test('sessionState-less snapshot is never adopted as continuation state', async () => {
351+
const setup = await createTestRenderer({ width: 80, height: 3 })
352+
const root = createRoot(setup.renderer)
353+
flushSync(() => {
354+
root.render(<Host />)
355+
})
356+
await setup.renderOnce()
357+
358+
let run1: Promise<void> | null = null
359+
let run2: Promise<void> | null = null
360+
try {
361+
flushSync(() => {
362+
run1 = api!.sendMessage({ content: 'first', agentMode: 'DEFAULT' })
363+
})
364+
await waitFor('first run to start', () => runCalls.length === 1)
365+
366+
// Abort before the SDK ever emitted a sessionState-bearing snapshot.
367+
// The placeholder snapshot has no session state and must not be
368+
// adopted (it would blank the next run).
369+
flushSync(() => {
370+
stopActiveRun('user-interrupt')
371+
})
372+
373+
// The abort resolver returns a state without any sessionState (no
374+
// snapshot was ever emitted). It must not be adopted as continuation
375+
// state: handing it to the SDK would silently blank the next session.
376+
runCalls[0].resolve({
377+
traceSessionId: 'trace-A-no-state',
378+
output: {
379+
type: 'error',
380+
message: 'Session ended before this response completed.',
381+
},
382+
} as RunState)
383+
await run1
384+
385+
flushSync(() => {
386+
run2 = api!.sendMessage({ content: 'second', agentMode: 'DEFAULT' })
387+
})
388+
await waitFor('second run to start', () => runCalls.length === 2)
389+
390+
expect(runCalls[1].runConfig.previousRun).toBeUndefined()
391+
} finally {
392+
resolveAllPending()
393+
await Promise.all([run1, run2])
394+
flushSync(() => root.unmount())
395+
setup.renderer.destroy()
396+
}
397+
})
398+
})

0 commit comments

Comments
 (0)