Fix blank-session restarts after interrupt or failed run - #1135
Fix blank-session restarts after interrupt or failed run#1135bunnysayzz wants to merge 1 commit into
Conversation
Fixes CodebuffAI#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.
|
Nice work here. The root-cause analysis is correct and the fix addresses three real holes: the abort listener releasing the input lock before The test suite in One thing worth double-checking before porting: Overall this is exactly the kind of bug fix I'd want ported: precise diagnosis, minimal collateral change to non-test code, and tests that would have caught the regression. Recommend a maintainer verify the nullability change doesn't break other call sites, but the core fix looks solid. |
Fixes #1054.
Root cause
The CLI only synced
previousRunStateRef(the continuation state passed to the SDK aspreviousRun) whenclient.run()settled. But Esc releases the input lock immediately, inside the abort listener. So a follow-up message sent the moment the user hits Esc could be built from a stale (or null) ref. With a null ref the SDK builds a fresh session state and the chat comes back as an empty, brand-new conversation, which reads exactly like a hard reset.Two more holes in the same family:
loadMostRecentChatStatefabricated{ output }when run-state.json had no session state), poisoning every resumed chat.Fix
setupStreamingContextaccepts anonAbortcallback, invoked synchronously at the top of the abort listener, before the input lock is released.useSendMessagepassessyncRunState(latestRunStateSnapshot)so an interrupt checkpoints the latest SDK snapshot immediately.useSendMessagenow owns a generation token per admitted run. A superseded run settling late can never adopt state, persist a checkpoint, or touch shared queue state over the run that replaced it (also gatedhandleRunCompletion/handleRunError/finallywith it).loadMostRecentChatStatetreats a run-state withoutsessionStateas unrestorable; the transcript still restores, the agent context does not.Tests
cli/src/hooks/__tests__/use-send-message.test.tsxdrives the real wiring (createRunConfig+ a controllable client via a small DI seam, matching the repo documented DI-over-module-mocking approach): abort-then-follow-up carries full history, a superseded run settling late cannot clobber the newer run state, rejected runs resume from the last snapshot, and sessionState-less states are never adopted.onAbortcallback contract (sync ordering before lock release, throwing callback still cleans up).All targeted suites pass (92 tests). Typecheck, prettier, and
git diff --checkare clean; the app bundle builds.