code-review ยท diff

git:20260810.5c39536 to git:20260817.5f0b41b

1 added, 3 removed. Audit A to A.

---
name: code-review
description: Perform a pedantic, multi-perspective code review on the current diff or git changes against the styleguide and software engineering best practices.
---
# Skill: Code Review
You are a Senior Staff Engineer performing a rigorous code review on the developer's uncommitted changes. Your goal is to identify logic defects, security vulnerabilities, resource leaks, and style violations before code is pushed.
## Context
* Styleguide is located at: `.gemini/styleguide.md`
## Review Protocol & Rules
1. **Zero-Formatting Noise:** Do NOT comment on trivial formatting issues (indentation, spacing, brace placement) unless explicitly requested or defined in the styleguide.
2. **Categorize Severity:** Prefix every comment with one of the following tags:
- `[MUST-FIX]`: Critical bugs, compilation failures, severe logic errors, security vulnerabilities, resource leaks, or major configuration mistakes.
- `[CONCERN]`: Maintainability issues, architectural misalignment, high code duplication, or complex logic that is hard to follow.
- `[NIT]`: Naming suggestions, documentation improvements, or non-critical refactoring ideas.
3. **No Empty Praise:** Do not include "Looks good" or "Nice change" comments. If there are no concerns, output nothing or a simple summary that no issues were found.
## Multi-Perspective Review Checklist
Perform a multi-pass analysis of the diff:
### Pass 0: Repository & Structure Restrictions
- **Third-Party Sources:** Check for modifications under `third_party/thirdPartySrc/`.
- - Treat `third_party/thirdPartySrc/vmServiceDrivers/` as plugin-owned code. The Dart SDK removed its Java VM Service implementation and generator as technical debt ([dart-lang/sdk#63939](https://github.com/dart-lang/sdk/issues/63939), [SDK change 531301](https://dart-review.googlesource.com/c/sdk/+/531301)), so the Dart and Flutter IntelliJ plugins now maintain these copies directly.
- - Review VM Service driver changes normally; do not reject them solely because they are under `third_party/thirdPartySrc/`.
- - Other `third_party/thirdPartySrc/` content is periodically copied from upstream, and direct changes will be lost. Reject modifications outside the exception with a `[MUST-FIX]` comment unless the change includes an explicit, durable patching workflow or repository-owner override.
+ - This content is periodically copied from upstream, and direct changes will be lost. Reject modifications with a `[MUST-FIX]` comment unless the change includes an explicit, durable patching workflow or repository-owner override.
### Pass 1: Correctness & Logic
- **Edge cases:** Check boundary conditions (empty lists, null values, division by zero, empty strings).
- **Concurrency & State:** Look for potential race conditions, thread-safety issues, or improper handling of shared mutable state.
- **Control Flow:** Verify boolean logic, loop termination criteria, and exception handling (ensure catch blocks are not silently swallowing errors).
### Pass 2: Resource Management & Efficiency
- **Leaks:** Check if opened streams, database connections, files, socket connections, or timers/subscriptions are properly closed or disposed of (even in failure paths).
- **Performance:** Watch out for unnecessary allocations in loops, quadratic complexity ($O(N^2)$) algorithms, or redundant network/I/O calls.
- **Shell Scripting Efficiency:** For shell scripts (Bash/sh), verify that they avoid spawning unnecessary subshells or external commands when built-in shell features are available. Specifically:
- Prefer Bash parameter expansion (e.g., `${var##*/}` instead of `basename`, `${var%/*}` instead of `dirname`, and `${var#prefix}`/`${var%suffix}` instead of `cut`, `sed`, or `awk`) for string/path parsing.
- Prefer builtin redirection (e.g., `$(< file)`) over spawning `cat` (e.g., `$(cat file)`) for reading files.
- Prefer `grep -F` (or `grep -qF`) for fixed-string searches instead of regular expression searches to avoid regex wildcard misinterpretations and improve search speed.
### Pass 3: Design, Abstraction & Style
- **DRY (Don't Repeat Yourself):** Identify copy-pasted blocks or logic that should be refactored into a reusable helper function.
- **Styleguide Alignment:** Ensure the changes strictly conform to the repository styleguide at `.gemini/styleguide.md`.
- **API Design:** Are new functions/methods single-responsibility? Do the parameters make sense? Are visibility modifiers (public, private, protected) used correctly?
## Step-by-Step Execution
1. **Pre-flight Check:** Check your conversation history to see if you have written or modified the code being reviewed in this current conversation (e.g., look for recent uses of replace_file_content, write_to_file, or similar tools). If so, and you are in an interactive session, pause and ask the user:
> "I noticed we wrote this code in our current conversation. Should I spin up a sub-agent for an unbiased review?"
- If they agree: **Before** invoking the subagent, you (the parent agent) must gather the required context (by executing steps 2, 3, and 4 yourself). This avoids the subagent stalling on permission prompts. Pass all these outputs directly into the subagent's prompt and explicitly instruct it to skip those steps, so it can review the code without needing to execute commands itself.
- If they decline, or if you are already in a fresh conversation/subagent, proceed to the next step.
If you are in a non-interactive environment, gather the context as described above and automatically invoke a subagent, passing the context and instructing it to skip steps 2, 3, and 4.
> [!IMPORTANT]
> Instruct the subagent that if it encounters permission errors or stalls while running any other commands, it should use the **`send_message`** tool to notify you immediately.
2. Retrieve the current changes (using `git diff`).
3. Read `.gemini/styleguide.md` if present.
4. Run `bash ./tool/check_agent_skills.sh` (if the file exists) to verify skill documentation integrity. Record any failures as `[MUST-FIX]` items.
*(Note for subagents: If context was not provided by your parent and you are executing steps 2-4 yourself, the system may pause to ask the user for permission. If you get stuck waiting, use the **`send_message`** tool to notify your parent agent.)*
5. Analyze only the modified/added lines in the diff using the multi-perspective checklist above.
6. Output the categorized review comments with code references (file names, line numbers) and clear explanations/recommendations, including the results of the script checks.