fix(plan): isolate plan files per session#18757
Conversation
Summary of ChangesHello @Adib234, 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 improvement to how temporary plan files are managed by implementing session-based isolation. By ensuring that each session has its own dedicated directory for plan storage, it effectively eliminates potential conflicts and data corruption that could arise from multiple concurrent sessions. This change also includes necessary updates to the file system policy to maintain security and proper operation within the new directory structure. 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
|
|
Size Change: +423 B (0%) Total Size: 24.3 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request successfully implements session-based isolation for plan storage by incorporating the sessionId into the plans directory path. This change prevents plan file collisions between different sessions and ensures that the Plan Mode policy correctly restricts file operations to the session-specific plans folder. The modifications to the Storage class, the Config class integration, and the updated plan.toml policy file are all well-aligned with the stated goal, and a new test case for getProjectTempPlansDir ensures proper coverage. However, it is important to note that a full security analysis could not be performed as the model was unable to read the content of the specified files, therefore no vulnerabilities were found or assessed by the security agent.
jerop
left a comment
There was a problem hiding this comment.
we should flip the order of plans and session id
~/.gemini/tmp/<project>/plans/<session-id>/ --> ~/.gemini/tmp/<project>/<session-id>/plans/
this way, we can add easily tasks and other planning items within the session subdirectory:
~/.gemini/tmp/<project>/<session-id>/plans/~/.gemini/tmp/<project>/<session-id>/tasks/
this would be much better than:
~/.gemini/tmp/<project>/plans/<session-id>/~/.gemini/tmp/<project>/tasks/<session-id>/
then down the road, we can move the other artifacts like chat history within this subdirectory e.g. ~/.gemini/tmp/<project>/chats --> ~/.gemini/tmp/<project>/<session-id>/chats (out of scope for now though)
aaefb1a to
7c68d20
Compare
Summary
This PR implements session-based isolation for plan storage. By including the sessionId in the plans directory path, it prevents plan file collisions between different sessions in the same project and ensures that the Plan Mode policy correctly restricts file operations to the session-specific plans folder.
Details
(/plans/<sessionId>/<file>.md), ensuring write_file and replace tools remain functional and secure within Plan Mode.Related Issues
Fixes #18723
How to Validate
Start a new session and create a plan. End it and try re-running the same prompt to create the plan. Gemini CLI should have no memory of the plan that was created in the past and act as if it's creating the plan for the first time.
Pre-Merge Checklist