# Git Code Review

> Review uncommitted or committed changes in the current branch and generate a code review report. Use when the user wants to review their changes before or after committing.

- Skill: `zhaoyis/git-code-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add zhaoyis/git-code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/zhaoyis/git-code-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Docs & Writing
- License: MIT
- Author: ZhaoYis (https://skillmd.com/u/zhaoyis)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/zhaoyis/git-code-review

---


Review uncommitted or committed changes in the current branch and generate a code review report.

**Input**: Optionally specify:
- Review scope: `unstaged`, `staged`, `unpushed`, `all`
- Review focus: `security`, `performance`, `style`, `architecture`, `all`

**IMPORTANT**: This skill MUST read `openspec/project.md` before performing the review to understand project-specific conventions and constraints.

**重要**: 审查报告必须使用中文输出。

**Steps**

1. **Load project conventions**

   **CRITICAL**: First read `openspec/project.md` to get:
   - Technology stack constraints
   - Layered architecture rules
   - Naming conventions
   - Code style requirements
   - Design patterns
   - Important constraints

   Use these conventions as the baseline for the review.

2. **Determine review scope**

   Run `git status` to see all changes.

   Run `git branch --show-current` to get the current branch name.

   Use **AskUserQuestion tool** to ask what to review:

   **Review scope options:**
   - `unstaged` - Only unstaged changes (default if no staged changes)
   - `staged` - Only staged changes (default if staged changes exist)
   - `unpushed` - All commits not yet pushed to remote
   - `all` - All uncommitted changes + unpushed commits

   **If `unpushed` or `all`:**
   ```bash
   git log origin/<current-branch>..HEAD --oneline
   ```
   to list unpushed commits.

3. **Get diff content**

   For **unstaged changes:**
   ```bash
   git diff
   ```

   For **staged changes:**
   ```bash
   git diff --cached
   ```

   For **unpushed commits:**
   ```bash
   git diff origin/<current-branch>..HEAD
   ```

   For **all changes:**
   ```bash
   git diff HEAD
   git log origin/<current-branch>..HEAD --oneline
   ```

   Run `git diff --stat` to get a summary of changes.

4. **Secret scanning**

   Use `git diff` output to scan for potential secrets:
   - API keys (patterns like `api_key`, `apikey`, `API_KEY`)
   - Passwords (`password`, `passwd`, `pwd`)
   - Tokens (`token`, `access_token`, `refresh_token`)
   - Private keys (`-----BEGIN.*PRIVATE KEY-----`)
   - Database URLs with credentials
   - AWS/Azure/GCP credentials

   **If secrets detected:**
   - Add to Critical issues section
   - Suggest using environment variables or secret management
   - Warn about git history (may need `git filter-branch` or BFG)

5. **Perform code review based on openspec/project.md**

   Analyze the changes against the project conventions loaded from `openspec/project.md`:

   **5.1 Technology Stack Compliance:**
   - Java 8 compatibility (no Java 9+ features: `var`, `record`, `sealed`, pattern matching, text blocks)
   - Spring Boot annotations usage
   - MyBatis/MyBatis-Plus usage (`@TableName`)
   - MapStruct for object conversion
   - Lombok annotations (`@Data`, `@Builder`, `@Slf4j`, `@AllArgsConstructor`, `@NoArgsConstructor`)
   - PageHelper for pagination
   - Swagger 2 (`@Api`, `@ApiOperation`, `@ApiModelProperty`) or OpenAPI 3 (`@Schema`)

   **5.2 Layered Architecture Compliance:**
   Check that code follows the strict layering: Web → Biz → Core → Common
   - No reverse dependencies (lower layers depending on upper layers)
   - No cross-layer dependencies (each layer only depends on direct lower layer)

   **5.3 Naming Conventions:**

   | Type | Pattern | Example |
   |------|---------|---------|
   | Controller | `*Controller` | `PurWebContractPaymentBaseController` |
   | BizService | `*BizService` / `*BizServiceImpl` | `ContractPaymentBaseBizService` |
   | DomainService | `*DomainService` / `*DomainServiceImpl` | `ContractPaymentBaseDomainService` |
   | Mapper | `@Repository` | `ContractPaymentBaseMapper` |
   | ManualMapper | `*ManualMapper` | `ContractPaymentBaseManualMapper` |
   | DO | `*DO` | `ContractPaymentBaseDO` |
   | Model | `*Model` | `ContractPaymentBaseModel` |
   | VO | `*VO` | `WebContractPaymentConfirmedVO` |
   | Request | `*Request` | `ContractPaymentBaseAddRequest` |
   | Converter | `*Convert` / `*Converter` | `ContractPaymentBaseConvert` |
   | FacadeClient | `*FacadeClient` / `*FacadeClientImpl` | `IdGeneratorFacadeClient` |
   | Utils | `*Utils` | `AssertUtils` |
   | ConditionDalRequest | `*ConditionDalRequest` | `ContractPaymentBaseConditionDalRequest` |

   **5.4 Annotation Usage:**

   **Controller Layer:**
   - `@RestController`, `@RequestMapping`
   - `@Api` + `@ApiOperation` (Swagger 2) or `@Schema` (OpenAPI 3)
   - `@Slf4j`
   - `@Authority(permissionCode = ...)` or `@NonLoginAuthority`
   - `@Validated` with JSR-303 annotations
   - Return type: `YzwResult<T>`

   **BizService Layer:**
   - Interface: `*BizService`
   - Implementation: `@Service`, `@Slf4j`, `*BizServiceImpl`
   - Injection: `@Resource` (preferred) or `@Autowired`

   **DomainService Layer:**
   - Interface: `*DomainService`
   - Implementation: `@Service`, `@Slf4j`, `*DomainServiceImpl`
   - Write operations: `@Transactional(rollbackFor = Throwable.class)`
   - Injection: `@Autowired` or `@Resource`

   **DO Objects:**
   - Extend `AbstractBaseDO`
   - `@TableName` (MyBatis-Plus)
   - `@Data`, `@ToString(callSuper = true)`, `@EqualsAndHashCode(callSuper = true)`
   - JavaDoc for fields

   **Model Objects:**
   - Extend `AbstractBaseBO`
   - `@Data`
   - JavaDoc for fields

   **Request Objects:**
   - AddRequest extends `CreateInfo`
   - UpdateRequest extends `UpdateInfo`
   - `@Data`, `@Builder`, `@AllArgsConstructor`, `@NoArgsConstructor`

   **5.5 Code Style:**
   - Import order: Java stdlib → Third-party → Project internal
   - No full package name imports (e.g., use `List` not `java.util.List`)
   - Empty lines between class members and methods
   - Use `log.debug()`, `log.info()`, `log.error()` - NO `System.out.println()`

   **5.6 Error Handling:**
   - Use `AssertUtils` for validation
   - Throw `BusinessException` with `BizErrorCode`
   - Private validation methods: `validateAddXxx()`, `validateUpdateXxx()`

   **5.7 Pagination:**
   - Use `PageHelper.startPage(pageNum, pageSize)`
   - Convert to `Page<DO>` type
   - Use `PageConvertUtils.pageResultConvert(PageInfo, List)`
   - Empty result: `PageConvertUtils.getResult(PageInfo)`

   **5.8 Object Conversion:**
   - MUST use MapStruct (`@Mapper` with `INSTANCE` constant)
   - Call pattern: `XxxConvert.INSTANCE.method(...)`
   - NO manual field-by-field conversion

   **5.9 Collection Handling:**
   - Use `CollectionUtils.isEmpty()` / `CollectionUtils.isNotEmpty()`
   - Use `StringUtils` (Apache Commons Lang3) for strings

   **5.10 Transaction Management:**
   - Write operations MUST have `@Transactional(rollbackFor = Throwable.class)`
   - Read operations: no transaction needed

   **5.11 Security:**
   - SQL injection prevention (parameterized queries)
   - XSS prevention
   - Input validation
   - Authentication/Authorization with `@Authority`

   **5.12 Performance:**
   - N+1 query detection
   - Batch query optimization
   - Efficient algorithms

6. **Check historical reviews**

   If review reports exist in `openspec/review/`:
   - List previous review reports
   - Compare with last review (if same branch)
   - Highlight recurring issues
   - Show improvement trend

7. **Save review report to file (使用中文)**

   **CRITICAL: 不需要询问用户，直接保存。**

   a. 用 Shell 创建目录（如不存在）：`mkdir -p openspec/review`
   b. 使用 Write 工具将报告写入 `openspec/review/YYYY-MM-DD-HH-mm-<branch-name>-review.md`
   c. 如果同名文件已存在，使用 `-N` 后缀：`YYYY-MM-DD-HH-mm-<branch-name>-review-N.md`

   **Report structure (中文模板):**
   ```markdown
   # 代码审查报告

   **日期:** YYYY-MM-DD HH:mm
   **分支:** <branch-name>
   **审查人:** Claude Opus 4.6
   **审查范围:** 未暂存 / 已暂存 / 未推送 / 全部
   **规范基准:** openspec/project.md

   ## 概要

   - 变更文件数: X
   - 新增行数: X
   - 删除行数: X
   - 审查提交数: X (如果是未推送)
   - 发现问题数: X (严重: X, 重要: X, 一般: X, 建议: X)

   ## 严重问题 🚨

   <!-- 必须立即修复的严重问题 -->

   1. **[安全] SQL注入风险** - `path/to/file.java:123`
      ```java
      // 问题代码片段
      ```
      **修复建议:** 使用参数化查询

   ## 重要问题 ⚠️

   <!-- 应该修复的重要问题 -->

   ## 一般问题 📝

   <!-- 轻微问题或代码风格改进 -->

   ## 改进建议 💡

   <!-- 可选的改进建议 -->

   ## 规范违规 (openspec/project.md)

   <!-- 违反项目规范的问题 -->

   | 规范类型 | 文件 | 问题描述 |
   |----------|------|----------|
   | 命名规范: Controller | path/to/file.java | 类名应以 'Controller' 结尾 |
   | 架构规范: 分层 | path/to/file.java | Core层不应依赖Biz层 |

   ## 已审查文件

   | 文件 | 变更行数 | 问题数 | 严重程度 |
   |------|----------|--------|----------|
   | path/to/file.java | +50/-10 | 2 | 重要 |

   ## 敏感信息扫描结果

   - 未检测到敏感信息 ✅
   <!-- 或者 -->
   - ⚠️ 在 X 个文件中检测到潜在敏感信息

   ## 与上次审查对比

   <!-- 如果存在之前的审查记录 -->
   - 上次审查日期: YYYY-MM-DD HH:mm
   - 已解决问题: X
   - 新发现问题: X
   - 重复问题: X

   ## 修复建议

   1. 优先修复项
   2. 后续步骤

   ## 亮点肯定 ✨

   <!-- 代码中发现的良好实践 -->
   ```

8. **Output full report to conversation, then prompt for fix proposal (输出报告+询问提案，合并为一条消息)**

   **在同一条消息中完成以下全部内容：**

   a. **输出完整报告内容**到对话（与 Step 7 保存到文件的内容相同）
   b. **在报告末尾注明**：`> 报告已保存到 openspec/review/YYYY-MM-DD-HH-mm-<branch-name>-review.md`
   c. **如果发现严重或重要问题**，在报告输出之后使用 **AskUserQuestion tool** 询问：
      > "审查发现 X 个严重问题和 Y 个重要问题。是否需要生成 OpenSpec 修复提案？"

      **选项：**
      - `生成修复提案` - 使用 `/opsx:propose` 生成修复提案
      - `暂不生成` - 稍后手动处理
   d. **如果仅有一般问题和建议**，不发起 AskQuestion，消息到此结束。

9. **If user chose to generate fix proposal, create proposal and append summary to report (提案生成+摘要回写)**

   **当用户选择"生成修复提案"后：**

   a. 根据问题类型生成提案名称（kebab-case）：
      - 安全问题：`fix-security-<issue-type>`
      - 规范违规：`fix-convention-<issue-type>`
      - 性能问题：`fix-performance-<issue-type>`
      - 混合问题：`fix-<主要问题描述>`

   b. 构建提案描述：
      - **Why**: 说明发现的问题及其影响
      - **What Changes**: 列出需要修复的文件和修改内容
      - 引用审查报告路径

   c. 调用 `/opsx:propose` 生成修复提案

   d. 提案生成完成后，**必须将提案摘要追加写入已保存的报告文件末尾**（Read 报告文件获取当前内容，在末尾追加后重新 Write），追加内容：

   ```markdown
   ## 修复提案

   **提案名称:** <change-name>
   **提案路径:** `openspec/changes/<change-name>/`
   **生成时间:** YYYY-MM-DD HH:mm

   ### 提案包含制品
   - `proposal.md` - 提案文档
   - `design.md` - 设计文档
   - `specs/` - 规格文档
   - `tasks.md` - 实施任务

   ### 后续操作
   运行 `/opsx:apply` 开始实施修复。
   ```

   e. 在对话中告知用户：提案已生成，摘要已追加到报告文件中，运行 `/opsx:apply` 开始实施修复。

   **如果只有一般问题或建议：**
   - 不自动提示生成提案
   - 用户可手动运行 `/opsx:propose` 生成提案

**Output On Success (中文输出)**

```
## 代码审查完成

**分支:** <branch-name>
**审查范围:** <scope>
**规范基准:** openspec/project.md
**报告路径:** openspec/review/YYYY-MM-DD-HH-mm-<branch-name>-review.md

### 问题统计

| 严重程度 | 数量 |
|----------|------|
| 严重 | X |
| 重要 | X |
| 一般 | X |
| 建议 | X |

### 规范违规统计

| 类型 | 数量 |
|------|------|
| 命名规范 | X |
| 架构规范 | X |
| 代码风格 | X |

### 敏感信息扫描
✅ 未检测到敏感信息
<!-- 或者 -->
⚠️ 在 X 个文件中检测到潜在敏感信息 - 详情见报告

### 严重问题（必须在提交前修复）
1. **[安全] SQL注入** - `path/to/file.java:123`
2. ...

### 后续步骤
1. 修复严重问题
2. 修复规范违规
3. 审查重要问题
4. 重新运行代码审查: `/git-code-review`
```

**Output With Fix Proposal (用户同意生成提案后的最终输出)**

```
## 修复提案已生成

**提案名称:** fix-<issue-name>
**提案路径:** openspec/changes/fix-<issue-name>/
**提案摘要已追加到报告文件中:** openspec/review/YYYY-MM-DD-HH-mm-<branch-name>-review.md

### 提案包含制品
- `proposal.md` - 提案文档
- `design.md` - 设计文档
- `specs/` - 规格文档
- `tasks.md` - 实施任务

运行 `/opsx:apply` 开始实施修复。
```

**严重程度说明**

| 严重程度 | 图标 | 判定标准 | 处理建议 |
|----------|------|----------|----------|
| 严重 | 🚨 | 安全漏洞、Bug、破坏性变更、敏感信息泄露 | 必须在提交前修复 |
| 重要 | ⚠️ | 规范违规、性能问题、SOLID原则违反 | 应在合并前修复 |
| 一般 | 📝 | 代码风格、命名、注释 | 建议修复 |
| 建议 | 💡 | 最佳实践、优化建议 | 可选修复 |

**Review Focus Areas**

When user specifies focus areas, prioritize:
- `security` - Focus on security vulnerabilities + secret scanning
- `performance` - Focus on performance issues + N+1 queries
- `style` - Focus on code style and conventions
- `architecture` - Focus on layered architecture and design patterns
- `all` - Full review (default)

**Guardrails**

- ALWAYS read `openspec/project.md` before reviewing
- Always ask for review scope first (Step 2 is the ONLY place to use AskQuestion before the report)
- Always run secret scanning
- **ALWAYS save review report to `openspec/review/` — do NOT ask, just save and also output to conversation**
- **审查报告必须使用中文输出**
- Use severity levels consistently
- Check against project-specific conventions from openspec/project.md
- Provide actionable recommendations with file paths and line numbers
- Include code snippets in issue descriptions
- Include positive highlights to encourage good practices
- Never auto-fix issues without user confirmation
- Compare with previous reviews for trend analysis
- **AskQuestion 使用约束**：整个流程最多使用两次 AskQuestion —— Step 2（选择审查范围）和 Step 8c（仅当存在严重或重要问题时询问是否生成提案）。不要在其他步骤额外询问。
- **Step 7 仅保存文件，Step 8 才输出到对话**：严格分离"保存"和"展示"两个动作，避免重复或遗漏。
- **生成修复提案时：**
  - 仅在发现严重或重要问题时提示生成提案
  - 提案名称使用 kebab-case 格式
  - 提案描述必须包含审查报告路径
  - 生成的提案应包含具体的修复步骤
  - **提案生成后必须将摘要追加到已保存的报告文件末尾**

**Error Handling**

- If `openspec/project.md` not found: Warn user and use default conventions
- If no changes to review: Inform user and exit
- If diff is too large (>5000 lines): Review file by file and summarize
- If review directory creation fails: Show error and suggest manual creation
- If secret scanning fails: Continue review and note the failure

**Convention Checklist (from openspec/project.md)**

| Category | Check |
|----------|-------|
| Java Version | Java 8 compatible |
| Layering | Web → Biz → Core → Common, no reverse deps |
| Naming | Controller/BizService/DomainService/DO/Model/VO/Request |
| Annotations | @RestController/@Service/@Transactional/@Data/@Builder |
| Transaction | @Transactional(rollbackFor = Throwable.class) on writes |
| Pagination | PageHelper.startPage() + PageConvertUtils |
| Conversion | MapStruct with INSTANCE, no manual conversion |
| Error | AssertUtils + BusinessException + BizErrorCode |
| Logging | log.debug/info/error, no System.out |
| Imports | Java stdlib → Third-party → Project internal |
| Collections | CollectionUtils.isEmpty/isNotEmpty |
| Strings | StringUtils (Apache Commons) |