fix(cli): recover agent context when run-state.json is torn - #1169
Open
nordicnode wants to merge 1 commit into
Open
fix(cli): recover agent context when run-state.json is torn#1169nordicnode wants to merge 1 commit into
nordicnode wants to merge 1 commit into
Conversation
A resumed chat could start amnesiac — the transcript intact, the model with no memory of earlier turns — through a chain with no single point of failure: - writeFileAtomic renamed the temp over the target without fsync, so a power loss could land the rename while the data blocks were never written: the 'atomic' file contained garbage. fsync the temp before renaming (both sync and async paths). - loadMostRecentChatState tried only the primary and fell back to a RunState placeholder with no sessionState. The SDK starts a fresh session when previousRun.sessionState is absent, so the next turn silently lost every earlier turn. Now the load tries the rotated .bak (the previous complete generation) and then the newest complete checkpoint temp (a SIGKILL between write and rename leaves one behind), self-heals the primary from whichever recovered, and only gives up when all three are unreadable. - The loss used to be invisible to the user: the transcript rendered normally and the model just 'forgot'. loadMostRecentChatState now reports whether agent context survived, and the resume flow prepends an error-variant notice when it did not. Also: retry the async rename briefly on EPERM/EBUSY/EACCES — on Windows the just-closed handle can still be held by AV/indexer scans, which the new fsync widens the window for. Refs CodebuffAI#1166 (persistence-side companion to the compaction wipes)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Companion to the compaction wipes in #1166 — this is the persistence-side path to the same "model lost all chat history" symptom (details and repro in #1168). A resumed chat could start amnesiac through a chain with no single point of failure:
writeFileAtomic/writeFileAtomicAsyncnow fsync the temp before renaming. Without it, a power loss could land the rename while the data blocks were never written — a tornrun-state.jsonexactly where the atomic rename was supposed to guarantee a complete file.loadMostRecentChatStaterecovers instead of giving up. On an unreadable primary it now tries the rotatedrun-state.json.bak(previous complete generation; each synchronous save rotates the current file aside before overwriting), then the newest complete checkpoint.tmp(left behind when a process dies between write and rename), self-heals the primary from whichever recovered, and only falls back to the context-less placeholder when all three are unreadable.loadMostRecentChatStatereturnsrunStateRestored, and the resume flow prepends an error-variant notice ("the assistant starts this chat without memory of earlier turns; the transcript below is intact") when it isfalse. Previously the transcript rendered normally and the model just "forgot" — indistinguishable from a broken assistant.clearChatStatealso removes the.bak.Validation
bun test cli/src/utils/__tests__/run-state-storage.test.ts cli/src/utils/__tests__/write-file-atomic.test.ts cli/src/utils/__tests__/chat-meta.test.ts cli/src/utils/__tests__/safe-json.test.ts(75 pass) — 6 new recovery tests:.bakrecovery + self-heal, newest-temp recovery, healthy primary flagged restored, all-three-unreadable fallback with the loss flagged, backup rotation on save, backup cleanupmain; this change adds 6 passing tests, breaks nothing)bunx tsc --noEmit -p cli/tsconfig.json— no new errors (10 pre-existing, all in unrelated test files: missingtartypes,react-dom/servertypes)Refs #1168