review-changes

审查变更

审查功能分支中的代码变更，并在合并前找出问题。

工作流

1. 确定审查目标

• 远程 PR：如果用户提供了 PR 编号或 URL（例如“审查 PR #123”“审查 https://github.com/org/repo/pull/123”）：

  1. 检出 PR：gh pr checkout <PR_NUMBER>
  2. 阅读 PR 上下文：gh pr view <PR_NUMBER> --json title,body,comments
  3. 将 PR 描述和评论作为审查的补充上下文

• 本地变更：如果未指定 PR，则审查当前分支与默认分支的差异（继续执行第 2 步）

1.5 加载仓库指南

搜索仓库专属的编码指南。这些指南优先于内置指南。

查找顺序（优先级由高到低）：

1. CLAUDE.md、.claude/CLAUDE.md
2. CODE_GUIDELINES.md、.github/CODE_GUIDELINES.md、docs/CODE_GUIDELINES.md
3. STYLE_GUIDE.md、.github/STYLE_GUIDE.md、docs/STYLE_GUIDE.md
4. CONTRIBUTING.md、.github/CONTRIBUTING.md、docs/CONTRIBUTING.md

提取与审查相关的内容（编码标准、错误处理、测试、安全、命名规范）。跳过与审查无关的内容（问题模板、行为准则）。用户指南优先于内置指南，并与其共同适用。审查远程 PR 时，从远程仓库加载指南。

2. 检测分支

# Get current branch
git branch --show-current

# Detect default branch (try remote HEAD, fall back to main)
git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@' || echo "main"

3. 获取变更文件

# Assess scope first
git diff --stat <default-branch>...HEAD

# Get full diff
git diff <default-branch>...HEAD

性能限制：

1. 跳过锁文件、.min.js、.min.css、生成的文件和编译输出
2. 如果变更文件超过 30 个，优先审查源代码而非测试/配置，并概述跳过的文件
3. 如果单个文件的变更超过 500 行，则进行概述，而不是逐行审查
4. 如果差异总计超过 3000 行，则挑选约 20 个最重要的文件，并概述其余内容

4. 分析变更

以差异为主要输入。结合整个差异综合应用指南，而不是逐个文件审查。只有在需要上下文来理解变更时，才阅读完整文件。

审查流程：

1. 确定适用的指南：仓库专属指南（第 1.5 步）、通用指南（下文）和语言专属指南（参考文件）
2. 优先查找严重问题——先报告，再扫描较轻微的问题
3. 对于发现的每个问题，记录：违反的指南、文件和行号、问题说明、修复建议
4. 跳过不适用的指南（没有数据库操作 → 跳过“数据库与持久化”）

5. 检查测试覆盖率

对于每个包含业务逻辑的新增或修改文件：

• 检查是否存在对应的测试文件
• 如果存在测试，确认新代码路径已覆盖
• 对关键路径缺少测试的情况进行标记

6. 格式化输出

按严重程度分组呈现发现的问题，顺序为 Critical → Major → Minor。

## Branch Review: `feature/xyz` → `main`

**Repository guidelines loaded:**
- `.github/CONTRIBUTING.md` - coding standards, testing requirements

*(These take precedence over built-in rules where they conflict)*

### 🔴 Critical (X issues)

**1. [Brief title]**
- **File**: `path/to/file.ts:42`
- **Problem**: Clear description of what's wrong
- **Fix**: Specific solution

### 🟠 Major (X issues)

**1. [Brief title]** `[repo]`
- **File**: `path/to/file.ts:87`
- **Problem**: Description
- **Fix**: Solution

### 🟡 Minor (X issues)

**1. [Brief title]**
- **File**: `path/to/file.ts:123`
- **Problem**: Description
- **Fix**: Solution

---
**Summary**: X critical, Y major, Z minor issues found.
[Ready to merge / Needs fixes before merge]

如果某个类别没有发现问题，则省略该部分。结尾应明确给出合并建议。标记为 [repo] 的问题基于仓库专属指南提出。如果未加载仓库指南，则省略“已加载仓库指南”部分。

反馈语气： 保持建设性——解释为什么需要修改。提供可执行的建议。假定对方出于善意。批准时，肯定这项贡献的具体价值。

---

通用指南

无论使用何种语言，这些指南都适用于每次审查。

API 与破坏性变更

• 未提供弃用期就移除或重命名公共函数/方法
• 更改函数签名（新增必填参数、更改返回类型）
• 修改 API 端点的响应结构（移除字段、更改类型）
• 更改默认值，导致现有行为发生变化
• 更改数据库架构但未提供迁移脚本

身份验证与授权（严重）

• 新增端点或路由缺少身份验证检查
• 降低权限要求（仅限管理员 → 公开）
• 硬编码凭据或 API 密钥
• JWT/会话问题：缺少过期时间、密钥强度不足、验证不当
• CORS 配置错误：来源限制过于宽松（生产环境中使用 *）

数据库与持久化

• 多步骤原子操作缺少事务
• N+1 查询模式：在循环中获取相关数据，而不是使用联接或预加载
• 经常查询的列缺少索引
• 查询结果无上限：在大型表上使用不带 LIMIT 的 SELECT *
• SQL 注入：在查询中拼接字符串，而不是使用参数化查询

并发（对避免数据损坏至关重要）

• 访问共享可变状态时未进行同步
• 修改共享资源时缺少锁／互斥锁
• 检查后执行模式不具备原子性（TOCTOU）
• 存在死锁风险：以不一致的顺序获取多个锁

异步代码

• 每次 await 之后，都要确认假设仍然成立——状态可能已经改变
• 标记未验证异步操作是否达到预期结果就返回成功的代码

外部 API 处理

• HTTP 请求缺少超时设置
• 对瞬时故障没有重试逻辑
• 对反复故障的服务缺少熔断机制
• 未区分 4xx 和 5xx 错误
• 未考虑速率限制（遇到 429 时不进行退避）

边界情况与边界值

• 代码假定数组／列表非空
• 对可选值缺少 null／undefined 检查
• 循环中存在差一错误，范围检查不正确
• 类型强制转换导致意外行为
• Unicode／编码问题：假定内容为 ASCII，或字符串长度计算不正确

防御性编码

• 未验证就使用外部输入（API／用户输入）
• 未处理可能失败的操作的失败情况
• 未确认索引有效就访问数组
• 代码依赖未记录的行为或顺序

输入清理（至关重要）

• 命令注入：在 Shell 命令中使用未经清理的输入——使用参数数组，而不是字符串插值
• 路径遍历：用户输入导致文件路径越出预期目录——解析并验证路径
• XSS：将用户输入渲染为 HTML——进行转义或使用安全 API，并验证 URL 协议
• 日志注入：未经清理的输入伪造日志条目——使用结构化日志
• SQL 注入：在查询中拼接字符串——使用参数化查询

内存与性能

• 无界集合（数组／映射不断增长且没有限制）
• 内存泄漏：未移除事件监听器、闭包持有引用
• 在循环中进行可复用的大型内存分配
• 在主线程上执行阻塞式同步 I/O 或 CPU 密集型工作
• 缺少分页：加载整个数据集

文件系统操作

• 读取文件前未确认文件存在
• 写入文件前未确保父目录存在
• 文件操作缺少错误处理

日志与可观测性

• 重要操作的日志记录不足
• 过度记录日志，造成噪声或性能问题
• 跨服务追踪缺少关联 ID
• 严重：记录敏感数据（PII、密码、令牌）
• 新功能缺少可观测性钩子

测试质量

• 测试没有断言任何有意义的内容
• 缺少边界情况覆盖（只测试正常路径）
• 脆弱的测试：存在竞态条件、依赖时间或依赖外部因素
• 测试污染：共享状态、缺少清理
• 模拟过多：测试未覆盖真实代码路径

无障碍访问

• 图片缺少替代文本
• HTML 语义不正确（用 div 代替按钮、表单缺少标签）
• 交互元素无法通过键盘访问
• 仅含图标的按钮缺少 ARIA 标签
• 仅通过颜色传达信息

代码风格

• 优先使用提前返回，而不是嵌套的 if 语句——扁平的代码更易阅读

错误消息

• 错误消息必须提供可操作的信息，并且结合上下文、具体明确
• 包含用于调试的标识符（订单 ID、用户 ID）
• 不要向最终用户暴露堆栈跟踪或内部细节
• 包含用于程序化处理的错误代码

---

特定语言指南

根据差异中的文件扩展名，加载相关参考资料：

• .ts、.js、.mjs、.cjs → 阅读 references/typescript.md
• .tsx、.jsx → 阅读 references/react.md（同时加载 typescript.md）
• .py → 阅读 references/python.md
• .go → 阅读 references/go.md
• .rs → 阅读 references/rust.md
• .java、.kt → 阅读 references/java-kotlin.md

仅加载变更文件中涉及的语言对应的参考资料。

---

严重程度定义

• 严重 — 阻止合并。代码损坏、安全漏洞、数据泄露，或会导致生产环境崩溃的运行时故障。
• 主要 — 应当修复。未处理的异步操作、缺少错误处理、资源泄漏、竞态条件、缺少验证。
• 次要 — 建议修复。代码清晰度、一致性、性能、死代码、重复内容、未解决的 TODO。