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:

   git log origin/<current-branch>..HEAD --oneline
   

   to list unpushed commits.

3. Get diff content

   For unstaged changes:

   git diff
   

   For staged changes:

   git diff --cached
   

   For unpushed commits:

   git diff origin/<current-branch>..HEAD
   

   For all changes:

   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 (中文模板):

   # 代码审查报告
   
   **日期:** 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），追加内容：

   ## 修复提案
   
   **提案名称:** <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)