Conversation
gemini-cli
Bot
added
the
status/need-issue
|
Size Change: -30 B (0%) Total Size: 26.5 MB
ℹ️ View Unchanged
|
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 refactors the initialization logic for the console message store to ensure it only runs when the interactive UI is active. By decoupling this from the main CLI execution flow, the changes prevent console output from being suppressed in non-interactive environments, thereby fixing existing test failures. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request moves the initialization of the console store from the main entry point in gemini.tsx to the startInteractiveUI function in interactiveCli.tsx. Feedback was provided regarding the need to drain backlogs after initialization to ensure early messages are captured, and a concern was raised about the removal of the store initialization in the main entry point affecting expected output redirection and security.
| resumedSessionData: ResumedSessionData | undefined, | ||
| initializationResult: InitializationResult, | ||
| ) { | ||
| initializeConsoleStore(); |
There was a problem hiding this comment.
The initializeConsoleStore call registers listeners for CoreEvent.ConsoleLog and CoreEvent.Output, which collect raw data. To prevent terminal injection attacks, ensure that UI components rendering these messages properly sanitize terminal control characters, treating all user-provided data as untrusted. Additionally, to ensure early startup messages are captured, explicitly drain the coreEvents backlog after initializing the store.
| initializeConsoleStore(); | |
| initializeConsoleStore(); | |
| coreEvents.drainBacklogs(); |
References
- Always treat user-provided data as untrusted and apply proper validation and sanitization at the point of use, even if it is believed to have been filtered or sanitized upstream.
| process.exit(ExitCodes.FATAL_INPUT_ERROR); | ||
| } | ||
|
|
||
| initializeConsoleStore(); |
There was a problem hiding this comment.
The removal of initializeConsoleStore() in gemini.tsx is incorrect as the redirection of output to stderr is intentional and expected behavior. Reverting this change is necessary to maintain the intended architecture. Furthermore, the resulting logic that writes raw data chunks directly to stdout and stderr poses a terminal injection risk. All user-provided data must be treated as untrusted and sanitized at the point of use, following the principle of defense-in-depth.
References
- In gemini.tsx, the ConsolePatcher is intentionally configured with stderr: true to redirect all console output to the standard error stream; this is the expected behavior.
- Always treat user-provided data as untrusted and apply proper validation and sanitization at the point of use, even if it is believed to have been filtered or sanitized upstream.
4 participants
Summary
Fixes the tests by not swallowing output in non-interactive mode.
Details
This was broken by me when I impatiently force-merged #24235 skipping the e2e tests