Migrate core render util to use xterm.js as part of the rendering loop.#19044
Merged
Migrate core render util to use xterm.js as part of the rendering loop.#19044
Conversation
Contributor
Summary of ChangesHello @jacob314, 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 undertakes a significant refactoring of the CLI's UI rendering infrastructure by replacing Highlights
Changelog
Activity
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed migration of the core rendering utility for tests from ink-testing-library to a custom solution using xterm.js. This change provides a more accurate terminal emulation for testing purposes. The new render utility and its associated helper functions like waitUntilReady are well-designed and have been consistently applied across the entire test suite. The removal of setTimeout-based delays in tests is a major improvement for test stability and speed. The code quality is high, and the changes are thorough and consistent. I have no critical or high-severity issues to report. This is an excellent piece of work.
jacob314
commented
Feb 14, 2026
| stderr: stderr as unknown as NodeJS.WriteStream, | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion | ||
| stdin: stdin as unknown as NodeJS.ReadStream, | ||
| debug: false, |
Contributor
Author
There was a problem hiding this comment.
previously we used debug: true which was needed for the hacks to get the last frame without going through a real ink pipeline.
jacob314
force-pushed
the
branch_xterm_test
branch
from
418e58e to
ffb6f4c
Compare
scidomino
approved these changes
Feb 14, 2026
|
Size Change: +919 B (0%) Total Size: 24.5 MB ℹ️ View Unchanged
|
gemini-cli
bot
added
area/platform
jacob314
force-pushed
the
branch_xterm_test
branch
6 times, most recently
from
86c3844 to
babdca9
Compare
krsjenmt
approved these changes
Feb 18, 2026
krsjenmt
approved these changes
Feb 18, 2026
jacob314
force-pushed
the
branch_xterm_test
branch
from
2b3255a to
db5c06e
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
jacob314
force-pushed
the
branch_xterm_test
branch
2 times, most recently
from
e7b4e21 to
4c2ca37
Compare
jacob314
force-pushed
the
branch_xterm_test
branch
from
4c2ca37 to
4a960db
Compare
This fixes a bug where tests that run animations skip them due to NODE_ENV=test. By modifying process.env.NODE_ENV to development during testing we ensure tests accurately test animation logic. We mutate process.env directly instead of using vi.stubEnv because vi.unstubAllEnvs in test-setup.ts clears the stub after each test.
…ould be resolved.
… to increase windows robustness. Add todos
jacob314
force-pushed
the
branch_xterm_test
branch
from
4a960db to
a686d2e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Switch all tests to xterm headless for snapshot rendering.
render.ts is the main file requiring real review. All other changes are minor snapshot churn due to switching renderers (new renderer always adds trailing newlines).
Test changes should just be mechanical code adding async and waiting for the renderer to be ready.
Diff is way smaller than it looks
Fixes #19045
It is possible we could have gotten away with not making these async but they will need to be async anyway in a bit when we update the ink renderer so didn't attempt to remove that.