CI/CD Pipeline / Backend Build & Test (push) Canceled after 0s
CI/CD Pipeline / Admin Web Build (push) Canceled after 0s
CI/CD Pipeline / Mini Program Compile Check (push) Canceled after 0s
CI/CD Pipeline / Migration Validation (push) Canceled after 0s
CI/CD Pipeline / Test Shell Guard (Phase 23 Gate) (push) Canceled after 0s
CI/CD Pipeline / UI Contracts (source-structure gates) (push) Canceled after 0s
CI/CD Pipeline / E2E (live stack) (push) Canceled after 0s
与 scripts/run-ui-contracts.mjs 同级:固化一组关键后端回归/契约测试 (含 v1.1.0 修复的 3 个回归测试 + JwtAuthFilter Cookie 兜底 + 核心不变量), 先校验测试源文件存在性下限(防改名/删除后门禁假绿),再用 mvn test -Dtest 真跑并解析 surefire 汇总。在 backend job 的 mvn verify 之后执行,复用已 编译产物与依赖缓存,开销极小。同步更新 CODE_REVIEW_STANDARDS.md 门禁说明。
17 KiB
17 KiB
代码审查标准与流程 — 善缘丹阳公益婚恋平台
版本:V1.0 · 生效:2026-09-23 · owner:技术负责人 / 全体提交者 配套:
PLAYBOOK.md(操作手法) ·known-risks.md(已知债务) ·decision-register.md(已确认决策) ·.github/workflows/ci.yml(自动门禁) 目的:本仓库是多端单体仓库(Spring Boot 后端 + 微信小程序 + Vue3 管理端 + 迁移脚本 + E2E),历史上已多次出现"本地/CI 全绿但线上失效"的假绿陷阱。本文档把已踩过的坑固化成人在合并前必须执行的审查纪律,与 CI 自动门禁形成互补——CI 是底线兜底,人工审查是质量上限。
0. 总原则
- 证据优先,信任代码而非措辞:PR 描述里的"✅ 已完成 / 已修复 / 已验证"不是证据,只是主张。审查者只认:可读的代码改动 + 能证明判别力的测试 + 运行时实测结论。文档/PPT/截图里写的"已完成"一律按"未证实"处理。
- 审查是协作质量门,不是找茬:目标是让合并后的代码比提交时更稳。区分"必须拦下"(Blocker/Major) 与"建议改进"(Minor/Nit)。
- 清单是地板不是天花板:本文清单覆盖的是本仓库反复出过事的点;未列出的通用工程规范(命名、复杂度、可读性)同样适用。
- 任何放行都需可追溯:审查意见、拒绝原因、已知债务豁免都留在 PR 或
known-risks.md,不靠口头"应该没问题"。
1. 适用范围与仓库分层
| 层 | 路径 | 技术栈 | 本仓库最高风险点 |
|---|---|---|---|
| 后端 | backend/ |
Spring Boot / Java 21 / Maven / Flyway / JWT | 裸 RuntimeException→500、弱断言、业务不变量被绕过 |
| 小程序 | miniapp/ |
原生 WXML/WXSS/TS,app.json/pages.json 声明式 |
tsc 不保跨端字段名;ignoreDevUnusedFiles 吞资源;WXSS 编译静默失效 |
| 管理端 | admin-web/ |
Vue3 / TS / pnpm / Element Plus | 非响应式状态不重渲染;设计 token 编译期生成;vite build 清 dist/ |
| 迁移 | backend/src/main/resources/db/migration/V*.sql |
Flyway | 版本缺号/重复、CHECK 约束与 Java 枚举漂移 |
| 测试 | tests/e2e/(pytest)、ui-tests/*.mjs(契约守卫)、backend/src/test/ |
多框架 | 空壳测试、缺负向注入、守卫数下限被悄悄删 |
| 脚本/基建 | scripts/、infrastructure/ |
Node/Python/Compose | 依赖声明与实际不符(本地绿/CI 红) |
2. 审查分级(按变更风险)
| 级 | 名称 | 触发条件 | 最少审查人数 | 审查深度 |
|---|---|---|---|---|
| T0 | 琐碎 | 纯文案/注释/文档、*.md、与逻辑无关的格式化、资源重命名(无引用变更) |
1(可自审) | 仅看是否误伤其他引用 |
| T1 | 常规 | 单端功能开发、Bug 修复、UI 调整、单个 API 增改 | 1 | 走 §4 对应技术栈清单 + §5 判据纪律 |
| T2 | 高危 | 涉及:鉴权/登录链路、支付/资金、数据隐私(PII)、跨端契约字段、数据库 schema 变更、并发/幂等、未接真实现的已知阻塞项(见 §6.4) | 2(含 1 名对应该端熟悉者) | 全清单 + 运行时实测结论 + 回滚方案 |
判定不清时就高不就低。一人团队下"第 2 人"可由技术负责人交叉复核,或在 PR 中显式标注"已自审 + 已跑实测"并附命令与输出。
3. 严重级别定义
| 级 | 含义 | 合并策略 | 本项目真实示例 |
|---|---|---|---|
| Blocker | 合并后会破坏生产或导致数据错误/安全泄露 | 必须修,否则拒绝合并 | 裸 RuntimeException 掩盖业务拒绝(→500);updateById 对 user_id 主键静默 no-op;跨端字段名写错使请求永不落库 |
| Major | 重要功能不可用或存在明显技术债放大风险 | 必须修或可书面豁免(登记到 known-risks.md) |
tsc 通过但 WXML 多一个 </view> 编译失败;守卫被删导致"假绿";新增死表/死配置 |
| Minor | 不影响功能但降低可维护性/一致性 | 建议修,可不阻塞 | 魔法数字未抽常量;日志缺失;字段命名与 decision-register 不一致 |
| Nit | 风格/偏好层面 | 不阻塞,鼓励修 | 换行、注释措辞、导入顺序 |
原则:Blocker/Major 必须在该 PR 闭环或明确转移到 known-risks.md;Minor/Nit 不得作为合并阻塞,避免"审查变成挑刺"。
4. 各技术栈审查清单(核心)
每项审查时逐项打勾(在 PR 模板里),打勾 = 已实际验证,不是"应该没问题"。
4.1 后端 Java / Spring Boot
- 异常分类正确:业务拒绝抛
BusinessException+ApiResponseCode,HTTP 落在400 ≤ code < 500;不得用裸RuntimeException(orElseThrow(() -> new RuntimeException(...))形态也要改,历史上漏过 17 处)。 - 断言有判别力:测试对"业务拒绝"断言
400 ≤ status < 500,不得只断言!= 200(会把 500 洗成"业务拒绝"假绿)。 - 不变量守住:匹配/聊天/活动互选等核心规则(见
decision-register的 LOCKED 项,如 INV-014/015 两阶段同意、Required/Mutual/Block)未被绕过;无"直连绕过"或"单方直建"的捷径。 - 数据落库正确:主键/唯一键用法正确(如
UserPreference真实主键是user_id,不能用updateById对其 no-op);NOT NULL / CHECK 约束与 Java 枚举一致。 - 幂等/并发:写接口有幂等键或乐观锁;无裸
synchronized跨实例假设。 - 配置可信源:新增开关走活键(
application*.yml的match.*/features.*),不得新增system_configs/match_rule_configs这类"死表"(见 KR-02/KR-03)。 - 敏感信息:密钥/口令不入库、不进日志;JWT Secret 来自环境变量。
4.2 小程序 miniapp(原生)
app.json/pages.json一致性:tabBar、页面路径、资源路径与磁盘真实文件一致;路径不带前导/;引用的图标/图片文件确实存在。- 资源不被吞:
project.config.json的ignoreDevUnusedFiles/ignoreUploadUnusedFiles必须为false(否则只被app.json声明、不被任何 WXML 引用的图标会被打包/上传剔除)。 - 跨端字段名:小程序发出的字段名与后端规范名完全一致(历史漂移:
hasChildren/occupationCategory/workRegion/ageMin/educationAccepted/maritalStatusAccepted等);tsc 不保护 JS 层字段名,必须看值。 - WXSS 合法性:无通配符
*(编译期致命);不用属性选择器[attr](静默失效);注释不嵌套;display:grid在 Skyline 不支持→用 flex。 - 胶囊安全区:自定义导航让出
--nav-right(utils/nav-metrics.js唯一事实源),不遮挡右上角胶囊。 <image>高度:只写width不写height会按默认 240px 撑高(卡片异常高大真因),必须显式高度或mode。
4.3 管理端 admin-web(Vue3 / TS)
- 响应式状态:涉及渲染的数组/对象改动走
ref/reactive或reactive()包裹,不用普通声明数组当状态写且出现在模板(曾致"静态守卫全绿但页面不重渲染");改完必须补渲染态实测。 - 设计 token:颜色/字号收敛到
src/theme/index.css+src/theme/tokens.ts(同名取值双向校验);Element Plus 主色阶是编译期静态生成,--el-color-primary改了必须补全-light-N/-dark-2/-rgb,否则残留默认蓝。 - 类型安全:跨组件 props/接口用 TS 类型;不靠
any绕过;表单校验用 Zod/类型安全方案,错误内联且保留用户输入(出错不清空)。 - 触摸目标:交互元素 ≥ 44×44px,间距 ≥ 8px;尊重
prefers-reduced-motion。
4.4 数据库迁移 SQL(Flyway)
- 版本连续唯一:
V<序号>__<描述>.sql命名,无缺号/重复(CI 的Migration Validationjob 会校验,但人也要看变更意图)。 - 向后兼容:不破坏现有列/约束;NOT NULL 新列须有默认值或配套数据迁移。
- CHECK 约束与枚举一致:新增/修改 CHECK(如
chk_profile_gender、chk_profile_edu)必须与 Java 枚举值逐一对应,避免DataIntegrityViolationException(500)。 - 可逆性:破坏性变更(DROP/重命名)必须有专门的回滚迁移或明确登记为一次性。
4.5 测试(单测 / E2E / 契约守卫)
- 无空壳:无
skip/pass空测试(CITest Shell Guard会拦,但人也要看测试是否真在断言什么)。 - 负向注入:新增或修改测试后必须做负向注入证明测试有判别力(故意改坏被测代码,测试应转红);不做负向注入 = 视为未验证。
- 守卫数下限:
scripts/run-ui-contracts.mjs的MIN_GUARDS(当前 23)是硬下限,增删守卫必须同步改该常量;审查时确认守卫数量未偷偷下降。 - E2E 真实执行:
tests/e2e用例在本机用真实栈跑过(非 import 期即崩的"假绿");依赖docker在 PATH(否则FileNotFoundError误判)。
4.6 脚本与基础设施
- 依赖声明即真实依赖:脚本 import 的包(如
ui-tests守卫依赖miniapp/node_modules/typescript)必须在 CI 对应 job 里安装——"本地能跑"不等于"CI 能跑"。 - 幂等与可重入:脚本可重复执行、不产生副作用累积;密钥仅在运行期、不落盘。
- Compose 隔离:本地栈固定
-p danyang,不与邻项目撞名(KR-09 教训:-p infra互删容器)。
5. 判据纪律:审查必须拒收的"假绿"模式(重点)
以下每一条都来自本仓库真实事故。审查者看到对应改动,必须主动排查,不能因为 CI 绿就放行。
| # | 假绿模式 | 为什么是缺陷 | 审查动作 |
|---|---|---|---|
| 1 | 文档/PPT 写"✅ 已完成" | 措辞不是证据 | 要求贴代码 diff / 测试输出 / 实测命令,否则按未证实处理 |
| 2 | 空数据假绿 | 列表/推荐接口返回空数组也能"通过",掩盖查询逻辑失效(推荐曾恒空) | 审查测试是否用真实种子数据断言非空结果 |
| 3 | 直连绕过假绿 | 测试直连 Service 而非走 Controller,跳过鉴权/序列化/HTTP 语义 | 关键路径至少有一个经 HTTP 层的端到端断言 |
| 4 | 测试库缺约束假绿 | 测试库没建 CHECK/外键,生产库的约束会在运行时炸成 500 | 约束相关改动必须在带约束环境下验证 |
| 5 | 弱断言 != 200 |
把 500 洗成"业务拒绝",假绿 | 业务拒绝一律断言 400 ≤ code < 500 |
| 6 | 裸 RuntimeException |
属于 500 家族缺陷,掩盖真实错误 | grep new RuntimeException 与 orElseThrow(() -> new RuntimeException,要求改 BusinessException |
| 7 | 代码合法 ≠ 渲染正确 | Vue 普通声明数组当状态、WXML 结构错误,静态守卫全绿仍不渲染 | 涉及渲染的改动必须补渲染态实测(CDP/截图) |
| 8 | job 名 ≠ 实际行为 | CI Mini Program Compile Check 只跑 tsc --noEmit,不真编译 WXML/WXSS |
小程序改动不能只看该 job 绿;真编译走 scripts/miniapp-compile-check.mjs(本地) |
| 9 | 跨端字段名漂移 | JS 层字段名不受 tsc 保护,写错使请求字段落空/不落库 | 跨端契约改动逐字段核对 miniapp ↔ backend 规范名 |
| 10 | 同一文件并行 Edit | 后者覆盖前者,改动丢失 | 确认 PR 内对同一文件无冲突编辑;合并后回读该文件 |
| 11 | 缺负向注入 | 测试必然全绿,无判别力 | 要求演示"改坏后测试转红" |
6. 合并准入(Definition of Done / 合并门槛)
6.1 硬性门槛(全部满足才可合并)
- CI 7 道门禁全绿(backend / admin-web / miniapp / migration / test-shell-guard / ui-contracts / e2e)。其中 backend 门禁在
mvn verify之外,额外由scripts/run-backend-contracts.mjs(与run-ui-contracts.mjs同级)固化一组关键后端契约/回归测试,并校验这些测试源文件的存在性下限(防静默删除/改名后门禁假绿)。 - PR 模板所有 T 级对应清单项已逐项打勾并附验证说明。
- 无未闭环的 Blocker/Major(或已书面转移到
known-risks.md并取得 owner 确认)。 - 新增/修改的测试已做负向注入。
- 跨端契约字段、数据库约束、设计 token 已对齐各自事实源。
6.2 与现有 CI 门禁的分工(人审补 CI 之不能)
| CI job | 它验证了什么 | 它验证不了的(必须由人审) |
|---|---|---|
| Backend Build & Test | mvn verify 编译+单测;run-backend-contracts.mjs 固化关键回归/契约测试并校验源文件下限 |
断言是否有判别力、是否直连绕过、业务不变量是否被逻辑绕过 |
| Admin Web Build | pnpm build 通过 |
运行时渲染正确性、token 是否补全色阶、响应式是否真重渲染 |
| Mini Program Compile Check | 仅 tsc --noEmit |
真编译 WXML/WXSS、资源是否被 ignoreDevUnusedFiles 吞、胶囊安全区 |
| Migration Validation | 序列/命名/非空 | CHECK 与枚举语义一致、向后兼容、可逆性 |
| Test Shell Guard | 无空壳测试 | 测试是否真在断言、是否缺负向注入 |
| UI Contracts | 23 守卫结构门 | 守卫数下限是否被偷偷删、守卫是否覆盖新契约 |
| E2E (live stack) | 活栈 pytest | 隐私/资金链路、已知阻塞项是否被正确处置 |
6.3 合并后验证(回读纪律)
- 合并/推送后必须回读:
git rev-parse HEAD/git log -1/git status --porcelain,不只用退出码判断。 - 远端真值只认
git ls-remote origin refs/heads/main(本机origin/main会卡旧值谎报 ahead)。 - 推送后轮询 CI(
.workbuddy/poll_ci.py),确认本次 head 的 run 真正 success,而非"上次绿的缓存印象"。
6.4 已知上线阻塞(触碰即须显式处置)
以下两项为当前上线阻塞(见 PLAYBOOK.md §E),任何相关 PR 必须:要么本次解决,要么在 PR 中明确说明"本次未解决,仍依赖 Mock/未落库,已登记 known-risks"。
- 微信登录无真实实现:全库仅
MockWechatLoginProvider,WECHAT_APP_SECRET无人读。 - 照片直传不落库:无直传确认端点;小程序用
photoId、后端返fileKey,契约不符。
7. 审查流程(端到端)
作者自检 ──▶ 提 PR(用 .github/PULL_REQUEST_TEMPLATE.md)──▶ Reviewer 按 T 级执行 §4+§5
│
▼
有 Blocker/Major? ──是──▶ 退回修改(登记若豁免)
│否
CI 7 门禁触发 ──失败──▶ 修复并重跑
│全绿
合并(保护分支)──▶ 合并后回读 + 轮询 CI
- 作者自检:提交前本地跑对应测试 + 契约守卫(
scripts/run-ui-contracts.mjs、scripts/run-backend-contracts.mjs),自认 T 级。 - 提 PR:必须填 PR 模板,含"改了什么 / 怎么验证的(命令+输出)/ 风险级别 / 相关决策或已知风险"。
- Reviewer:先判 T 级,再走清单;对 Blocker/Major 给可执行的修改建议(具体到文件:行),不只写"这里有问题"。
- CI 回退:CI 红必须修复或证明是环境问题(如 KR-07 的
dockerPATH、KR-08 的镜像源),不得"重试蒙过"。 - 合并:保护分支禁止直接 push
main;合并即视为已承担该 PR 的审查结论。 - 合并后:回读 + 轮询 CI(§6.3)。
8. Reviewer 职责与礼仪
- 区分 blocker 与 nit:Blocker/Major 拦下并给方案;Minor/Nit 用"建议"语气,不阻塞。
- 给可执行的反馈:"建议把这里的
RuntimeException换成BusinessException(ApiResponseCode.USER_NOT_FOUND)" 优于 "异常处理不规范"。 - 不自以为替代测试:审查不豁免作者写测试与负向注入的责任。
- 尊重决策源:与
decision-register的 LOCKED 项冲突的改动,先要求回到决策层确认,不擅自放行。 - 留痕:拒绝理由、豁免转移都留在 PR 或
known-risks.md,便于复盘"逃逸"。
9. 度量与持续改进
- 逃逸率:合并后在生产/实测发现的缺陷 ÷ 合并总数,按月统计,目标递减。
- 审查时延:PR 从开到合并的中位时长,避免积压。
- 假绿复发计数:§5 的 11 类模式再次出现的次数,归零为目标。
- 季度回看:结合
known-risks.md与PLAYBOOK.md,把新踩的坑补充进 §4/§5,把已解决的债务从known-risks移除。 - 守卫同步:每次新增契约守卫,同步上调
MIN_GUARDS;审查时核对守卫数。
最后更新: 2026-09-23 — V1.0 初版。基于 PLAYBOOK 判据纪律、known-risks、decision-register 与 ci.yml 门禁,建立人工审查分级/清单/严重级别/合并准入/端到端流程。