admin/danyang-match
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity
Files
47cccc9ca5716b9fe25b3dfeb10fee8ca42031e6
danyang-match/docs/governance/CODE_REVIEW_STANDARDS.md
T
WorkBuddy ca0db57f24 chore(release): 冻结 v1.1.0 版本 
- 同步版本号：VERSION / backend pom / admin-web / miniapp 均升至 1.1.0
- 合并本轮代码审查修复（详见 docs/CODE-REVIEW-2026-09-24.md）：
  - 后端：微信登录真实实现 RealWechatLoginProvider、实名核验真实实现 RealIdentityVerificationProvider（本地强校验 + 可配置第三方 + fail-closed/fail-fast）
  - 照片直传落库端点 POST /me/photos/confirm（fileKey 归属校验 + 对象存在性校验后 savePhoto）
  - 刷新 Token Redis TTL 修正（getExpirationMs()*2 的 4h → refreshExpirationMs 的 14 天）
  - 管理端 token 改为 HttpOnly Cookie + silent-refresh，移除 localStorage 明文存储与 30+ 处手工注入
  - 小程序：红娘接单鉴权头错位修复、serviceType 中文映射、生产域名可配置
- 隔离本地产物（.workbuddy/、*.bundle、*.dump、secrets、application-dev.yml 等已被 .gitignore 覆盖）

未验证项（打标签 ≠ 生产发布）：
- 后端编译 / 单测未在本环境执行（Maven 慢、Docker DNS 受限）
- 管理端 登录→刷新→登出 端到端、小程序真机走查待人工验收
- 生产部署需注入 WECHAT_APP_ID / WECHAT_APP_SECRET、VERIFICATION_API_URL / VERIFICATION_API_KEY 与小程序生产域名
2026-09-30 12:10:00 +08:00

17 KiB
Raw Blame History

代码审查标准与流程 — 善缘丹阳公益婚恋平台

版本：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. 总原则

  1. 证据优先，信任代码而非措辞：PR 描述里的"✅ 已完成 / 已修复 / 已验证"不是证据，只是主张。审查者只认：可读的代码改动 + 能证明判别力的测试 + 运行时实测结论。文档/PPT/截图里写的"已完成"一律按"未证实"处理。
  2. 审查是协作质量门，不是找茬：目标是让合并后的代码比提交时更稳。区分"必须拦下"(Blocker/Major) 与"建议改进"(Minor/Nit)。
  3. 清单是地板不是天花板：本文清单覆盖的是本仓库反复出过事的点；未列出的通用工程规范（命名、复杂度、可读性）同样适用。
  4. 任何放行都需可追溯：审查意见、拒绝原因、已知债务豁免都留在 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 Validation job 会校验，但人也要看变更意图）。
  • 向后兼容：不破坏现有列/约束；NOT NULL 新列须有默认值或配套数据迁移。
  • CHECK 约束与枚举一致：新增/修改 CHECK（如 chk_profile_gender、chk_profile_edu）必须与 Java 枚举值逐一对应，避免 DataIntegrityViolationException(500)。
  • 可逆性：破坏性变更（DROP/重命名）必须有专门的回滚迁移或明确登记为一次性。

4.5 测试（单测 / E2E / 契约守卫）

  • 无空壳：无 skip/pass 空测试（CI Test 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 硬性门槛（全部满足才可合并）

  1. CI 7 道门禁全绿（backend / admin-web / miniapp / migration / test-shell-guard / ui-contracts / e2e）。
  2. PR 模板所有 T 级对应清单项已逐项打勾并附验证说明。
  3. 无未闭环的 Blocker/Major（或已书面转移到 known-risks.md 并取得 owner 确认）。
  4. 新增/修改的测试已做负向注入。
  5. 跨端契约字段、数据库约束、设计 token 已对齐各自事实源。

6.2 与现有 CI 门禁的分工（人审补 CI 之不能）

CI job 它验证了什么 它验证不了的（必须由人审）
Backend Build & Test mvn verify 编译+单测 断言是否有判别力、是否直连绕过、业务不变量是否被逻辑绕过
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），自认 T 级。
  • 提 PR：必须填 PR 模板，含"改了什么 / 怎么验证的（命令+输出）/ 风险级别 / 相关决策或已知风险"。
  • Reviewer：先判 T 级，再走清单；对 Blocker/Major 给可执行的修改建议（具体到文件:行），不只写"这里有问题"。
  • CI 回退：CI 红必须修复或证明是环境问题（如 KR-07 的 docker PATH、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 门禁，建立人工审查分级/清单/严重级别/合并准入/端到端流程。

Reference in New Issue View Git Blame Copy Permalink