feat(cli): add --delete flag to /exit command for session deletion#19332
feat(cli): add --delete flag to /exit command for session deletion#19332AbdulTawabJuly wants to merge 22 commits intogoogle-gemini:mainfrom
Conversation
Summary of ChangesHello @AbdulTawabJuly, 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 a significant privacy enhancement to the CLI by allowing users to delete their session data upon exit. By adding a Highlights
Changelog
Activity
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 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 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
|
4eabd9d to
82873ba
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a --delete flag to the /exit and /quit commands, allowing users to delete the current session's data upon exiting. The implementation correctly parses the flag and triggers the session deletion logic. However, the deletion implementation in slashCommandProcessor.ts uses synchronous I/O, which can make the CLI unresponsive on exit. Additionally, the method for identifying the session to be deleted is fragile and tightly coupled to the implementation details of the ChatRecordingService. I've provided feedback on how to make this more robust and performant.
82873ba to
4980dce
Compare
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we have updated our contribution policy (see Discussion #17383). We only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. All other community pull requests are subject to closure after 14 days if they do not align with our current focus areas. For this reason, we strongly recommend that contributors only submit pull requests against issues explicitly labeled as 'help-wanted'. This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding and for being part of our community! |
|
@AbdulTawabJuly, apologies for the bot closing this PR! We have reopened it. Please sync your branch to the latest |
|
@cocosheng-g I’ve synced this branch with main and resolved all conflicts. Could you please review it now? Thanks! |
|
Exiting should be something users do not have to think about, adding this Right now typing |
|
|
||
| - **Description:** Exit Gemini CLI. | ||
| - **Flags:** | ||
| - **`--delete`**: Exit and permanently delete the current session's history |
There was a problem hiding this comment.
clearly state this is optional please
| await tool.buildAndExecute(result.toolArgs, abortSignal, undefined, { | ||
| shellExecutionConfig: { | ||
| sanitizationConfig: DEFAULT_SANITIZATION_CONFIG, | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment |
There was a problem hiding this comment.
why are we adding all of these?
|
Hey @jackwotherspoon , thanks for the review, I've addressed all your feedback Autofire behavior : I tested the Docs : Updated the section title in Unrelated eslint-disable comments : Removed them all. I had added those initially because after syncing with main, the pre commit hook was failing with lint errors across several files. I suppressed them to unblock the merge, but I understand those belong to pre existing issues in those files and shouldn't be in this PR. They're now cleaned up. |
|
Hi @jackwotherspoon , could you review this PR again? I’ve implemented all the suggestions and changes you recommended. |
Summary
Add an optional
--deleteflag to the/exit(and/quit) command that allows users to delete the current session's history and temporary files when exiting the CLI. This provides a privacy-focused workflow for users who want to ensure their session data doesn't persist after they quit.Details
Implementation:
QuitActionReturninterface with an optionaldeleteSession?: booleanfieldquitCommand.tsto parse the--deleteflag from command argumentsslashCommandProcessor.tsto detect the flag and callChatRecordingService.deleteSession()before quittingTesting:
quitCommand.test.tsto verify flag parsingslashCommandProcessor.test.tsxto verify session deletion behaviorDocumentation:
docs/cli/commands.mdwith flag documentationdocs/cli/tutorials/session-management.mdRelated Issues
Fixes #18871
How to Validate
Manual Testing: