WorkBuddy
9fc3385d6c
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 门禁说明。
197 lines
17 KiB
Markdown
197 lines
17 KiB
Markdown
# 代码审查标准与流程 — 善缘丹阳公益婚恋平台
|
||
|
||
> **版本**：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）。其中 backend 门禁在 `mvn verify` 之外，额外由 `scripts/run-backend-contracts.mjs`（与 `run-ui-contracts.mjs` 同级）固化一组关键后端契约/回归测试，并校验这些测试源文件的存在性下限（防静默删除/改名后门禁假绿）。
|
||
2. PR 模板所有 **T 级对应清单项**已逐项打勾并附验证说明。
|
||
3. 无未闭环的 Blocker/Major（或已书面转移到 `known-risks.md` 并取得 owner 确认）。
|
||
4. 新增/修改的测试已做负向注入。
|
||
5. 跨端契约字段、数据库约束、设计 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 的 `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 门禁，建立人工审查分级/清单/严重级别/合并准入/端到端流程。*
|