code-review-web

Web 代码审查

审查和调试 Web 应用代码，重点关注那些确实会导致生产环境故障的模式。跨技术栈的原则见 SKILL.md。特定技术栈的模式见 references。

---

适用场景

• 合并前审查拉取请求
• 调试生产环境问题
• 调查构建失败
• 审计现有代码的安全性或性能
• 调查环境变量或配置问题
• 处理“网站坏了”报告

不适用场景

• 编写新功能规格（使用 pm-spec-writing）
• 针对运行中的网站进行上线前 QA（使用 qa-testing）
• 深入分析 Core Web Vitals 性能（使用 performance-optimization）
• 深入审查无障碍合规性（使用 accessibility-audit）

---

必需输入

• 待审查的代码、PR、错误消息或症状
• 如果正在调试，需能访问日志（构建日志、函数日志、服务端日志）
• 技术栈（框架、托管平台、数据库）——即使只提供大致信息也可以

如果只提供症状（“网站坏了”），工作流的第一步是收集足够的上下文以进行调查。

---

框架：5 个审查维度

每次代码审查都涵盖五个维度。根据具体情况选择审查深度。

1. 正确性

代码是否实现了其声称的功能？

• 逻辑符合规格说明或 PR 描述中的意图
• 已处理边缘情况（空状态、错误状态、网络故障）
• 检查差一错误、null/undefined 处理和异步竞态条件
• 变更有相应测试，或者有不编写测试的合理原因
• 变更不会破坏现有功能（回归风险）

2. 安全性

代码是否暴露了敏感信息或扩大了攻击面？

• 机密信息处理。 客户端代码或版本控制中不得包含机密信息、API 密钥或服务角色凭据
• 身份验证检查。 每个变更端点在执行操作前都要验证调用方
• 输入验证。 使用用户输入构造查询、文件路径或 HTML 前，先对其进行清理
• 外部请求。 验证外发 URL；避免用户可控输入造成 SSRF
• CSRF 防护。 状态变更请求必须要求令牌或遵循同源策略
• 速率限制。 面向公众的变更端点必须设置速率限制
• 仅限 HTTPS。 生产代码路径中不得使用 HTTP
• Cookie。 为会话 Cookie 设置 Secure、HttpOnly 和 SameSite 属性
• 环境变量。 仅供服务端使用的机密信息不得使用任何会将其暴露给客户端包的前缀

3. 性能

代码能否扩展并保持快速？

• 数据库查询。 避免 N+1 模式。优先使用联接或批量获取，而不是在循环中执行查询。
• 分页。 对大型结果集进行分页，绝不一次性全部加载。
• 缓存。 根据数据新鲜度需求选择合适的缓存策略。
• 包体积。 确保客户端依赖合理。确保摇树优化正常工作。
• 图像处理。 使用现代格式、延迟加载并明确指定尺寸。
• 后台工作。 将耗时操作移出请求路径。
• 冷启动敏感性。 如果冷路径经常触发，应对其进行优化。

4. 可靠性

代码出现故障时会怎样？

• 错误处理。 捕获并处理错误，而不是吞掉错误。记录错误时附上上下文。
• 重试。 网络调用应针对暂时性故障设置重试逻辑。
• 超时。 为外部调用明确设置超时时间（避免无限等待）。
• 优雅降级。 非关键路径发生故障时，不应导致页面崩溃。
• 幂等性。 可能重试的变更操作应确保重试安全。
• 日志记录。 日志中应包含足够的上下文，以便在不复现问题的情况下进行诊断。

5. 可维护性

六个月后，下一个接手的人（或未来的你）能看懂这段代码吗？

• 命名。 函数和变量的命名应体现其用途，而不是实现方式。
• 注释。 解释原因，而不是说明内容。代码已经说明了内容。
• 复杂度。 函数只做一件事。如果一个函数有 200 行，那它做的事情就太多了。
• 重复。 多处出现的相同逻辑应提取出来。
• 依赖项。 添加新依赖项要有充分理由。每个依赖项都会增加维护负担。
• 魔法值。 代码中不要出现字面量 60000。请使用有名称的常量。

---

常见的 Bug 模式（与技术栈无关）

这些模式会在不同技术栈中反复出现，每次审查时都值得检查。

构建和部署

• 构建时获取数据导致超时。 在静态生成期间查询数据库的路由可能会在规模扩大时失败。如果数据必须保持最新，请将其标记为运行时渲染。
• 环境变量未传递。 某个变量在本地可用、但在生产环境中出错，通常是因为它未添加到生产环境中。
• 预览环境和生产环境的变量不一致。 部署在预览环境中正常、但在生产域名下出错，通常是因为代码中硬编码了特定技术栈的 URL。

URL 和域名问题

• Canonical 指向暂存或预览 URL。 这是因为客户端可见的环境变量获取了错误的域名。Canonical 域名应从仅限服务端的环境变量中读取。
• API URL 指向会回环的主域名。 DNS 切换后，主域名可能已改为服务另一个应用。API 应使用专用子域名。
• HTTPS 升级问题。 混合内容（在 HTTPS 页面中加载 HTTP 资源）会破坏浏览器的安全模型。

缓存失效

• 部署后内容仍然过时。 可能是缓存未失效，也可能是缓存失效需要手动触发，但触发操作没有运行。
• CDN 在同一文件名下提供旧资源。 替换资源时，请始终使用新文件名或添加缓存清除查询字符串。
• 缓存头设置过于激进。 对频繁变化的资源设置较长的 max-age，会导致用户看到过时内容，持续数小时甚至数天。

数据库和数据

• N+1 查询。 在循环中执行查询。请改用批量获取或联接。
• 全表扫描缺少限制。 查询大型表时忘记添加 LIMIT。
• 连接池耗尽。 并发连接过多，通常是并行路由在构建时获取数据所致。
• 架构迁移未回填数据。 添加 NOT NULL 列，却未为现有行填充数据。

图像处理

• 上传后图像无法加载。 CDN 缓存了之前的文件名。替换图像时请使用新文件名。
• 图像导致布局偏移。 缺少宽度或高度属性。请务必同时指定两者。
• 大型图像导致 LCP 变慢。 主视觉图像未针对大小或格式进行优化。

外部集成

• 机器人防护机制阻止服务端到服务端调用。 CDN 或防火墙正在挑战合法的自动化流量。请将服务端 IP 加入允许列表，或对 API 端点禁用挑战。
• 生产环境触发 API 速率限制。 本地流量很少，因此未暴露问题。请添加退避机制，并考虑速率限制。

安全

• 重新验证或管理端点未受保护。 请务必要求提供密钥令牌。
• URL 中包含个人身份信息（PII）。 这些信息会显示在服务端日志、浏览器历史记录和引荐来源标头中。
• 密钥暴露在客户端资源包中。 任何发送到浏览器的内容都是公开的。

---

工作流

1. 收集上下文。 使用什么技术栈？有什么问题或正在审查什么？是否有日志？
2. 选择深度。 小型 PR 快速浏览即可。重大变更需全面审查。生产事故需深入调查。
3. 逐项检查 5 个维度。 按严重程度记录问题（阻塞、重要、轻微）。
4. 检查特定技术栈的模式。 参考相应的技术栈指南。
5. 处理事故时： 找出最小化、由假设驱动的修复方案。尽可能在本地复现。
6. 撰写审查意见。 正式审查请使用 references/review-template.md 中的模板。

---

常见失误模式

• 对 500 行的 PR 只说“看起来没问题”。 如果审查大型 PR 只花了 5 分钟，那就不算真正审查过。
• 不运行代码就进行审查。 有些 bug 只会在运行时出现。拉取分支并运行代码。
• 过度关注风格。 为格式问题争论不休，却漏掉逻辑 bug。
• 跳过“内部”功能的安全审查。 内部功能变为外部功能的速度往往超出预期。
• 把警告当作装饰。 依赖项更新后，构建警告常常会变成生产环境错误。
• 不读完整错误信息就开始调试。 堆栈跟踪的第一行通常并非真正原因。请完整阅读。

---

调试工作流

收到生产问题报告时：

1. 阅读完整错误信息。 包括堆栈跟踪。
2. 检查托管平台的构建日志和函数日志。 通常可以在这里找到确切的故障行。
3. 确定最后一个正常运行的版本。 使用 git log --oneline 并检查最近的提交。
4. 在本地复现。 确认问题来自代码，而非环境。
5. 检查环境变量。 尤其是在部署或 DNS 变更之后。
6. 检查缓存状态。 在断定是代码 bug 之前，先强制使缓存失效。
7. 进行最小化修复。 事故期间的大规模重构会引发更多事故。
8. 在生产环境中验证。 确认实际修复有效，而不只是部署成功。
9. 记录文档。 根本原因是什么？如何才能避免？记录经验教训。

---

输出格式

PR 审查：在 PR 中直接添加行内评论，并视需要附上摘要。

正式代码审查：在 code-review-[date].md 中创建 Markdown 文档，包含：

1. 范围（审查了什么）
2. 摘要（总体评估）
3. 严重问题（阻塞项）
4. 重要问题
5. 轻微问题
6. 后续建议

事故处理：编写事后复盘文档。格式请参阅 after-action-report。

---

参考文件

• references/review-template.md - 正式代码审查的 Markdown 模板。
• references/nextjs-patterns.md - Next.js 的特定技术栈模式（App Router、ISR、Server Components、常见 bug）。
• references/wordpress-headless-patterns.md - Headless WordPress 集成的特定技术栈模式。