-
Notifications
You must be signed in to change notification settings - Fork 10.4k
fix(core): use platform-specific shell commands in system prompt #16152
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
base: main
Are you sure you want to change the base?
fix(core): use platform-specific shell commands in system prompt #16152
Conversation
Summary of ChangesHello @Thomas-Shephard, 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 significantly improves the cross-platform compatibility of the Gemini CLI agent by making its system prompt aware of the underlying operating system. Previously, the agent would instruct users with Linux-centric shell commands, leading to execution failures on Windows machines running PowerShell. The changes introduce logic to adapt command syntax and utility suggestions, ensuring the agent provides relevant and executable instructions regardless of whether it's running on Windows or a Unix-like system. This enhances the agent's reliability and user experience across diverse development environments. 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. 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
|
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 effectively addresses the issue of using platform-specific shell commands in the system prompt, particularly for Windows environments. The changes are logical and the accompanying tests provide good coverage for both Windows and Linux platforms. I have one suggestion to enhance the flexibility of the tail and head command equivalents on Windows by not hardcoding the line count, which would allow the agent to specify it as needed.
| const tailCommand = isWindows ? 'Get-Content -Tail 10' : 'tail'; | ||
| const headCommand = isWindows ? 'Get-Content -TotalCount 10' : 'head'; |
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.
Hardcoding the line count to 10 for tail and head equivalents on Windows limits the agent's flexibility. The agent might need to inspect more or fewer lines depending on the context. A better approach would be to provide the command and parameter, allowing the agent to specify the number of lines itself. This would be more consistent with how the tail and head commands are used on Linux, where the agent can add flags like -n 20.
| const tailCommand = isWindows ? 'Get-Content -Tail 10' : 'tail'; | |
| const headCommand = isWindows ? 'Get-Content -TotalCount 10' : 'head'; | |
| const tailCommand = isWindows ? 'Get-Content -Tail' : 'tail'; | |
| const headCommand = isWindows ? 'Get-Content -TotalCount' : 'head'; |
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.
10 is the default for tail and head so I think this is fine
| expect(prompt).toContain("'Get-Content -Tail 10'"); | ||
| expect(prompt).toContain("'Get-Content -TotalCount 10'"); |
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.
To align with the suggested change in prompts.ts, this test should check for the command and parameter without the hardcoded line count.
| expect(prompt).toContain("'Get-Content -Tail 10'"); | |
| expect(prompt).toContain("'Get-Content -TotalCount 10'"); | |
| expect(prompt).toContain("'Get-Content -Tail'"); | |
| expect(prompt).toContain("'Get-Content -TotalCount'"); |
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.
See above
|
Comment from: jacob314
Done :) |
|
Comment from: jacob314
This is for token efficiency tail and head both default to 10 so that's why I went with it |
This comes from #16089, I accidentally deleted the forked branch so the pr was automatically closed :/
Summary
Fixes an issue where the agent would attempt to use Linux-specific shell commands (like grep and &&) when running on Windows. Since Gemini CLI's shell tool implementation forces the use of PowerShell on Windows, these Unix-style commands were failing. This PR makes the system prompt platform-aware, ensuring the agent uses compatible syntax and utilities.
Details
The core problem was a mismatch between the agent's instructions (which assumed a POSIX-like shell) and the CLI's actual execution environment on Windows (PowerShell).
Related Issues
Fixes #16151
How to Validate
npx vitest packages/core/src/core/prompts.test.tsPre-Merge Checklist