Instruction file imported from Sramler/tiny-platform (
.cursor/rules/55-code-review.mdc). Copyright stays with the author.
55 代码审查规范
适用范围
- 适用于：Pull Request、代码审查、代码评审相关流程
- 不适用于：紧急安全修复（但应事后补审并补齐记录）
总体策略
- 质量优先：代码审查是质量门禁的一部分，不是可有可无的礼节动作。
- 风险优先：优先审查安全、权限、多租户、数据库迁移、接口兼容性、关键用户路径。
- 自动化先行：能由 CI、静态检查、测试证明的问题，不应退化为纯主观争论。
禁止（Must Not）
1) 审查态度
- ❌ 审查时使用攻击性语言、人身攻击或情绪化表达。
- ❌ 只提“有问题”“需要优化”这类不可执行意见，不说明原因、影响和建议。
- ❌ 把可交给 formatter / lint / CI 的问题当作主要审查内容，忽略真实风险。
2) 审查流程
- ❌ 未经审查直接合并代码（紧急修复除外）。
- ❌ 审查者与提交者为同一人且无额外复核。
- ❌ 在必需检查失败、缺失、被跳过或明显不可信时批准合并。
3) 审查内容
- ❌ 忽略安全、权限、多租户隔离、数据库迁移、接口兼容性、配置风险等高风险项。
- ❌ 只关注功能“看起来能跑”，不检查失败路径、边界条件、回滚影响和测试质量。
- ❌ 前端变更只看截图，不验证交互、权限态、加载态、错误态和关键流程回归。
必须（Must）
1) 审查检查清单
- ✅ 功能正确性：实现是否符合需求，核心流程、边界条件、异常路径是否合理。
- ✅ 安全性：是否存在 SQL 注入、XSS、敏感信息泄漏、权限绕过、未授权访问等问题。
- ✅ 多租户隔离：业务查询、唯一性约束、缓存键、事件数据是否正确带入
tenant_id语义。 - ✅ 数据与迁移：数据库变更是否包含 migration、兼容性策略、回滚或 expand-contract 方案。
- ✅ 测试覆盖：测试类型是否与风险匹配，关键逻辑是否有自动化回归。
- ✅ 前端行为：若影响前端，是否验证权限控制、表单校验、加载/错误态、关键交互。
- ✅ 配置与发布：是否涉及环境变量、开关、密钥、部署步骤、回滚路径变更。
2) 审查反馈
- ✅ 审查意见必须清晰、具体、可操作，至少说明：问题位置、问题本身、影响、建议动作。
- ✅ 审查意见应标记优先级：必须修改、建议修改、可选优化。
- ✅ 审查者应在 24 小时内给出首次有效反馈；阻塞项必须明确写出阻塞原因。
3) PR 入口要求
- ✅ PR 描述必须包含：变更内容、变更原因、影响范围、验证方式、风险点、回滚思路。
- ✅ PR 描述必须写明自动化验证命令或 CI job，不能只写“已测试”。
- ✅ 涉及数据库、认证、权限、多租户、配置、发布行为的改动，PR 必须显式标注对应风险。
- ✅ 涉及前端页面或交互的改动，PR 必须提供截图、录屏、预览链接或明确的手动验证路径之一。
4) 合并条件
- ✅ 至少一名合适的审查者通过后才能合并；高风险模块应由对应 owner 或熟悉域模型的人审查。
- ✅ 必需 CI 检查全部通过后才能合并；紧急例外必须保留审批和补偿记录。
- ✅ 审查意见、修复记录和最终决策必须保留在 PR 中，便于审计和追溯。
- ✅ 如存在任何 CI / PR / 测试门禁豁免，reviewer 必须核对豁免项、原因、补跑计划和批准依据是否具体且可信；不允许只看见标签就直接放行。
应该（Should）
1) 审查重点
- ⚠️ 优先从行为变化、兼容性、可维护性、性能影响、数据一致性角度审查，而不是先看风格。
- ⚠️ 重点检查“看不见但代价高”的问题：越权、串租户、迁移破坏、空值处理、时序问题、缓存污染。
- ⚠️ 对 bug 修复应优先确认是否补了回归测试，而不是只确认当前现象消失。
2) 审查工具
- ⚠️ 使用 CI、测试报告、覆盖率报告、预览环境、schema diff、OpenAPI diff 等辅助审查。
- ⚠️ 大变更建议拆分为可审查的提交或 PR，避免一次性提交过大导致审查失效。
- ⚠️ 高风险改动建议在 PR 中附带检查清单，便于 reviewer 逐项核对。
3) 审查沟通
- ⚠️ 审查意见应说明理由和预期影响，减少“风格偏好型”争论。
- ⚠️ 复杂争议应在线下或异步讨论达成结论，再把结论沉淀回 PR。
- ⚠️ 如果 reviewer 基于假设提出意见，应明确写出假设，避免误判。
可以（May）
- 💡 使用代码审查模板或 PR 模板，确保风险项不遗漏。
- 💡 为高风险模块使用 CODEOWNERS、双审或领域 reviewer 机制。
- 💡 对较大重构提供“按提交查看”建议，降低 reviewer 负担。
例外与裁决
紧急修复
- 紧急安全修复可先合并后补审，但必须保留审批记录，并在事后补齐测试、审查和风险说明。
小改动
- 文档、注释、纯文案、纯样式微调可简化审查深度，但仍需至少一人确认且不得绕过基本 CI。
冲突裁决
- 平台特定规则（90+）优先于本规范。
- CI/CD 合并门禁以
58-cicd.rules.md为准。 - 测试分层与自动化回归要求以
50-testing.rules.md为准。
示例
✅ 正例：PR 描述
## 变更内容
- 修复字典项在跨租户场景下查询串数据的问题
- 新增后端回归测试与前端请求适配测试
- 补充数据库 migration，收紧唯一性约束
## 变更原因
现有实现未在特定查询链路中带入 tenant_id，存在越权读取风险
## 影响范围
- 后端：dict 查询链路
- 数据库：新增 tenant 维度唯一性约束
- 前端：字典查询请求参数
## 风险与回滚
- 风险：历史脏数据可能导致唯一性约束落库失败
- 回滚：先回滚应用，再回滚 migration 版本；保留兼容脚本清理脏数据
## 验证方式
- `mvn -pl tiny-oauth-server test -Dtest=DictControllerTest`
- `npm run test -- dict.test.ts`
- CI: `backend-unit`, `webapp-unit`, `migration-smoke`
## 前端验证
- 已验证租户切换后字典列表仅显示当前租户数据
- 附录：截图 / 录屏 / 预览链接✅ 正例：审查意见（清晰、具体、可操作）
## 必须修改（Must Fix）
1. 多租户隔离缺失
- 位置：`DictItemRepository.java:42`
- 问题：查询条件未带 `tenant_id`
- 影响：可能返回其他租户的数据
- 建议：改为基于当前租户上下文的查询方法，并补一个跨租户回归测试
2. 回滚方案不完整
- 位置：PR 描述
- 问题：新增唯一性约束，但没有说明历史脏数据处理策略
- 影响：生产 migration 可能失败
- 建议：补充数据清理脚本或 expand-contract 方案
## 建议修改（Should Fix）
1. 前端验证信息不足
- 问题：提交了页面权限逻辑改动，但没有截图或预览路径
- 建议：补一段手动验证步骤或录屏❌ 反例：审查意见不清晰
1. 这里不太对
2. 感觉会有问题
3. 建议优化一下✅ 正例：审查检查清单
## 审查检查清单
### 合并条件
- [ ] 必需 CI 全绿
- [ ] PR 描述包含验证、风险、回滚
### 业务正确性
- [ ] 核心流程、边界条件、异常路径已确认
- [ ] 变更与需求一致
### 安全 / 权限 / 多租户
- [ ] 权限校验充分
- [ ] 查询和唯一性约束包含 tenant 语义
- [ ] 敏感信息未泄漏
### 测试
- [ ] 自动化测试类型与风险匹配
- [ ] bug 修复已补回归测试
- [ ] 前端交互或关键页面已有验证
### 数据 / 发布
- [ ] migration 与回滚路径明确
- [ ] 配置变更说明完整