fix(core): keep recreated session files loadable after deletion#27905
fix(core): keep recreated session files loadable after deletion#27905i-anubhav-anand wants to merge 2 commits into
Conversation
appendRecord() only checked that this.conversationFile was non-null, not that the file still existed. If session cleanup (or a manual delete) removed the file while the process was still running, the next appendFileSync() recreated it containing only the appended record, with no leading metadata line. The resulting file held messages but no sessionId, so loadConversationRecord() could never load it — a zombie session that list/resume silently ignored. Before appending, if the file is gone but we still hold the in-memory conversation, re-write its metadata line first so the recreated file stays a valid, loadable session. Fixes google-gemini#27279
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where session files could become corrupted or unreadable if they were deleted while the application was still running. By verifying the existence of the session file before appending new records, the system can now gracefully restore the necessary metadata, preventing the creation of 'zombie' sessions that cannot be reloaded. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
📊 PR Size: size/M
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds file recovery logic to ChatRecordingService to handle cases where the session file is deleted mid-session, recreating the file with metadata first. However, the current implementation only writes the metadata and the new record, which silently discards all previous messages in the conversation history. To prevent data loss, the reviewer suggests also writing back all previous messages and the memory scratchpad when recreating the file.
gemini-cli
Bot
added
priority/p2
…file Addressing review feedback: when the session file is recreated after being deleted mid-session, write the full in-memory conversation (metadata line plus every cached message), not just the metadata. Otherwise the prior messages — still held in memory — were silently dropped from the persisted file. The test now asserts both the earlier and the new message survive.
1 participant
Summary
Fixes #27279.
appendRecord()guards only onthis.conversationFilebeing non-null — it never checks whether the file still exists on disk. If session cleanup (or a manual delete) removes the session file while the process is still running, the nextfs.appendFileSync()recreates it containing only the record being appended, with no leading metadata line. The recreated file then has message records but nosessionId, soloadConversationRecord()can never load it: a zombie session thatlist/resumesilently ignore.Fix
Before appending, if the file is gone but the recorder still holds the in-memory
cachedConversation, re-write the conversation's metadata line first, then append the record. The recreated file stays a valid, loadable session.fs.existsSyncper append and only does extra work in the (rare) deletion case.cachedConversationis not yet set) or normal appends (the file exists), so the happy path is unchanged.Testing
chatRecordingService.test.ts: initialize a session, record a message, delete the file mid-session, record another message, then load it. Verified it fails before this change (loader returnsnull) and passes after.npx vitest run src/services/chatRecordingService.test.ts→ 51 passed.eslint(touched files) andnpm run typecheck(packages/core) clean.