ci(triage): fix comment spam by separating internal explanation from public comment#26672
ci(triage): fix comment spam by separating internal explanation from public comment#26672itzzSPcoder wants to merge 5 commits intogoogle-gemini:mainfrom
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 refactors the authentication error handling mechanism to improve the user experience and security of the CLI. By centralizing the detection and formatting of authentication errors, the changes ensure consistent and safe error reporting across the application. Note: The provided PR description regarding comment spam appears to be unrelated to the actual code changes, which focus exclusively on authentication error management. Highlights
Ignored Files
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 the 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 counterproductive. 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 centralizes and improves authentication error handling by introducing formatAuthError and isAuthRelatedError utilities. It updates handleError to provide more user-friendly terminal output and consistent exit behavior for authentication failures. Feedback suggests using the AuthType enum instead of hardcoded strings, passing pre-parsed error messages to formatting functions to avoid redundant parsing, and preserving original error types when generating JSON output to maintain diagnostic metadata.
| char === '\n' || char === '\r' || char === '\t' ? char : '' | ||
| ); | ||
|
|
||
| const fix = authType === 'google-cloud' |
There was a problem hiding this comment.
The check authType === 'google-cloud' uses a hardcoded string that does not match the AuthType values used elsewhere in the codebase (e.g., LOGIN_WITH_GOOGLE, COMPUTE_ADC, USE_VERTEX_AI). This will likely cause the wrong 'Fix' instructions to be displayed for Google Cloud-based authentication methods. You should use the AuthType enum/object to perform this check correctly.
| ); | ||
|
|
||
| if (isAuthError) { | ||
| errorMessage = formatAuthError(error, config.getContentGeneratorConfig()?.authType); |
There was a problem hiding this comment.
formatAuthError is being passed the raw error object, which causes it to re-parse the error message. However, parseAndFormatApiError has already been called on line 114, which likely provides a more accurate and sanitized message (e.g., by handling specific API error structures). Passing the already-parsed errorMessage to formatAuthError ensures that the most descriptive reason is preserved in the final output.
| errorMessage = formatAuthError(error, config.getContentGeneratorConfig()?.authType); | |
| errorMessage = formatAuthError(errorMessage, config.getContentGeneratorConfig()?.authType); |
| const formattedError = formatter.formatError( | ||
| error instanceof Error ? error : new Error(getErrorMessage(error)), | ||
| new Error(errorMessage), | ||
| errorCode, | ||
| config.getSessionId(), | ||
| ); |
There was a problem hiding this comment.
Wrapping the formatted message in a generic new Error(errorMessage) when generating JSON output causes the loss of the specific error type (e.g., FatalTurnLimitedError). This makes the JSON output less useful for programmatic consumption or IDE integrations that rely on the error type to provide specific handling. Consider preserving the original error's type while updating its message.
| const formattedError = formatter.formatError( | |
| error instanceof Error ? error : new Error(getErrorMessage(error)), | |
| new Error(errorMessage), | |
| errorCode, | |
| config.getSessionId(), | |
| ); | |
| const errorToFormat = error instanceof Error ? error : new Error(getErrorMessage(error)); | |
| errorToFormat.message = errorMessage; | |
| const formattedError = formatter.formatError( | |
| errorToFormat, | |
| errorCode, | |
| config.getSessionId(), | |
| ); |
References
- When catching exceptions, log the detailed error for debugging instead of providing only a generic error message.
Addresses Gemini Code Assist review feedback by sanitizing ANSI escape codes from authentication error reasons. Also adds context-aware fix instructions based on the AuthType.
…ation as public comment
7557dab to
813b89d
Compare
This PR fixes an issue where the scheduled triage bot was unconditionally posting its internal reasoning as a public comment on all triaged issues. By introducing a new \comment\ field in the JSON schema, the bot will now only comment when it explicitly needs to ask the user for more information.