Avoid persisting empty resume sessions#27770
Conversation
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 improves session management by ensuring that only meaningful conversations are persisted and surfaced in the UI. By defining what constitutes 'resumable content'—specifically excluding command-only or system-generated messages—the system now avoids cluttering the session history with empty or trivial startup artifacts. This change includes robust cleanup logic to discard abandoned sessions upon exit and updates existing listing utilities to respect these new filtering criteria. 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/L
|
There was a problem hiding this comment.
Code Review
This pull request introduces logic to identify and clean up non-resumable sessions (such as startup-only, command-only, or internal-context-only sessions) to prevent them from cluttering the session history. It adds filtering helpers, registers a cleanup task in the interactive CLI, and includes comprehensive unit tests. The review feedback highlights two critical issues: first, the cleanup function is removed from the registry during normal exit without being executed, which prevents session cleanup on clean exits; second, message.content lacks a safety guard, which could lead to a runtime TypeError and crash the CLI when processing malformed or legacy records.
|
Size Change: +2.01 kB (+0.01%) Total Size: 33.9 MB
ℹ️ View Unchanged
|
galz10
left a comment
There was a problem hiding this comment.
Code Review
Intent Summary: Avoid persisting and listing interactive sessions that contain only commands (e.g., /resume) or system contexts without any real conversational content (user text or model responses).
🧹 Refactoring & Nits (P2/P3)
Recommended improvements.
packages/cli/src/utils/sessionUtils.ts&packages/cli/src/utils/sessionUtils.test.ts: The newly addedhasResumableContentwrapper function is exported but never actually used in the codebase (getAllSessionFilesnow correctly consumes thehasResumableContentboolean property directly from the parsed backend object). Remove this wrapper function entirely and move its extensive test cases directly intopackages/core/src/services/chatRecordingService.test.tsto test the actual backend implementation.packages/core/src/services/chatRecordingService.ts:102: TheisIgnoredUserContentfunction duplicates the exact skip-prefix rules (startsWith('/'),startsWith('<session_context>'), etc.) already hardcoded inpackages/core/src/utils/sessionUtils.ts. Extract this into a shared utility function inpackages/core/src/utils/sessionUtils.tsto ensure skip-rules remain in sync.packages/cli/src/utils/sessionUtils.tsandpackages/core/src/services/chatRecordingService.ts: ThehasUserOrAssistantMessageutility function, its tests, and its corresponding property on theloadConversationRecordreturn type are now dead code sincegetAllSessionFileshas fully migrated tohasResumableContent. Remove them.
📝 Metadata Review
Feedback on PR description or commit message clarity.
- The PR description is extremely clear, links to the relevant issue, and provides concrete, platform-specific validation steps. Excellent work detailing the intent and testing strategy.
Port from google-gemini/gemini-cli#27770: starting the CLI and immediately quitting (or rotating with /new, /clear) left an empty untitled session row behind. These ghost rows pile up in /resume, `hermes sessions list`, and the in-chat recent-sessions browser. - SessionDB.delete_session_if_empty(): transactional check-and-delete that only removes rows with no messages, no title, and no child sessions (delegate subagent parents are preserved). Also removes on-disk transcript files via the existing _remove_session_files. - HermesCLI._discard_session_if_empty(): thin wrapper, wired into the cli_close shutdown path and the new_session() rotation path. Skipped when /exit --delete already handles removal. Unlike the one-shot prune_empty_ghost_sessions migration (TUI-only, 24h-old rows), this prevents new ghost rows from accumulating at the moment they would be created.
Port from google-gemini/gemini-cli#27770: starting the CLI and immediately quitting (or rotating with /new, /clear) left an empty untitled session row behind. These ghost rows pile up in /resume, `hermes sessions list`, and the in-chat recent-sessions browser. - SessionDB.delete_session_if_empty(): transactional check-and-delete that only removes rows with no messages, no title, and no child sessions (delegate subagent parents are preserved). Also removes on-disk transcript files via the existing _remove_session_files. - HermesCLI._discard_session_if_empty(): thin wrapper, wired into the cli_close shutdown path and the new_session() rotation path. Skipped when /exit --delete already handles removal. Unlike the one-shot prune_empty_ghost_sessions migration (TUI-only, 24h-old rows), this prevents new ghost rows from accumulating at the moment they would be created.
Port from google-gemini/gemini-cli#27770: starting the CLI and immediately quitting (or rotating with /new, /clear) left an empty untitled session row behind. These ghost rows pile up in /resume, `hermes sessions list`, and the in-chat recent-sessions browser. - SessionDB.delete_session_if_empty(): transactional check-and-delete that only removes rows with no messages, no title, and no child sessions (delegate subagent parents are preserved). Also removes on-disk transcript files via the existing _remove_session_files. - HermesCLI._discard_session_if_empty(): thin wrapper, wired into the cli_close shutdown path and the new_session() rotation path. Skipped when /exit --delete already handles removal. Unlike the one-shot prune_empty_ghost_sessions migration (TUI-only, 24h-old rows), this prevents new ghost rows from accumulating at the moment they would be created.
Port from google-gemini/gemini-cli#27770: starting the CLI and immediately quitting (or rotating with /new, /clear) left an empty untitled session row behind. These ghost rows pile up in /resume, `hermes sessions list`, and the in-chat recent-sessions browser. - SessionDB.delete_session_if_empty(): transactional check-and-delete that only removes rows with no messages, no title, and no child sessions (delegate subagent parents are preserved). Also removes on-disk transcript files via the existing _remove_session_files. - HermesCLI._discard_session_if_empty(): thin wrapper, wired into the cli_close shutdown path and the new_session() rotation path. Skipped when /exit --delete already handles removal. Unlike the one-shot prune_empty_ghost_sessions migration (TUI-only, 24h-old rows), this prevents new ghost rows from accumulating at the moment they would be created.
Port from google-gemini/gemini-cli#27770: starting the CLI and immediately quitting (or rotating with /new, /clear) left an empty untitled session row behind. These ghost rows pile up in /resume, `hermes sessions list`, and the in-chat recent-sessions browser. - SessionDB.delete_session_if_empty(): transactional check-and-delete that only removes rows with no messages, no title, and no child sessions (delegate subagent parents are preserved). Also removes on-disk transcript files via the existing _remove_session_files. - HermesCLI._discard_session_if_empty(): thin wrapper, wired into the cli_close shutdown path and the new_session() rotation path. Skipped when /exit --delete already handles removal. Unlike the one-shot prune_empty_ghost_sessions migration (TUI-only, 24h-old rows), this prevents new ghost rows from accumulating at the moment they would be created.
Port from google-gemini/gemini-cli#27770: starting the CLI and immediately quitting (or rotating with /new, /clear) left an empty untitled session row behind. These ghost rows pile up in /resume, `hermes sessions list`, and the in-chat recent-sessions browser. - SessionDB.delete_session_if_empty(): transactional check-and-delete that only removes rows with no messages, no title, and no child sessions (delegate subagent parents are preserved). Also removes on-disk transcript files via the existing _remove_session_files. - HermesCLI._discard_session_if_empty(): thin wrapper, wired into the cli_close shutdown path and the new_session() rotation path. Skipped when /exit --delete already handles removal. Unlike the one-shot prune_empty_ghost_sessions migration (TUI-only, 24h-old rows), this prevents new ghost rows from accumulating at the moment they would be created.
Summary
Avoid surfacing empty or command-only sessions in resume flows, and clean up startup-only interactive sessions on exit so immediately-started-and-quit sessions are not persisted.
Details
Related Issues
Related to #18007
How to Validate
Pre-Merge Checklist