feat(sessions): Add automatic session cleanup and retention policy#7662
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @bl-ue, 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 introduces automatic session cleanup and retention policies to the CLI, enhancing the management of recorded chat sessions. It allows users to configure how long or how many sessions are kept, preventing an accumulation of old data. This is a crucial step in the ongoing development of the chat recording and resuming feature, ensuring that the system remains efficient and user-friendly by automatically managing session storage.
Highlights
- Automatic Session Cleanup Configuration: Introduced new configuration settings for automatic chat session cleanup, allowing users to define retention policies based on age or count.
- Session Cleanup Logic Implementation: Implemented the core logic for identifying and deleting expired chat sessions, ensuring the current session is always preserved and handling various error scenarios gracefully.
- Integration into CLI Startup: Integrated the session cleanup process into the CLI's startup sequence, so cleanup runs automatically when the application initializes.
- Comprehensive Testing: Added extensive unit and integration tests for the session cleanup functionality, covering various configuration scenarios, edge cases, and error handling.
- Part of Larger Feature Rollout: This PR is the third in a series of nine, contributing to the larger feature of automatic chat recording and resuming, broken down from a monolithic PR for easier review and integration.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. 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.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces an automatic session cleanup feature, which is a great addition for managing disk space. The implementation is well-tested and handles many edge cases gracefully. However, I've found a few critical and high-severity issues in the core cleanup logic:
- The count-based retention (
maxCount) logic is flawed and may not delete the correct number of sessions. - The
minRetentionsetting is ignored, with a hardcoded 1-day minimum being used instead. - The logic to identify the currently active session to prevent its deletion is buggy and could fail, potentially leading to the deletion of the user's active session.
- The method for flagging the current session in the session list is fragile and should be made more robust.
I've provided detailed comments and code suggestions to address these issues. Once these are fixed, the feature should be solid.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a robust session cleanup mechanism based on retention policies (age and count). The implementation is well-structured, with clear separation of concerns into session utilities, cleanup logic, and configuration. The changes are accompanied by comprehensive unit and integration tests, ensuring the feature is reliable and handles edge cases gracefully. My main feedback concerns improving error handling to prevent silent failures in case of unexpected issues like file system permissions.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a session cleanup and retention policy, which is a valuable addition for managing session files. The implementation is robust, with good test coverage for various scenarios. I've identified a potential high-severity race condition in the cleanup logic that could affect users running multiple concurrent sessions, and I've provided a suggestion to address it. Additionally, I found a critical issue where a method is called that does not appear to be defined, which would likely cause a runtime error.
|
@mrcabbage972 Yes, done. Thank you for the review! |
|
@bl-ue can you please fix the CI issues so this PR can be merged? |
|
@mrcabbage972 Done. |
|
Hi @mrcabbage972, I'm not sure what the last test failure on Windows was about. It didn't appear to be related to my changes, and I couldn't reproduce it locally on Windows with |
… and retention policy (google-gemini#7662)
This PR is the third in a series of PRs that implement automatic chat recording and resuming in both interactive and non-interactive modes, with command line flags for non-interactive resuming and session management, and an interactive session browser.
PR #4401 was the original monolithic PR, which implemented this feature from the ground up. It's too large to merge as-is, however, and so it was broken up into several smaller PRs, of which this PR is the third.
Please see #4401's description for the full context of this PR. The original monolithic PR's description contains a detailed analysis of the entire system, with an example of the recording JSON and a demo video.