mirror of
https://github.com/catlog22/Claude-Code-Workflow.git
synced 2026-02-14 02:42:04 +08:00
feat: Add orchestrator and state management for code review process
- Implemented orchestrator logic to manage code review phases, including state reading, action selection, and execution loop. - Defined state schema for review process, including metadata, context, findings, and execution tracking. - Created action catalog detailing actions for context collection, quick scan, deep review, report generation, and completion. - Established error recovery strategies and termination conditions for robust review handling. - Developed issue classification and quality standards documentation to guide review severity and categorization. - Introduced review dimensions with detailed checklists for correctness, security, performance, readability, testing, and architecture. - Added templates for issue reporting and review reports to standardize output and improve clarity.
This commit is contained in:
337
.claude/skills/review-code/specs/review-dimensions.md
Normal file
337
.claude/skills/review-code/specs/review-dimensions.md
Normal file
@@ -0,0 +1,337 @@
|
||||
# Review Dimensions
|
||||
|
||||
代码审查维度定义和检查点规范。
|
||||
|
||||
## When to Use
|
||||
|
||||
| Phase | Usage | Section |
|
||||
|-------|-------|---------|
|
||||
| action-deep-review | 获取维度检查规则 | All |
|
||||
| action-generate-report | 维度名称映射 | Dimension Names |
|
||||
|
||||
---
|
||||
|
||||
## Dimension Overview
|
||||
|
||||
| Dimension | Weight | Focus | Key Indicators |
|
||||
|-----------|--------|-------|----------------|
|
||||
| **Correctness** | 25% | 功能正确性 | 边界条件、错误处理、类型安全 |
|
||||
| **Security** | 25% | 安全风险 | 注入攻击、敏感数据、权限 |
|
||||
| **Performance** | 15% | 执行效率 | 算法复杂度、资源使用 |
|
||||
| **Readability** | 15% | 可维护性 | 命名、结构、注释 |
|
||||
| **Testing** | 10% | 测试质量 | 覆盖率、边界测试 |
|
||||
| **Architecture** | 10% | 架构一致性 | 分层、依赖、模式 |
|
||||
|
||||
---
|
||||
|
||||
## 1. Correctness (正确性)
|
||||
|
||||
### 检查清单
|
||||
|
||||
- [ ] **边界条件处理**
|
||||
- 空数组/空字符串
|
||||
- Null/Undefined
|
||||
- 数值边界 (0, 负数, MAX_INT)
|
||||
- 集合边界 (首元素, 末元素)
|
||||
|
||||
- [ ] **错误处理**
|
||||
- Try-catch 覆盖
|
||||
- 错误不被静默吞掉
|
||||
- 错误信息有意义
|
||||
- 资源正确释放
|
||||
|
||||
- [ ] **类型安全**
|
||||
- 类型转换正确
|
||||
- 避免隐式转换
|
||||
- TypeScript strict mode
|
||||
|
||||
- [ ] **逻辑完整性**
|
||||
- If-else 分支完整
|
||||
- Switch 有 default
|
||||
- 循环终止条件正确
|
||||
|
||||
### 常见问题模式
|
||||
|
||||
```javascript
|
||||
// ❌ 问题: 未检查 null
|
||||
function getName(user) {
|
||||
return user.name.toUpperCase(); // user 可能为 null
|
||||
}
|
||||
|
||||
// ✅ 修复
|
||||
function getName(user) {
|
||||
return user?.name?.toUpperCase() ?? 'Unknown';
|
||||
}
|
||||
|
||||
// ❌ 问题: 空 catch 块
|
||||
try {
|
||||
await fetchData();
|
||||
} catch (e) {} // 错误被静默吞掉
|
||||
|
||||
// ✅ 修复
|
||||
try {
|
||||
await fetchData();
|
||||
} catch (e) {
|
||||
console.error('Failed to fetch data:', e);
|
||||
throw e;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. Security (安全性)
|
||||
|
||||
### 检查清单
|
||||
|
||||
- [ ] **注入防护**
|
||||
- SQL 注入 (使用参数化查询)
|
||||
- XSS (避免 innerHTML)
|
||||
- 命令注入 (避免 exec)
|
||||
- 路径遍历
|
||||
|
||||
- [ ] **认证授权**
|
||||
- 权限检查完整
|
||||
- Token 验证
|
||||
- Session 管理
|
||||
|
||||
- [ ] **敏感数据**
|
||||
- 无硬编码密钥
|
||||
- 日志不含敏感信息
|
||||
- 传输加密
|
||||
|
||||
- [ ] **依赖安全**
|
||||
- 无已知漏洞依赖
|
||||
- 版本锁定
|
||||
|
||||
### 常见问题模式
|
||||
|
||||
```javascript
|
||||
// ❌ 问题: SQL 注入风险
|
||||
const query = `SELECT * FROM users WHERE id = ${userId}`;
|
||||
|
||||
// ✅ 修复: 参数化查询
|
||||
const query = `SELECT * FROM users WHERE id = ?`;
|
||||
db.query(query, [userId]);
|
||||
|
||||
// ❌ 问题: XSS 风险
|
||||
element.innerHTML = userInput;
|
||||
|
||||
// ✅ 修复
|
||||
element.textContent = userInput;
|
||||
|
||||
// ❌ 问题: 硬编码密钥
|
||||
const apiKey = 'sk-xxxxxxxxxxxx';
|
||||
|
||||
// ✅ 修复
|
||||
const apiKey = process.env.API_KEY;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Performance (性能)
|
||||
|
||||
### 检查清单
|
||||
|
||||
- [ ] **算法复杂度**
|
||||
- 避免 O(n²) 在大数据集
|
||||
- 使用合适的数据结构
|
||||
- 避免不必要的循环
|
||||
|
||||
- [ ] **I/O 效率**
|
||||
- 批量操作 vs 循环单条
|
||||
- 避免 N+1 查询
|
||||
- 适当使用缓存
|
||||
|
||||
- [ ] **资源使用**
|
||||
- 内存泄漏
|
||||
- 连接池使用
|
||||
- 大文件流式处理
|
||||
|
||||
- [ ] **异步处理**
|
||||
- 并行 vs 串行
|
||||
- Promise.all 使用
|
||||
- 避免阻塞
|
||||
|
||||
### 常见问题模式
|
||||
|
||||
```javascript
|
||||
// ❌ 问题: N+1 查询
|
||||
for (const user of users) {
|
||||
const posts = await db.query('SELECT * FROM posts WHERE user_id = ?', [user.id]);
|
||||
}
|
||||
|
||||
// ✅ 修复: 批量查询
|
||||
const userIds = users.map(u => u.id);
|
||||
const posts = await db.query('SELECT * FROM posts WHERE user_id IN (?)', [userIds]);
|
||||
|
||||
// ❌ 问题: 串行执行可并行操作
|
||||
const a = await fetchA();
|
||||
const b = await fetchB();
|
||||
const c = await fetchC();
|
||||
|
||||
// ✅ 修复: 并行执行
|
||||
const [a, b, c] = await Promise.all([fetchA(), fetchB(), fetchC()]);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 4. Readability (可读性)
|
||||
|
||||
### 检查清单
|
||||
|
||||
- [ ] **命名规范**
|
||||
- 变量名见名知意
|
||||
- 函数名表达动作
|
||||
- 常量使用 UPPER_CASE
|
||||
- 避免缩写和单字母
|
||||
|
||||
- [ ] **函数设计**
|
||||
- 单一职责
|
||||
- 长度 < 50 行
|
||||
- 参数 < 5 个
|
||||
- 嵌套 < 4 层
|
||||
|
||||
- [ ] **代码组织**
|
||||
- 逻辑分组
|
||||
- 空行分隔
|
||||
- Import 顺序
|
||||
|
||||
- [ ] **注释质量**
|
||||
- 解释 WHY 而非 WHAT
|
||||
- 及时更新
|
||||
- 无冗余注释
|
||||
|
||||
### 常见问题模式
|
||||
|
||||
```javascript
|
||||
// ❌ 问题: 命名不清晰
|
||||
const d = new Date();
|
||||
const a = users.filter(x => x.s === 'active');
|
||||
|
||||
// ✅ 修复
|
||||
const currentDate = new Date();
|
||||
const activeUsers = users.filter(user => user.status === 'active');
|
||||
|
||||
// ❌ 问题: 函数过长、职责过多
|
||||
function processOrder(order) {
|
||||
// ... 200 行代码,包含验证、计算、保存、通知
|
||||
}
|
||||
|
||||
// ✅ 修复: 拆分职责
|
||||
function validateOrder(order) { /* ... */ }
|
||||
function calculateTotal(order) { /* ... */ }
|
||||
function saveOrder(order) { /* ... */ }
|
||||
function notifyCustomer(order) { /* ... */ }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 5. Testing (测试)
|
||||
|
||||
### 检查清单
|
||||
|
||||
- [ ] **测试覆盖**
|
||||
- 核心逻辑有测试
|
||||
- 边界条件有测试
|
||||
- 错误路径有测试
|
||||
|
||||
- [ ] **测试质量**
|
||||
- 测试独立
|
||||
- 断言明确
|
||||
- Mock 适度
|
||||
|
||||
- [ ] **测试可维护性**
|
||||
- 命名清晰
|
||||
- 结构统一
|
||||
- 避免重复
|
||||
|
||||
### 常见问题模式
|
||||
|
||||
```javascript
|
||||
// ❌ 问题: 测试不独立
|
||||
let counter = 0;
|
||||
test('increment', () => {
|
||||
counter++; // 依赖外部状态
|
||||
expect(counter).toBe(1);
|
||||
});
|
||||
|
||||
// ✅ 修复: 每个测试独立
|
||||
test('increment', () => {
|
||||
const counter = new Counter();
|
||||
counter.increment();
|
||||
expect(counter.value).toBe(1);
|
||||
});
|
||||
|
||||
// ❌ 问题: 缺少边界测试
|
||||
test('divide', () => {
|
||||
expect(divide(10, 2)).toBe(5);
|
||||
});
|
||||
|
||||
// ✅ 修复: 包含边界情况
|
||||
test('divide by zero throws', () => {
|
||||
expect(() => divide(10, 0)).toThrow();
|
||||
});
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 6. Architecture (架构)
|
||||
|
||||
### 检查清单
|
||||
|
||||
- [ ] **分层结构**
|
||||
- 层次清晰
|
||||
- 依赖方向正确
|
||||
- 无循环依赖
|
||||
|
||||
- [ ] **模块化**
|
||||
- 高内聚低耦合
|
||||
- 接口定义清晰
|
||||
- 职责单一
|
||||
|
||||
- [ ] **设计模式**
|
||||
- 使用合适的模式
|
||||
- 避免过度设计
|
||||
- 遵循项目既有模式
|
||||
|
||||
### 常见问题模式
|
||||
|
||||
```javascript
|
||||
// ❌ 问题: 层次混乱 (Controller 直接操作数据库)
|
||||
class UserController {
|
||||
async getUser(req, res) {
|
||||
const user = await db.query('SELECT * FROM users WHERE id = ?', [req.params.id]);
|
||||
res.json(user);
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ 修复: 分层清晰
|
||||
class UserController {
|
||||
constructor(private userService: UserService) {}
|
||||
|
||||
async getUser(req, res) {
|
||||
const user = await this.userService.findById(req.params.id);
|
||||
res.json(user);
|
||||
}
|
||||
}
|
||||
|
||||
// ❌ 问题: 循环依赖
|
||||
// moduleA.ts
|
||||
import { funcB } from './moduleB';
|
||||
// moduleB.ts
|
||||
import { funcA } from './moduleA';
|
||||
|
||||
// ✅ 修复: 提取共享模块或使用依赖注入
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Severity Mapping
|
||||
|
||||
| Severity | Criteria |
|
||||
|----------|----------|
|
||||
| **Critical** | 安全漏洞、数据损坏风险、崩溃风险 |
|
||||
| **High** | 功能缺陷、性能严重问题、重要边界未处理 |
|
||||
| **Medium** | 代码质量问题、可维护性问题 |
|
||||
| **Low** | 风格问题、优化建议 |
|
||||
| **Info** | 信息性建议、学习机会 |
|
||||
Reference in New Issue
Block a user