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 resolves an issue where the Gemini CLI agent was unable to write plan files due to the IDE's security restrictions on file operations outside the active workspace. The solution enhances the 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 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue where the ACP IDE client would block file writes outside the workspace. By passing the current working directory to AcpFileSystemService and checking if file paths are within the workspace root, the implementation now correctly falls back to the native file system for out-of-bounds operations. The changes are logical and the accompanying tests are thorough, verifying the new fallback mechanism. I have one suggestion to improve maintainability by reducing code duplication.
|
Size Change: +939 B (0%) Total Size: 26.2 MB
ℹ️ View Unchanged
|
gemini-cli
bot
added
area/non-interactive
nmcnamara-eng
left a comment
nmcnamara-eng
left a comment
There was a problem hiding this comment.
This seems like a sensible fix to the problem to my eyes. Though I didn't directly test it.
sripasg
force-pushed
the
sripas_fix_22191
branch
from
fa98e4e to
67d078d
Compare
… ensure file operations outside `cwd` use the fallback service.
sripasg
force-pushed
the
sripas_fix_22191
branch
from
67d078d to
266aec9
Compare
…lures in asking for perms to write plan md file (google-gemini#23612)
2 participants
Summary
The gemini-cli agent was sending requests to the ACP IDE client to write its .md plan files into a designated scratch directory (
~/.gemini/tmp/) located outside your project's workspace, causing the IDE to block the action for security reasons. To fix this, we need to update the ACP file system integration to detect when a target file resides outside the workspace root. When it does that, the agent bypasses the IDE's file system and natively writes the file to the local disk.Details
Main issue: The IDE's ACP implementation correctly denies writing files outside of its active workspace scope, which halted Plan Mode completely because the agent stores markdown plans in its internal global cache (
~/.gemini/tmp/).So we do the following:
AcpFileSystemServiceconstructor to accept and store the active workspace's root directory string (cwd).AcpFileSystemServicefrom both thenewSessionandloadSessionsetup methods withinpackages/cli/src/acp/acpClient.ts.isWithinRootutility and wrap the ACPreadTextFileandwriteTextFilemethods with it. IfisWithinRoot(filePath, this.root)resolves to false, the operation aborts the IDE proxy communication and calls the native Node.js fallback instead.fileSystemService.test.tssuite to instantiate the service with a dummy root path, adding dedicated tests to explicitly verify that out-of-bounds file paths trigger the fallback logic without calling themockConnection.Related Issues
Fixes #22191
How to Validate
Use ACP in plan mode ans ensure that there is no failures to automatically write a plan.md file for the prompt.
Pre-Merge Checklist