-
Notifications
You must be signed in to change notification settings - Fork 10.4k
fix: prevent /copy crash on Windows by skipping /dev/tty #15657
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
fix: prevent /copy crash on Windows by skipping /dev/tty #15657
Conversation
Summary of ChangesHello @ManojINaik, 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 a critical bug that caused the CLI to crash on Windows when users attempted to use the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 fixes a crash on Windows when using the /copy command by avoiding an attempt to access /dev/tty. The change is correct and includes relevant tests. My review includes two suggestions: one to improve test isolation to prevent flaky tests, and another to enhance the robustness of error handling for fs.createWriteStream on non-Windows platforms to prevent potential crashes, building upon the insight from the PR description.
On Windows, the pickTty() function attempted to open /dev/tty which
doesn't exist, causing an ENOENT error ('C:\dev\tty'). This caused
the CLI to crash when users ran the /copy command.
The fix adds a platform check to skip /dev/tty on Windows and
fall back to stderr/stdout or clipboardy for clipboard operations.
Fixes google-gemini#15648
Fixes google-gemini#15567
dfdd105 to
287de4c
Compare
jackwotherspoon
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.
nits
|
I have verified this fix works on my Windows machine. Once builds pass will merge. Thanks for this @ManojINaik 👏 |
On Windows, the pickTty() function attempted to open /dev/tty which doesn't exist, causing an ENOENT error ('C:\dev\tty'). This caused the CLI to crash when users ran the /copy command.
The fix adds a platform check to skip /dev/tty on Windows and fall back to stderr/stdout or clipboardy for clipboard operations.
Fixes #15648
Fixes #15567
Summary
Fix CLI crash on Windows when using
/copycommand. The pickTty() function attempted to open /dev/tty (Unix-only) which caused an unhandled ENOENT error on Windows.Details
Root Cause: On Windows, Node.js interprets /dev/tty as C:\dev\tty, which doesn't exist. The
fs.createWriteStream()call doesn't throw synchronously - it emits an async 'error' event that was unhandled.Solution: Added
process.platform !== 'win32'check before attempting /dev/tty. On Windows:stderr/stdoutif TTY available (for SSH sessions)clipboardylibrary for native Windows clipboardFiles Changed:
Related Issues
Fixes #15648 - CLI crashes on Windows when using /copy (Error: ENOENT 'C:\dev\tty')
Fixes #15567 - [Bug] Windows crash when using /copy: Error: ENOENT: no such file or directory
Fixes #15660
Fixes #15667
Fixes #15920
How to Validate
cd packages/cli && npx vitest run src/ui/utils/commandUtils.test.tsskips /dev/tty on Windows and uses stderr fallback for OSC-52uses clipboardy on native Windows without SSH/WSLgemini, generate a response, run/copy, verify no crash and content is copiedPre-Merge Checklist