fix: enable Gemini CLI session deletion and UI state update#598
Open
shrewdact wants to merge 1 commit intositeboon:mainfrom
Open
fix: enable Gemini CLI session deletion and UI state update#598shrewdact wants to merge 1 commit intositeboon:mainfrom
shrewdact wants to merge 1 commit intositeboon:mainfrom
Conversation
The delete endpoint only handled UI sessions (~/.gemini/sessions/) but not CLI sessions (~/.gemini/tmp/chats/). Also, handleSessionDelete only filtered project.sessions, leaving geminiSessions and codexSessions untouched so the UI didn't reflect the deletion without a refresh. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe PR adds functionality to clean up Gemini CLI session files when sessions are deleted through the UI. A new deletion function scans and removes matching CLI chat files from disk, integrates into the session deletion route handler with error suppression, and ensures frontend state lists stay synchronized during deletion. Changes
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useProjectsState.ts (1)
435-443:⚠️ Potential issue | 🟠 MajorKeep
sessionMeta.totaltied to Claude deletions only.This callback now handles Codex/Gemini removals too, but
sessionMeta.totalstill represents the Claudeproject.sessionscount. Decrementing it here for every project means one external-session delete can make Claude totals drift until the next refresh.🩹 Suggested guard
setProjects((prevProjects) => - prevProjects.map((project) => ({ - ...project, - sessions: project.sessions?.filter((session) => session.id !== sessionIdToDelete) ?? [], - codexSessions: project.codexSessions?.filter((session) => session.id !== sessionIdToDelete) ?? [], - geminiSessions: project.geminiSessions?.filter((session) => session.id !== sessionIdToDelete) ?? [], - sessionMeta: { - ...project.sessionMeta, - total: Math.max(0, (project.sessionMeta?.total as number | undefined ?? 0) - 1), - }, - })), + prevProjects.map((project) => { + const removedClaudeSession = + project.sessions?.some((session) => session.id === sessionIdToDelete) ?? false; + + return { + ...project, + sessions: project.sessions?.filter((session) => session.id !== sessionIdToDelete) ?? [], + codexSessions: project.codexSessions?.filter((session) => session.id !== sessionIdToDelete) ?? [], + geminiSessions: project.geminiSessions?.filter((session) => session.id !== sessionIdToDelete) ?? [], + sessionMeta: removedClaudeSession + ? { + ...project.sessionMeta, + total: Math.max(0, (((project.sessionMeta?.total as number | undefined) ?? 0) - 1)), + } + : project.sessionMeta, + }; + }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useProjectsState.ts` around lines 435 - 443, The current updater in setProjects unconditionally decrements project.sessionMeta.total for any sessionIdToDelete even when the deleted session was a Codex/Gemini session; change the logic so you only decrement sessionMeta.total when the deleted id was present in the Claude sessions array (project.sessions). Specifically, inside the setProjects mapper (the function updating project), compute a boolean like "wasClaude = project.sessions?.some(s => s.id === sessionIdToDelete)" and only apply Math.max(0, (project.sessionMeta?.total ?? 0) - 1) when wasClaude is true; otherwise keep project.sessionMeta.total unchanged. Keep the existing filtering of project.sessions, project.codexSessions, and project.geminiSessions as-is and reference setProjects, project.sessionMeta, project.sessions, project.codexSessions, project.geminiSessions, and sessionIdToDelete when making the change.
🤖 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/projects.js`:
- Around line 2544-2576: Narrow the broad try/catch blocks so real FS errors
aren't masked as "not found": where you call fs.readdir(geminiTmpDir) and
fs.readdir(chatsDir) inspect the caught error (e.g. err.code) and only convert
ENOENT (missing dir) into the session-not-found/continue behavior; rethrow any
other errors. Likewise, in the per-file try that reads/JSON-parses and calls
fs.unlink(filePath), do not swallow non-ENOENT errors—if
fs.readFile/JSON.parse/fs.unlink throw anything other than a missing-file error,
rethrow it so actual delete/read failures surface instead of falling through to
the final `Gemini CLI session file not found` error; reference
variables/functions: geminiTmpDir, projectDirs, chatsDir, fs.readdir,
fs.readFile, fs.unlink, sessionId.
In `@server/routes/gemini.js`:
- Around line 16-29: The current empty catch blocks around
sessionManager.deleteSession(sessionId) and deleteGeminiCliSession(sessionId)
hide real failures; change them to only swallow explicit "not found" errors and
rethrow everything else (e.g. check err.code === 'ENOENT' or err.name/err
instanceof NotFoundError), and ensure sessionNamesDb.deleteName(sessionId,
'gemini') is only reached after both delete attempts either succeeded or threw
not-found (so permission/disk errors from either delete will bubble as 500s);
use the function names sessionManager.deleteSession and deleteGeminiCliSession
and the sessionId variable to locate and update the handlers.
---
Outside diff comments:
In `@src/hooks/useProjectsState.ts`:
- Around line 435-443: The current updater in setProjects unconditionally
decrements project.sessionMeta.total for any sessionIdToDelete even when the
deleted session was a Codex/Gemini session; change the logic so you only
decrement sessionMeta.total when the deleted id was present in the Claude
sessions array (project.sessions). Specifically, inside the setProjects mapper
(the function updating project), compute a boolean like "wasClaude =
project.sessions?.some(s => s.id === sessionIdToDelete)" and only apply
Math.max(0, (project.sessionMeta?.total ?? 0) - 1) when wasClaude is true;
otherwise keep project.sessionMeta.total unchanged. Keep the existing filtering
of project.sessions, project.codexSessions, and project.geminiSessions as-is and
reference setProjects, project.sessionMeta, project.sessions,
project.codexSessions, project.geminiSessions, and sessionIdToDelete when making
the change.
🪄 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: fee446e6-c98f-4b84-b72f-35dcc5a58b7f
📒 Files selected for processing (3)
server/projects.jsserver/routes/gemini.jssrc/hooks/useProjectsState.ts
Comment on lines
+2544
to
+2576
| try { | ||
| projectDirs = await fs.readdir(geminiTmpDir); | ||
| } catch { | ||
| throw new Error(`Gemini CLI session not found: ${sessionId}`); | ||
| } | ||
|
|
||
| for (const projectDir of projectDirs) { | ||
| const chatsDir = path.join(geminiTmpDir, projectDir, 'chats'); | ||
| let chatFiles; | ||
| try { | ||
| chatFiles = await fs.readdir(chatsDir); | ||
| } catch { | ||
| continue; | ||
| } | ||
|
|
||
| for (const chatFile of chatFiles) { | ||
| if (!chatFile.endsWith('.json')) continue; | ||
| try { | ||
| const filePath = path.join(chatsDir, chatFile); | ||
| const data = await fs.readFile(filePath, 'utf8'); | ||
| const session = JSON.parse(data); | ||
| const fileSessionId = session.sessionId || chatFile.replace('.json', ''); | ||
| if (fileSessionId === sessionId) { | ||
| await fs.unlink(filePath); | ||
| return true; | ||
| } | ||
| } catch { | ||
| continue; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| throw new Error(`Gemini CLI session file not found: ${sessionId}`); |
Contributor
There was a problem hiding this comment.
Don't turn real filesystem failures into a fake "not found".
The broad catches here also swallow readdir()/unlink() failures. If the matching file is found but Line 2567 fails, this helper falls through to Gemini CLI session file not found, which hides the real delete failure.
🩹 Suggested narrowing
let projectDirs;
try {
projectDirs = await fs.readdir(geminiTmpDir);
- } catch {
- throw new Error(`Gemini CLI session not found: ${sessionId}`);
+ } catch (error) {
+ if (error.code === 'ENOENT') {
+ throw new Error(`Gemini CLI session not found: ${sessionId}`);
+ }
+ throw error;
}
@@
for (const chatFile of chatFiles) {
if (!chatFile.endsWith('.json')) continue;
- try {
- const filePath = path.join(chatsDir, chatFile);
- const data = await fs.readFile(filePath, 'utf8');
- const session = JSON.parse(data);
- const fileSessionId = session.sessionId || chatFile.replace('.json', '');
- if (fileSessionId === sessionId) {
- await fs.unlink(filePath);
- return true;
- }
- } catch {
+ const filePath = path.join(chatsDir, chatFile);
+ let session;
+ try {
+ session = JSON.parse(await fs.readFile(filePath, 'utf8'));
+ } catch {
continue;
}
+
+ const fileSessionId = session.sessionId || chatFile.replace('.json', '');
+ if (fileSessionId !== sessionId) {
+ continue;
+ }
+
+ await fs.unlink(filePath);
+ return true;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| projectDirs = await fs.readdir(geminiTmpDir); | |
| } catch { | |
| throw new Error(`Gemini CLI session not found: ${sessionId}`); | |
| } | |
| for (const projectDir of projectDirs) { | |
| const chatsDir = path.join(geminiTmpDir, projectDir, 'chats'); | |
| let chatFiles; | |
| try { | |
| chatFiles = await fs.readdir(chatsDir); | |
| } catch { | |
| continue; | |
| } | |
| for (const chatFile of chatFiles) { | |
| if (!chatFile.endsWith('.json')) continue; | |
| try { | |
| const filePath = path.join(chatsDir, chatFile); | |
| const data = await fs.readFile(filePath, 'utf8'); | |
| const session = JSON.parse(data); | |
| const fileSessionId = session.sessionId || chatFile.replace('.json', ''); | |
| if (fileSessionId === sessionId) { | |
| await fs.unlink(filePath); | |
| return true; | |
| } | |
| } catch { | |
| continue; | |
| } | |
| } | |
| } | |
| throw new Error(`Gemini CLI session file not found: ${sessionId}`); | |
| let projectDirs; | |
| try { | |
| projectDirs = await fs.readdir(geminiTmpDir); | |
| } catch (error) { | |
| if (error.code === 'ENOENT') { | |
| throw new Error(`Gemini CLI session not found: ${sessionId}`); | |
| } | |
| throw error; | |
| } | |
| for (const projectDir of projectDirs) { | |
| const chatsDir = path.join(geminiTmpDir, projectDir, 'chats'); | |
| let chatFiles; | |
| try { | |
| chatFiles = await fs.readdir(chatsDir); | |
| } catch { | |
| continue; | |
| } | |
| for (const chatFile of chatFiles) { | |
| if (!chatFile.endsWith('.json')) continue; | |
| const filePath = path.join(chatsDir, chatFile); | |
| let session; | |
| try { | |
| session = JSON.parse(await fs.readFile(filePath, 'utf8')); | |
| } catch { | |
| continue; | |
| } | |
| const fileSessionId = session.sessionId || chatFile.replace('.json', ''); | |
| if (fileSessionId !== sessionId) { | |
| continue; | |
| } | |
| await fs.unlink(filePath); | |
| return true; | |
| } | |
| } | |
| throw new Error(`Gemini CLI session file not found: ${sessionId}`); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/projects.js` around lines 2544 - 2576, Narrow the broad try/catch
blocks so real FS errors aren't masked as "not found": where you call
fs.readdir(geminiTmpDir) and fs.readdir(chatsDir) inspect the caught error (e.g.
err.code) and only convert ENOENT (missing dir) into the
session-not-found/continue behavior; rethrow any other errors. Likewise, in the
per-file try that reads/JSON-parses and calls fs.unlink(filePath), do not
swallow non-ENOENT errors—if fs.readFile/JSON.parse/fs.unlink throw anything
other than a missing-file error, rethrow it so actual delete/read failures
surface instead of falling through to the final `Gemini CLI session file not
found` error; reference variables/functions: geminiTmpDir, projectDirs,
chatsDir, fs.readdir, fs.readFile, fs.unlink, sessionId.
Comment on lines
+16
to
29
| // Try deleting from UI sessions and CLI sessions | ||
| let deleted = false; | ||
| try { | ||
| await sessionManager.deleteSession(sessionId); | ||
| deleted = true; | ||
| } catch { } | ||
|
|
||
| try { | ||
| await deleteGeminiCliSession(sessionId); | ||
| deleted = true; | ||
| } catch { } | ||
|
|
||
| sessionNamesDb.deleteName(sessionId, 'gemini'); | ||
| res.json({ success: true }); |
Contributor
There was a problem hiding this comment.
Only suppress explicit "not found" cases here.
Both catch {} blocks hide real delete failures, and sessionManager.deleteSession() does not tell you whether anything actually existed. That means Line 29 can still return { success: true } and Line 28 can clear the custom name even when the CLI file could not be removed.
If you want DELETE to stay idempotent, keep the success response for true misses, but only swallow the helper's not-found case. Permission/disk errors should still bubble as 500s.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/gemini.js` around lines 16 - 29, The current empty catch blocks
around sessionManager.deleteSession(sessionId) and
deleteGeminiCliSession(sessionId) hide real failures; change them to only
swallow explicit "not found" errors and rethrow everything else (e.g. check
err.code === 'ENOENT' or err.name/err instanceof NotFoundError), and ensure
sessionNamesDb.deleteName(sessionId, 'gemini') is only reached after both delete
attempts either succeeded or threw not-found (so permission/disk errors from
either delete will bubble as 500s); use the function names
sessionManager.deleteSession and deleteGeminiCliSession and the sessionId
variable to locate and update the handlers.
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
deleteGeminiCliSession()to delete CLI sessions from~/.gemini/tmp/chats/handleSessionDeleteto also filtergeminiSessionsandcodexSessionsso the UI updates immediately without requiring a page refreshTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes