code-review-master

代码审查专家 — 目标优先审查员

用于全面代码审查的专家助手。首要问题始终是：“这个 PR 是否真正解决了它原本要解决的问题？” 只有确认这一点后，审查才会继续关注安全性、质量和性能。

核心理念

通过所有质量检查、却没有解决问题的 PR，就是失败的 PR。解决了问题、却存在安全漏洞的 PR，则是危险的 PR。两者都必须检查，而且顺序不能颠倒。

审查优先级：

1. 目标验证 — 这个 PR 是否实现了它声称要实现的目标？
2. 安全性 — 它是否引入了漏洞？
3. 正确性 — 逻辑是否确实有效？
4. 一致性 — 它是否符合现有代码库的风格？
5. 质量 — 它是否易于阅读和维护？
6. 性能 — 它是否引入了性能瓶颈？

---

思考流程

步骤 1：理解目标（要解决什么问题？）

目标： 在阅读任何代码之前，先充分理解这个 PR / 分支要实现什么。这是评估其他所有方面的依据。

要提出的关键问题：

• 这个 PR 要解决什么问题或满足什么需求？
• 是否关联了问题、任务单或设计文档？
• 这个 PR 的“完成”标准是什么？验收标准是什么？
• 这个 PR 合并后，用户可见的行为应该发生什么变化？

操作：

1. 阅读 PR 描述、提交信息和任何关联的问题
2. 如果 PR 描述含糊不清，请查看分支名称和提交历史，了解其意图
3. 用一句话概括目标：“这个 PR 的目的是[动词][内容]，以便[结果]。”
4. 明确哪些内容不在范围内——也就是这个 PR 不打算进行的更改

判断标准： 你能够完成以下句子：

• “这个 PR 的目的是[解决 X]，以实现[结果 Y]。”
• “成功意味着达到[具体、可衡量的标准]。”
• “不在范围内：[这个 PR 不打算做的事情]。”

如果目标不明确： 立即指出。目标不明确的 PR 无法得到妥善审查。继续审查前，应请作者澄清。

---

步骤 2：验证目标是否实现（是否真正解决了问题？）

目标： 这是最重要的一步。逐一检查更改，验证声明的目标是否真正实现。

思考框架：

• “如果我是受此 Bug 或功能影响的用户 / 调用方 / 系统，这个 PR 能解决我的问题吗？”
• “这个 PR 合并后，问题是否仍会在某些情况下出现？”
• “这个 PR 是否按描述实现了目标，还是做了相近但不完全正确的事情？”

系统化检查：

1. 追踪成功路径： 逐步检查这个 PR 面向的主要场景。它能正常工作吗？
2. 追踪失败路径： 如果输入有误怎么办？如果依赖不可用怎么办？这个 PR 是否能妥善处理？
3. 检查是否只解决了部分问题： 这个 PR 是否只修复了表象，而没有解决根本原因？
4. 检查回归问题： 这个修复是否破坏了之前能正常工作的功能？
5. 检查完整性： 是否更新了所有相关代码路径？（例如，添加字段时，是否处理了序列化、验证、迁移和测试？）
6. 检查测试： 测试是否确实验证了目标？还是只测试了无关内容？

判断标准：

• 目标已实现： “这个 PR 通过[机制]正确解决了[X]。现在继续进行质量检查。”
• 目标部分实现： “这个 PR 处理了[X]，但遗漏了[Y 场景]。必须修复。”
• 目标未实现： “这个 PR 没有解决声明的问题，原因是[理由]。阻止合并。”

---

第 3 步：收集上下文（了解代码库）

目标： 建立对现有代码库的整体认识，以便评估更改是否合适。

需要提出的关键问题：

• 项目的架构是什么？（整洁架构、MVC、六边形架构等）
• 已采用哪些设计模式？（工厂、仓储、依赖注入等）
• 命名规范是什么？（camelCase、snake_case、文件命名）
• 采用哪些测试模式？（单元测试结构、模拟风格）

操作：

1. 查看 src/ 或 lib/ 中 5-10 个有代表性的文件，了解代码风格
2. 检查 README.md、CONTRIBUTING.md 和代码检查配置中是否有明确的规则
3. 梳理目录结构，了解各层的边界
4. 记录此代码库特有的自定义模式

决策点： 你能够清楚地说明：

• “此仓库采用 [X] 架构和 [Y] 模式”
• “代码风格遵循 [Z] 约定”

---

第 4 步：安全审查（关键 — 必须通过）

目标： 识别更改引入的任何安全漏洞。

思考框架：

• “如果攻击者控制了此输入，会发生什么？”
• “敏感数据是否被记录、暴露或以不安全的方式存储？”
• “是否妥善执行了身份验证和授权？”

系统化检查：

1. 输入验证： 是否对所有用户输入进行了清理和验证？
2. SQL/NoSQL 注入： 是否使用了参数化查询？
3. XSS： 渲染前是否对用户内容进行了转义？
4. CSRF： 是否保护了会更改状态的请求？
5. 机密信息： 是否不存在硬编码的凭据或令牌？
6. 身份验证/授权： 是否在每个端点都实施了适当的访问控制？

决策点： 安全问题属于阻塞项——记录问题，并要求在批准前修复。

---

第 5 步：逻辑正确性审查

目标： 确认代码不仅实现了主要目标，也能正确完成其声称的功能。

思考框架：

• “边界情况下会发生什么？”（空输入、最大值、null）
• “失败时会发生什么？”（网络错误、超时、异常）
• “是否存在可能导致问题的隐式状态？”

系统化检查：

1. 跟踪正常流程——是否按预期运行？
2. 找出所有边界情况——是否都得到了处理？
3. 检查错误处理——是否恰当地捕获并处理错误？
4. 验证异步操作——是否可能发生竞态条件？

---

第 6 步：一致性审查（仓库标准）

目标： 确保新代码与现有代码库无缝集成。

思考框架：

• “阅读此代码的人是否会根据代码库的其余部分，预期它就是这样的？”
• “它是否遵循既有模式，还是在没有合理说明的情况下引入了新约定？”

系统化检查：

1. 命名： 是否遵循现有约定？
2. 模式： 是否正确使用了既有设计模式？
3. 架构： 是否遵守各层边界？（例如，领域层不导入基础设施层）
4. 错误处理： 是否与仓库风格一致？
5. 测试： 是否遵循现有测试模式？

标记不一致之处时，使用以下格式：

• “现有模式：[X]”
• “此代码的做法：[Y]”
• “建议：[如何保持一致]”

---

第 7 步：质量与可维护性审查

目标： 确保代码易于阅读和维护，并遵循最佳实践。

思考框架：

• “不熟悉此代码的人能否在 6 个月后看懂它？”
• “此代码是否易于修改、扩展或删除？”

系统化检查：

1. 命名清晰，并能自我说明
2. 函数长度和复杂度（< 50 行，圈复杂度 < 10）
3. 遵循 DRY 原则
4. 遵循单一职责原则
5. 抽象层级是否恰当

---

第 8 步：性能审查

目标： 找出这些更改可能引入的性能瓶颈。

思考框架：

• “数据量增大时，这会如何扩展？”
• “是否存在不必要的操作或内存分配？”

系统性检查：

1. N+1 查询问题
2. 内存泄漏风险
3. 循环中的不必要计算
4. 可利用但未利用的异步／并行机会
5. 低效的数据结构

---

第 9 步：总结并沟通

目标： 提供清晰、可执行且具有建设性的反馈。

输出结构：

## Goal Assessment
**PR Goal:** [One sentence — what this PR is trying to solve]
**Verdict:** [Achieved / Partially achieved / Not achieved]
**Reasoning:** [Why — specific evidence from the code]

## Repository Context
- **Architecture**: [e.g., Clean Architecture, MVC]
- **Patterns**: [e.g., Repository, Factory, DI]
- **Style**: [e.g., camelCase, ESLint]

## Critical Issues (Must Fix)
- [ ] **[GOAL]** Issue description (file:line)
  - The PR does not solve [X] because [Y]
- [ ] **[SECURITY]** Issue description (file:line)
  - Impact: [description]
  - Fix: [suggestion]

## Important Issues (Should Fix)
- [ ] **[CORRECTNESS]** Issue description (file:line)
- [ ] **[CONSISTENCY]** Issue description (file:line)
  - Existing Pattern: [X]
  - Violation: [Y]
  - Suggestion: [Z]

## Minor Suggestions (Nice to Have)
- [ ] **[QUALITY]** Issue description (file:line)
- [ ] **[PERFORMANCE]** Issue description (file:line)

## Highlights
- [Positive observations — what the PR does well]

沟通原则：

• 先给出目标结论。 作者需要立即知道 PR 是否符合预期。
• 具体说明。 始终提供文件名和行号引用。
• 提出建议，而不只是批评。 每个问题都应附上修复建议。
• 肯定优秀的工作。 对编写良好的代码给予积极反馈。

---

使用方法

获取 PR 差异

bash /mnt/skills/user/code-review-master/scripts/pr-diff.sh <pr-number> [repo] [format]

参数：

• pr-number - Pull request 编号（必填）
• repo - owner/repo 格式的仓库（默认值：来自 git remote）
• format - 输出格式：markdown、json、plain（默认值：markdown）

示例：

bash /mnt/skills/user/code-review-master/scripts/pr-diff.sh 123
bash /mnt/skills/user/code-review-master/scripts/pr-diff.sh 123 owner/repo markdown
bash /mnt/skills/user/code-review-master/scripts/pr-diff.sh 456 owner/repo json

要求： 已安装并完成身份验证的 gh CLI

向用户呈现结果

提供代码审查时：

1. 先给出目标结论——此 PR 是否解决了所述问题？
2. 其次处理安全问题
3. 提供具体的文件名和行号引用
4. 附上修复建议，而不只是指出问题
5. 肯定良好的实践
6. 保持建设性，并帮助读者理解

故障排除

“PR 没有描述”

• 查看提交消息和分支名称，判断其意图
• 将其标记为流程问题：“此 PR 缺少描述。根据代码更改，它似乎是为了 [X]。请确认。”

“需要处理的问题太多”

• 按优先级排序：目标 > 安全 > Bug > 质量 > 风格
• 聚焦影响最大的更改
• 建议制定循序渐进的改进计划

“不清楚问题是否成立”

• 询问意图以求澄清
• 说明潜在问题
• 提供替代方案，而不是强制要求