-
Notifications
You must be signed in to change notification settings - Fork 9.7k
feat: Multi-Directory Workspace Support (part1: add --include-directories option)
#4605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @y-okt, 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 comprehensive multi-directory workspace support, enabling users to define a virtual workspace composed of files and folders from disparate locations. This significantly enhances workflow flexibility by allowing the CLI and its integrated tools to operate seamlessly across multiple related projects or scattered files within a single session.
Highlights
- Multi-Directory Workspace Core: Introduced a new
WorkspaceContextclass in the@google/gemini-cli-corepackage to manage multiple workspace directories. This class handles adding directories, resolving paths, and validating if a given path falls within any of the registered workspace directories, including robust handling of symbolic links. - CLI Argument for Initial Workspace Setup: Added a new
--include-directoriesCLI argument, allowing users to specify additional directories to be included in the workspace when starting a session. This argument supports multiple values or comma-separated paths. - Interactive Workspace Management Command: Implemented a new
/directoryslash command in the CLI, with two subcommands:/directory add <path>to dynamically add new directories to the current session's workspace, and/directory showto list all currently included workspace directories. Note that/directory addis restricted in certain sandbox profiles, encouraging the use of--include-directoriesinstead. - Tooling Integration and Behavior Changes: Updated several core tools (
edit,glob,grep,ls,read-file,shell,write-file) to leverage the newWorkspaceContext. This ensures that file operations and path validations are performed against the entire virtual workspace. Notably, theglobandgreptools now search across all workspace directories by default if no specific path is provided, enhancing their utility in multi-project environments. - LLM Context Awareness: Modified the Gemini client's environment setup to inform the LLM about all directories currently included in the workspace, providing a more comprehensive understanding of the user's working context.
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 is currently in preview and 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 to provide feedback.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces multi-directory workspace support, a significant feature that enhances workflow flexibility. The implementation is well-structured, introducing a WorkspaceContext to manage multiple directories and updating various tools to be workspace-aware. The changes are comprehensive, touching documentation, configuration, core logic, and tests.
The most critical area of this change is path validation and security, which has been handled well by centralizing the logic in WorkspaceContext and using fs.realpathSync to mitigate risks from symbolic links.
I've identified a critical issue in the shell tool related to how it resolves directories by name, which could lead to ambiguity and incorrect behavior. My feedback focuses on resolving this to ensure the feature is robust and secure.
Overall, this is a great addition to the project. Addressing the identified issue will make it ready for merging.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant and well-structured feature for multi-directory workspace support. The core of this is the new WorkspaceContext class, which is correctly integrated into various file-system tools to centralize path validation and expand their capabilities. The new CLI options and commands are accompanied by good documentation and tests.
However, I've identified a critical security vulnerability in the isPathWithinWorkspace method. The current implementation does not correctly handle paths to non-existent files within symlinked directories, creating a potential for sandbox escape. I have provided a detailed comment with a suggested fix to address this vulnerability. Once this critical issue is resolved, the PR should be in excellent shape.
|
/gemini summary |
Summary of ChangesThis pull request significantly enhances the CLI's capabilities by introducing multi-directory workspace support. It allows users to define a virtual workspace composed of multiple, potentially disparate, directories. This feature is exposed via a new CLI argument for initial setup and a new slash command for runtime management. All relevant file system interaction tools have been updated to leverage this new workspace context, ensuring operations are performed correctly and securely across all included directories. Highlights
Changelog
Activity
|
jacob314
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding directories via command line arguments works well but I am having trouble when I add them from the CLI. Unless there is an easy fix I would suggest removing support for modifying the project from within the tool from this pull request and adding it to a follow up pull request instead. this pull request is already large and I am not surprised there are some additional challenges dynamically changing the project while the app is running.
a6a413c to
0c0052c
Compare
|
Hi @jacob314, thank you for reviewing. Separating PRs sounds good to me. I'll let you know once that is ready. |
|
@allenhutchison would be great to get your feedback on this as well as you filed the original issue. |
|
@y-okt thank you for working on this. Here are some high level comments to go along with my more detailed review.
This is OS dependant, and I'm not an expert on special characters that make sense in non-linux environments, it seems reasonable to special case at least ~ for the home directory.
Obviously this could come in a different PR, we don't need to add it to this one. |
|
Hi @allenhutchison, thanks for the feedback! Regarding your comments:
|
|
One other point. In my testing, it looks like you haven't added support for the read-many-files tool: |
|
Hi @allenhutchison, thanks for pointing this out! You're absolutely right. It appears the The error message This is a critical correctness issue for the multi-directory feature, as it prevents the |
|
Thank you for your feedback, @allenhutchison and @jacob314 , I'll address these points and will ask for reviews when ready! |
ad2cacc to
37e6d0c
Compare
ffd0ce1 to
1ed1da5
Compare
|
Hi @allenhutchison and @jacob314 , could you please review #4939 ? I added |
|
heads up that there are 4 merge conflicts. can you resolve those before we review? |
d1497d9 to
2fc0c7f
Compare
Head branch was pushed to by a user without write access
bd748a2 to
fb3839c
Compare
|
@allenhutchison Hi, thank you very much for creating a fix for tests! I really appreciate it. I applied your change to this branch. Could you please review again? Thank you! |
|
@allenhutchison @jacob314 Also, the auto-merge was cancelled, if everything is fine after your review, could you please enable that again? |
jacob314
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…ories` option) (#4605) Co-authored-by: Allen Hutchison <[email protected]>
…ories` option) (google-gemini#4605) Co-authored-by: Allen Hutchison <[email protected]>
…ories` option) (google-gemini#4605) Co-authored-by: Allen Hutchison <[email protected]>
|
Good evening, @allenhutchison, @jacob314, and @y-okt! I established the addition of the directories to the user-wide {
"checkpointing":
{
"enabled": true
},
"includeDirectories":
[
"~/.local/bin/personal-executables/",
"~/.gitmoji/",
"~/Git/GitHub/Gustavo/panforge-non-git"
],
"loadMemoryFromIncludeDirectories": true,
"selectedAuthType": "oauth-personal",
"showLineNumbers": false,
"theme": "GitHub"
}Moreover, and I faithfully replicated this same configuration in the project ( I entreated Gemini to peruse these contents aforementioned in the project-wide 1. Read the «`~/.local/bin/personal-executables/git-filter-branch`» script;
2. Repair thence to the «`../panforge-non-git`» directory and consider analysing the files at the `assets/docs/gemini` directory;
3. In continuation, subject the whole of the «`../panforge-non-git`» directory;
4. Upon the conclusion of such labours, compose a commit message adorned with an appropriate emoji enumerated within the «`~/.gitmoji/gitmojis.json`» register. But I was confused when it informed me that it was unable to access the directory aforementioned in the item 2 because:
In continuation, I effected some amendments, replacing the tilde with the full and true absolute path in both user-wide and project-wide But, afterwards, this error remains unchanged and persists. Then I updated the remaining path
I reviewed the result display in the chat session log: I really am very confused with it. |
|
Hi @gusbemacbe, Thank you for providing such detailed information about the issue you're encountering. I understand your confusion, especially since the The changes in this pull request ( Your debug output is very helpful: This message indicates that while you intended to include This suggests that the issue lies in why To help diagnose this, could you please check the following:
To get more insight, you could temporarily add some For example, you could add: // packages/core/src/utils/workspaceContext.ts
private addDirectoryInternal(
directory: string,
basePath: string = process.cwd(),
): void {
const absolutePath = path.isAbsolute(directory)
? directory
: path.resolve(basePath, directory);
console.log(`Attempting to add directory: ${absolutePath}`); // Add this line
if (!fs.existsSync(absolutePath)) {
console.error(`Error: Directory does not exist: ${absolutePath}`); // Add this line
throw new Error(`Directory does not exist: ${absolutePath}`);
}
const stats = fs.statSync(absolutePath);
if (!stats.isDirectory()) {
console.error(`Error: Path is not a directory: ${absolutePath}`); // Add this line
throw new Error(`Path is not a directory: ${absolutePath}`);
}
let realPath: string;
try {
realPath = fs.realpathSync(absolutePath);
console.log(`Successfully resolved to real path: ${realPath}`); // Add this line
} catch (error) {
console.error(`Error resolving path ${absolutePath}: ${error}`); // Add this line
throw new Error(`Failed to resolve path: ${absolutePath}`);
}
this.directories.add(realPath);
console.log(`Added ${realPath} to workspace context.`); // Add this line
}This logging should help pinpoint exactly where the process of adding |
|
@gusbemacbe Hello, thank you for raising that report. Could you please raise a GitHub issue? Also, some additional information is appreciated, such as an absolute path to the current directory. Thanks! |
|
@gusbemacbe Thank you! Will take a look |
…ories` option) (google-gemini#4605) Co-authored-by: Allen Hutchison <[email protected]>
…ories` option) (google-gemini#4605) Co-authored-by: Allen Hutchison <[email protected]>

TLDR
As described in #1118, enable users to include files and folders from disparate locations within a single session, creating a virtual workspace. This will improve workflow flexibility, especially when related project files are spread across different folders.
This was suggested by @jacob314 to separate PRs, and this is one of the PRs to add
--include-directoriesoption. In another PR, I'm planning to work on/directory add <dir>,/directory show, and configuration supports.Dive Deeper
Supported
--include-directoriesflag. By this flag, the following features are supported.Reviewer Test Plan
ReadFile
"@" command
Edit
Write
ReadManyFiles
shell
Testing Matrix
Linked issues / bugs
Partially resolve #1118 . Having
/directorycommand and having JSON config (suggested by @allenhutchison (comment) would be the next steps