fix(ui): guard word-wrap while-loop against infinite loop on wide/oversized characters#25432
fix(ui): guard word-wrap while-loop against infinite loop on wide/oversized characters#25432AKIB473 wants to merge 2 commits intogoogle-gemini:mainfrom
Conversation
…racters When a single codepoint is wider than inputWidth (e.g. a CJK character or emoji in a very narrow terminal), the inner for-loop breaks immediately with splitIndex=0. The subsequent cpSlice(wordToProcess, 0) then returns the entire word unchanged, causing the while-loop to spin forever and hang the CLI. Fix: after the for-loop, if splitIndex is still 0, force it to 1 and set part to the first codepoint. This guarantees the loop always advances by at least one codepoint per iteration. Fixes google-gemini#19985
Two regression tests: 1. Wide CJK character (width=2) in a 1-column terminal — previously hung 2. Long ASCII word in a 3-column terminal — exercises the wrap loop Related to fix for google-gemini#19985
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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 a critical issue where the CLI would hang indefinitely when encountering wide characters that exceed the available input width. By ensuring the word-wrapping loop always makes progress even when a single character is wider than the allowed space, the fix guarantees terminal stability under various input conditions. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a guard in the word-wrapping logic within InputPrompt.tsx to prevent infinite loops when processing wide characters that exceed the input width. Regression tests were added to verify this fix. However, the current test implementation is ineffective because it does not trigger the necessary React re-renders to exercise the ghost text logic, and the reviewer suggests adopting the TestWrapper pattern to ensure the tests accurately reflect component behavior.
| describe('word-wrap infinite loop guard (issue #19985)', () => { | ||
| it('should not hang when a wide CJK character exceeds inputWidth', async () => { | ||
| // Regression test: when a single CJK/emoji character is wider than inputWidth, | ||
| // the word-wrap while-loop previously spun forever because splitIndex stayed 0 | ||
| // and cpSlice(word, 0) returned the full word unchanged. | ||
| // The fix ensures splitIndex advances by at least 1 codepoint each iteration. | ||
| const narrowProps = { ...props, inputWidth: 1 }; | ||
| const { stdin, unmount } = await renderWithProviders( | ||
| <TestInputPrompt {...narrowProps} />, | ||
| { isTTY: true }, | ||
| ); | ||
| // Type a CJK character (width=2) into a terminal that is only 1 column wide. | ||
| // Without the fix this blocks indefinitely; with the fix it completes promptly. | ||
| await stdin.write('中'); | ||
| // If we reach here the loop did not hang — test passes. | ||
| unmount(); | ||
| }); | ||
|
|
||
| it('should not hang when a long ASCII word exceeds inputWidth', async () => { | ||
| const narrowProps = { ...props, inputWidth: 3 }; | ||
| const { stdin, unmount } = await renderWithProviders( | ||
| <TestInputPrompt {...narrowProps} />, | ||
| { isTTY: true }, | ||
| ); | ||
| // "hello" is 5 chars wide but inputWidth is 3 — triggers the wrap loop. | ||
| await stdin.write('hello'); | ||
| unmount(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The regression tests added here do not appear to exercise the code path containing the fix. getGhostTextLines returns early if completion.promptCompletion.text is empty (as configured in the beforeEach mock) or if it doesn't start with buffer.text. Furthermore, mutating mockBuffer.text directly via stdin.write does not trigger a React re-render in TestInputPrompt, meaning the ghost text logic is never re-evaluated with the new input. To effectively test this, consider using the TestWrapper pattern (see line 2459) which uses state to trigger re-renders, and ensure the mock completion text is set to a value that triggers the wrapping logic. When updating these tests, ensure that any use of renderWithProviders includes a call to unmount at the end to prevent resource leaks. Additionally, since getGhostTextLines involves complex layout logic, ensure the implementation contains detailed comments explaining the height derivations.
References
- When using renderWithProviders in tests, the returned unmount function must be called at the end of the test to ensure proper cleanup and prevent resource leaks.
- For complex layout calculations that depend on component rendering logic (like conditional borders or padding), add detailed comments explaining how the height is derived to prevent incorrect refactoring.
|
This PR hasn't been updated in 7 days. Is it still active? |
|
Thank you for your interest in contributing to the project! We are closing this PR due to inactivity. |
Fixes #19985
Root cause
In
InputPrompt.tsx, the word-wrap loop that splits oversized words across lines:When a single codepoint is wider than
inputWidth(e.g. a CJK character in a very narrow terminal, or any wide Unicode character), thefor-loop breaks on the first iteration withsplitIndex = 0.cpSlice(wordToProcess, 0)returns the entire word unchanged, sowordToProcessnever shrinks and thewhile-loop spins forever, hanging the CLI.Fix
After the
for-loop, ifsplitIndex === 0, advance it to1and setpartto the first codepoint:This mirrors the pattern used by
cpSlice(line, col - 1, col)already in the file, and guarantees the loop always makes progress.Tests
Two regression tests added in
InputPrompt.test.tsx:Checklist