fix(cli): prevent Escape from clearing input buffer (#17083)#26339
fix(cli): prevent Escape from clearing input buffer (#17083)#26339cocosheng-g merged 1 commit intomainfrom
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 addresses an issue where pressing the Escape key would inadvertently clear the CLI input buffer. The changes introduce a more granular cancellation mechanism that distinguishes between different cancellation triggers, ensuring that user-typed content is preserved during an Escape-based cancellation while maintaining the expected buffer-clearing behavior for Ctrl+C interruptions. 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
|
|
Size Change: +1.02 kB (0%) Total Size: 33.9 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request introduces a clearBuffer parameter to the cancellation logic within the Gemini CLI UI, enabling more granular control over whether the input buffer is cleared during different types of cancellations (e.g., Escape vs. Ctrl+C). The changes span AppContainer, useAgentStream, and useGeminiStream, along with corresponding test updates. Feedback focuses on ensuring the buffer is correctly cleared even when tools are awaiting confirmation or when a cancellation is already in progress, to maintain consistent terminal behavior.
a738136 to
5fed122
Compare
5fed122 to
584e802
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a clearBuffer parameter to the cancellation logic within AppContainer, useAgentStream, and useGeminiStream. This enhancement allows the system to differentiate between cancellation actions, specifically preserving the input buffer when the 'Escape' key is pressed while clearing it for other cancellation events like 'Ctrl+C'. The update includes refactored hooks, improved state handling for idle cancellations, and updated unit tests to accommodate the new function signatures. I have no feedback to provide as there are no review comments.
devr0306
left a comment
There was a problem hiding this comment.
Summary
This pull request correctly implements the intended logic to prevent the Escape key from unexpectedly clearing the input buffer during tool executions, while maintaining the buffer-clearing behavior for Ctrl+C.
Review Notes
- Correctness: The change effectively addresses the issue by introducing a
clearBufferparameter tocancelOngoingRequest()and passing it down toonCancelSubmit(). - Maintainability: The modification is clean and isolated. Extending the existing cancellation flow with a boolean flag is a straightforward and appropriate solution.
- Testing: The corresponding
useGeminiStreamanduseAgentStreamtests pass successfully locally, indicating that the core cancellation logic remains sound.
Overall, the logic is sound and addresses the bug cleanly. Approved!
Summary
Fixes the input buffer being cleared unexpectedly when hitting
Escapeduring tool executions.Ctrl+Cwill still clear the buffer as expected.Details
The cancellation flow in
AppContainer.tsxtriggered bycancelOngoingRequest()insideuseGeminiStreamunconditionally cleared the input buffer when a tool was executing. This causedEscape(which uses the same flow) to erase user-typed text.This PR introduces a
clearBufferflag inonCancelSubmitto properly distinguish between explicit interrupts (likeCtrl+C, which defaults to clearing the buffer) and soft dismissals (likeEscape, which only stops the streaming/execution and leaves the buffer intact). It also hardens the internal cancellation logic inuseGeminiStreamto be resilient to transitionary UI states so thatCtrl+Ccorrectly aborts active tools even if the UI flashes into anIdlestate momentarily.Related Issues
Closes #17083
How to Validate
npm run build && ./packages/cli/dist/index.jsrun "sleep 10"Don't clear meEscape.Don't clear meremains in the input buffer.run "sleep 10"Clear meCtrl+C.Pre-Merge Checklist