code-review

代码审查

此技能可让 AI Agent 对源文件、一组更改或拉取请求进行结构化、全面的代码审查。Agent 会从多个质量维度检查代码，包括正确性、安全性、性能、可读性和可维护性，并生成详细的审查报告，针对具体代码行提供可执行的反馈。

工作流

1. 解析输入并建立上下文。 确定输入是单个文件、目录还是拉取请求差异。如果是拉取请求，则获取差异并确定基准分支，以便仅审查更改的代码行。读取相关配置文件（如 Linter 配置、风格指南、类型定义），使审查符合项目标准。

2. 理解更改的意图。 阅读提交消息、PR 描述和周边代码，了解作者的意图。这有助于避免误报——审查者必须先了解目标，才能判断代码是否达成目标。继续之前，用一句话概述更改内容。

3. 检查正确性和错误。 逐一检查每个更改过的函数，并追踪数据流。查找对 null 或 undefined 的解引用、差一错误、不正确的布尔逻辑、未处理的错误路径、并发代码中的竞态条件，以及资源泄漏（打开的文件、数据库连接、未释放的锁）。确认边界情况——空输入、最大值、意外类型——均得到处理。

4. 评估安全性。 扫描常见漏洞模式：未经清理的用户输入（SQL 注入、XSS）、硬编码的密钥或凭证、不安全的加密用法、权限过宽的文件或网络访问，以及缺少身份验证或授权检查。标记新增的依赖项，并检查是否存在已知 CVE。

5. 评估性能和可扩展性。 查找算法复杂度问题（大型集合上的嵌套循环、循环中重复查询数据库、内存无界增长）。检查不必要的分配、可采用缓存的场景，以及异步上下文中的阻塞调用。考虑预期的数据量，并判断代码是否能够扩展。

6. 审查可读性和可维护性。 评估命名是否清晰、函数长度、代码重复（违反 DRY 原则）以及是否遵循项目风格指南。检查公共函数是否有文档字符串或类型注解。确认魔法数字已替换为具名常量，复杂逻辑已添加说明性注释。

审查清单

Agent 会依据以下类别评估每项更改：

类别	检查内容
错误	空值解引用、差一错误、逻辑错误、未处理的异常
安全性	注入、XSS、硬编码的密钥、缺少身份验证、不安全的依赖项
性能	O(n²) 循环、N+1 查询、不必要的分配、阻塞式 I/O
可读性	命名不清、函数过长、缺少文档、格式不一致
DRY	复制粘贴的代码块、应提取出来的重复逻辑
错误处理	吞掉异常、缺少重试、错误消息不明确
测试	新逻辑缺少测试、现有测试损坏、边界情况未经测试

用法

提供以下一种或多种输入：

• 文件路径 — 要完整审查的单个源文件。
• 目录路径 — Agent 会审查目录中的所有源文件。
• 拉取请求 URL — 例如，https://github.com/user/repo/pull/42。Agent 会获取差异，并结合上下文仅审查更改的代码行。
• 差异或补丁文本 — 直接粘贴的原始统一差异。
• 关注领域（可选）— 例如，“重点关注安全性”或“仅检查性能”，以缩小审查范围。

示例

示例 1 — 审查 Python 函数

给定文件 src/auth.py：

import hashlib

def authenticate(username, password, db):
    query = f"SELECT password_hash FROM users WHERE username = '{username}'"
    row = db.execute(query).fetchone()
    if row is None:
        return False
    stored_hash = row[0]
    input_hash = hashlib.md5(password.encode()).hexdigest()
    return input_hash == stored_hash

Agent 生成以下审查结果：

## Review: src/auth.py — authenticate()

### Critical — SQL Injection (line 4)
The query uses f-string interpolation with `username` directly in the SQL
string. An attacker can pass `' OR '1'='1` as the username to bypass
authentication entirely.

**Fix:** Use parameterized queries.
- row = db.execute(query).fetchone()
+ row = db.execute(
+     "SELECT password_hash FROM users WHERE username = ?", (username,)
+ ).fetchone()

### Critical — Weak Hashing Algorithm (line 9)
MD5 is cryptographically broken and unsuitable for password hashing. It is
vulnerable to rainbow table attacks and can be brute-forced in seconds on
modern hardware.

**Fix:** Use bcrypt or argon2 with a per-user salt.
- input_hash = hashlib.md5(password.encode()).hexdigest()
+ import bcrypt
+ return bcrypt.checkpw(password.encode(), stored_hash.encode())

### Warning — Timing Attack (line 10)
Direct string comparison of hashes with `==` leaks information through
timing differences. Use a constant-time comparison function.

**Fix:**
- return input_hash == stored_hash
+ import hmac
+ return hmac.compare_digest(input_hash, stored_hash)

### Info — Missing Type Annotations
Adding type hints improves readability and enables static analysis:
  def authenticate(username: str, password: str, db: Connection) -> bool:

**Summary:** 2 critical, 1 warning, 1 info. This function must not be
deployed until the SQL injection and hashing issues are resolved.

示例 2 — 审查拉取请求差异

给定 src/api/orders.js 的 PR 差异：

@@ -12,6 +12,20 @@ const express = require("express");
 const router = express.Router();

+router.get("/orders", async (req, res) => {
+  const userId = req.query.userId;
+  const orders = await db.query("SELECT * FROM orders WHERE user_id = $1", [userId]);
+  const enriched = [];
+  for (const order of orders.rows) {
+    const items = await db.query("SELECT * FROM order_items WHERE order_id = $1", [order.id]);
+    order.items = items.rows;
+    enriched.push(order);
+  }
+  res.json(enriched);
+});

Agent 生成：

## Review: src/api/orders.js — GET /orders

### Warning — N+1 Query (lines 18-21)
Each order triggers a separate SELECT for its items. For a user with 200
orders this executes 201 database queries. This will degrade significantly
under load.

**Fix:** Use a single JOIN or a batched IN query:
  const items = await db.query(
    "SELECT * FROM order_items WHERE order_id = ANY($1)",
    [orders.rows.map(o => o.id)]
  );
  // Then group items by order_id in application code.

### Warning — Missing Authorization Check (line 14)
The endpoint reads `userId` from the query string without verifying that
the authenticated user is allowed to view that user's orders. Any user
can read any other user's order history by changing the query parameter.

**Fix:** Validate that `req.user.id === userId` or that the requester
has an admin role before executing the query.

### Info — SELECT * Usage (lines 16, 19)
Selecting all columns returns data the client may not need (e.g.,
internal timestamps, soft-delete flags). Specify only the required columns
to reduce payload size and avoid leaking internal fields.

**Summary:** 0 critical, 2 warning, 1 info.

最佳实践

• 审查差异，而不只是文件。 重点关注已更改的行及其紧邻上下文。除非已有问题与新更改相互影响，否则不要评论这些问题。
• 明确标注严重程度。 使用 Critical / Warning / Info 等级，让作者知道哪些问题必须在合并前修复，哪些只是建议。
• 提出具体的修复方案，而不是含糊地批评。 不要只说“可以做得更好”，而应提供替换代码片段或具体的重构步骤。
• 控制每轮审查的范围。 如果一个文件有几十个问题，优先指出影响最大的 5-7 个问题。让作者应接不暇会降低问题得到修复的可能性。
• 肯定良好的模式。 如果作者设计了特别简洁的抽象，或妥善处理了边界情况，请指出来。正面反馈有助于强化良好习惯。
• 结合代码检查测试。 如果新逻辑缺少测试，请标出。如果已有测试，请确认它们确实覆盖了更改的行为，而不只是成功路径。

边界情况

• 生成代码或引入的代码： 通常应排除由代码生成器、Protocol Buffer 编译器生成的文件或引入的依赖。除非另有要求，否则 Agent 会跳过符合常见生成代码模式的文件。
• 大型差异（>1000 行）： 非常大的拉取请求难以进行全面审查。Agent 会提醒作者拆分 PR，然后优先检查风险最高的文件。
• 特定语言的惯用写法： 一种语言中的惯用模式（例如 Go 显式返回错误）在另一种语言中可能看起来像代码异味。Agent 会根据检测到的语言调整判断标准。
• 上下文不完整： 在无法访问完整代码库的情况下审查差异时，Agent 可能无法验证类型定义、配置或上游调用方。它会明确说明相关假设。
• 仅涉及风格的更改： 如果 PR 只包含格式调整或重命名，Agent 会确认没有语义差异，并给出简短的批准意见，而不是完整报告。