fix(sessionManager): implement write queue and LRU eviction#689
Open
menny wants to merge 3 commits intositeboon:mainfrom
Open
fix(sessionManager): implement write queue and LRU eviction#689menny wants to merge 3 commits intositeboon:mainfrom
menny wants to merge 3 commits intositeboon:mainfrom
Conversation
…g requests SessionManager loads all persisted Gemini sessions from ~/.gemini/sessions/*.json asynchronously in its constructor via `this.ready = this.init()`. Previously, `sessionManager.ready` was never awaited anywhere, so the server would start accepting WebSocket connections before loadSessions() had completed. This created a race condition on the very first request after a server restart: if a client tried to resume an existing Gemini session, `sessionManager.getSession(sessionId)` would return undefined (the session hadn't been loaded from disk yet), causing the `--resume <cliSessionId>` flag to be silently omitted when spawning the Gemini CLI process. The CLI would then start a blank new session instead of continuing the prior conversation, with no error or indication to the user. The fix adds `await sessionManager.ready` in startServer() immediately after `await initializeDatabase()`, following the same pattern already established for database initialization. This guarantees the session store is fully populated before any request handler can call getSession(), createSession(), or addMessage(). No other providers are affected: Claude, Cursor, and Codex session providers are stateless per-request (they read from disk or SQLite on demand) and have no equivalent eager-loading singleton. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1. Concurrent Writes (Issue siteboon#5): Added a per-session Promise-based write queue to saveSession() to ensure that overlapping save operations for the same session are executed sequentially, preventing data loss. 2. LRU Eviction (Issue siteboon#7, siteboon#8): Replaced FIFO eviction with Least Recently Used (LRU) logic based on the 'lastActivity' timestamp. This ensures that frequently used old sessions aren't prematurely evicted from memory. Updated both the run-time eviction and the post-load cleanup logic.
Replaced raw Date.now() based identifiers with random-suffixed strings in gemini-cli.js, cursor-cli.js, and openai-codex.js. This ensures that even if multiple sessions or processes are started within the same millisecond, their IDs remain unique, preventing silent state overwrites (Issue #2).
Contributor
📝 WalkthroughWalkthroughChanges introduce cryptographic randomization to session identifiers across CLI processes, implement per-session write-queue serialization in the session manager, switch to LRU-based session eviction, and ensure server startup waits for session manager readiness before proceeding. Changes
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/sessionManager.js`:
- Around line 149-174: saveSession currently captures the session object and
enqueues a write that may run after deleteSession has removed the session and
file, causing deleted sessions to be recreated; modify saveSession (and
symmetric logic near lines 220-225) to re-check that the session still exists in
this.sessions (or a deletion flag) immediately before writing and skip the write
if missing, and update deleteSession to either await/cancel the pending Promise
in saveQueues for that sessionId or mark the session as deleted (e.g., remove
from this.sessions and delete any queue entry in this.saveQueues) so queued
writes cannot recreate the file; reference saveSession, deleteSession,
this.saveQueues, this.sessions and _safeFilePath when making the changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cdf05348-d61c-4e92-a416-d702f1d7b5a9
📒 Files selected for processing (5)
server/cursor-cli.jsserver/gemini-cli.jsserver/index.jsserver/openai-codex.jsserver/sessionManager.js
Comment on lines
+149
to
+174
| // Save session to disk (Issue #5 - added queuing to prevent concurrent writes) | ||
| async saveSession(sessionId) { | ||
| const session = this.sessions.get(sessionId); | ||
| if (!session) return; | ||
|
|
||
| try { | ||
| const filePath = this._safeFilePath(sessionId); | ||
| await fs.writeFile(filePath, JSON.stringify(session, null, 2)); | ||
| } catch (error) { | ||
| // console.error('Error saving session:', error); | ||
| } | ||
| // Get current queue for this session or create new one | ||
| let currentQueue = this.saveQueues.get(sessionId) || Promise.resolve(); | ||
|
|
||
| // Chain the new write operation | ||
| const nextQueue = currentQueue.then(async () => { | ||
| try { | ||
| const filePath = this._safeFilePath(sessionId); | ||
| const data = JSON.stringify(session, null, 2); | ||
| await fs.writeFile(filePath, data); | ||
| } catch (error) { | ||
| // console.error('Error saving session:', error); | ||
| } | ||
| }).finally(() => { | ||
| // If no other operations are pending, remove the queue reference | ||
| if (this.saveQueues.get(sessionId) === nextQueue) { | ||
| this.saveQueues.delete(sessionId); | ||
| } | ||
| }); | ||
|
|
||
| this.saveQueues.set(sessionId, nextQueue); | ||
| return nextQueue; |
Contributor
There was a problem hiding this comment.
Prevent queued saves from recreating deleted sessions.
saveSession() captures session before the queued write runs, while deleteSession() does not wait for or cancel pending saves. A queued write can therefore run after fs.unlink() and recreate the deleted JSON file.
Proposed fix
async saveSession(sessionId) {
- const session = this.sessions.get(sessionId);
- if (!session) return;
+ if (!this.sessions.has(sessionId)) return;
// Get current queue for this session or create new one
let currentQueue = this.saveQueues.get(sessionId) || Promise.resolve();
// Chain the new write operation
- const nextQueue = currentQueue.then(async () => {
+ const nextQueue = currentQueue.catch(() => {}).then(async () => {
try {
+ const session = this.sessions.get(sessionId);
+ if (!session) return;
+
const filePath = this._safeFilePath(sessionId);
const data = JSON.stringify(session, null, 2);
await fs.writeFile(filePath, data);
} catch (error) {
// console.error('Error saving session:', error);
@@
async deleteSession(sessionId) {
this.sessions.delete(sessionId);
+
+ const pendingSave = this.saveQueues.get(sessionId);
+ if (pendingSave) {
+ await pendingSave.catch(() => {});
+ this.saveQueues.delete(sessionId);
+ }
try {
const filePath = this._safeFilePath(sessionId);
await fs.unlink(filePath);Also applies to: 220-225
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/sessionManager.js` around lines 149 - 174, saveSession currently
captures the session object and enqueues a write that may run after
deleteSession has removed the session and file, causing deleted sessions to be
recreated; modify saveSession (and symmetric logic near lines 220-225) to
re-check that the session still exists in this.sessions (or a deletion flag)
immediately before writing and skip the write if missing, and update
deleteSession to either await/cancel the pending Promise in saveQueues for that
sessionId or mark the session as deleted (e.g., remove from this.sessions and
delete any queue entry in this.saveQueues) so queued writes cannot recreate the
file; reference saveSession, deleteSession, this.saveQueues, this.sessions and
_safeFilePath when making the changes.
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
1 participant
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
This Pull Request hardens the Gemini session management layer to prevent data loss and improve memory efficiency.
Key Changes
saveSession()to ensure overlapping file writes for the same session are executed sequentially. This prevents race conditions where messages could be lost during rapid interactions.lastActivitytimestamp rather than Map insertion order.Date.now()timestamps to random-suffixed identifiers for session IDs and process keys, eliminating potential state overwrites during concurrent session starts.Impact
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor