Skip to content

Latest commit

 

History

History
118 lines (98 loc) · 8.32 KB

REVIEW.md

File metadata and controls

118 lines (98 loc) · 8.32 KB

自动 Code Review 审阅口径

本文件供 .github/workflows/pr-code-review.yml 触发的自动 reviewer 读取。人工 review 的完整口径见 docs/dev-rules/development-workflow.md §3,本文件不另立标准, 只把它翻译成自动 reviewer 能直接执行的形式。

1. 先读规则再评审

本仓的规则是正本,不要凭通用最佳实践判断对错:

  • AGENTS.md —— 规则索引与触发条件,先看它决定该读哪几份。
  • docs/dev-rules/ —— 工程规则;docs/product-rules/ —— 产品行为规则; docs/design-rules/DESIGN.md —— 权威视觉规范。
  • .github/PULL_REQUEST_TEMPLATE.md —— 风险分类的口径。

diff 命中哪个模块,就读 AGENTS.md 索引里对应那条指向的规则文件,再评审。

2. 严重度映射

docs/dev-rules/development-workflow.md §3 用 P0/P1/P2,本 workflow 的评论用 P1/P2。按下表转换,不要把仓库口径的 P2 改写成评论里的 P2 上报:

仓库口径 评论标记 含义
P0:红线/崩溃/数据丢失/跨平台失效/安全 P1 不改不能合
P1:明显 bug/规范违反/影响面没处理干净 P2 本次必须修
P2:可选优化/风格偏好 不上报 整条略过,不降级也不换个说法上报

3. 不要重复机器门禁

以下问题 client-ci 与其他 workflow 已经逐条断言,命中即红,自动 reviewer 报了 只是占用有限的 finding 名额:

  • 四语言 i18n key 结构、术语表译法、品牌名占位符(check:i18n / check:i18n-glossary / check:brand-terminology)。
  • 受控源文件里的生产端点与飞书 App ID 字面量(check:endpoints)。
  • migration 序号连续性、journal/snapshot 对齐、历史 migration 冻结 (desktop db:validate)。
  • scheduler 反向依赖、cron 三方库、scheduler renderer 色值白名单 (ci:scheduler-guard)。
  • mobile 的 Issue Confirm 范围守门(mobile test:scope)。
  • typecheck、单测、DCO 签名、PR 正文「引用的设计规范」字段(pr-design-basis, 仅在变更命中 UI 路径时校验该字段)。

另外,不要因为「PR 正文没写清楚」「commit message 格式」这类流程问题开评论。 注意这一条与上面那张清单的理由不同:它不是因为有机器门禁覆盖。pr-template-rules 只校验 .github/PULL_REQUEST_TEMPLATE.md 这个模板文件本身的二级标题,且只在模板、 校验脚本或该 workflow 自身变动时触发,从不读取任何具体 PR 的正文;也就是说「摘要、 怎么验证的、风险」这些必填段落缺失时,机器不会拦。这里仍然让自动 reviewer 略过, 是因为 PR 叙述质量属于人工 review 的范围,不该占用有限的 finding 名额——这是范围 划分,不是覆盖声明。

4. 重点看机器查不到的部分

按 PR 模板的风险分类,优先看这些:

  • 凭证与本地存储:凭证、令牌、授权文件是否写进了仓库或可能被 Git 跟踪的路径; 用户数据落盘位置是否越界。见 docs/dev-rules/credentials-and-local-storage.md。
  • Electron 进程边界:renderer/preload/IPC/CSP/WebView/导航与特权能力的改动 是否扩大了攻击面。见 docs/dev-rules/electron-security-and-process-boundaries.md。
  • 数据库:新 migration 的正确性与可回滚性、companion 脚本必须是 CommonJS (生产 Electron 用 require() 加载,ESM 语法只在用户端炸)。见 docs/dev-rules/database-and-migrations.md。
  • mobile 冷更边界:改动是否触碰 app.json/app.config.js/eas.json/ apps/mobile/package.json/plugins//modules/ 等进入 runtime fingerprint 的输入。 命中就在评论里点名——这类 PR 需要把关人对冷更单独确认。见 docs/dev-rules/mobile-development.md。
  • 协议兼容:本地协议 package、device-link/relay/隧道 payload、IPC allowlist 等跨端 wire protocol 的向后兼容性。见 docs/dev-rules/protocol-compatibility.md。
  • device-link 恢复动作的故障半径:重试/超时/断链/重连路径的改动,恢复动作的 作用半径是否大于故障半径——被控端一条 relay 连接服务同账号全部控制端,单 peer 故障不得强拆整条共享连接;扩大半径必须在 PR 描述里给出明确理由,并有多控制端拓扑 用例证明其它 peer 零感知。wire 向后兼容 + 单测全绿拦不住这类问题(单测只验证实现 忠实于设计,设计选错半径时不会报警)。见 docs/dev-rules/remote-and-mobile-adaptation.md 的「恢复动作先回答故障半径」。
  • system prompt 与 Agent 行为:进入模型 system 段的提示词、tool/MCP 暴露、 usage 计量。见 docs/dev-rules/maker-core-and-agent-behavior.md。
  • 插件沙箱:.cindy 运行时的权限、能力 slot、网络/凭证/文件交接。见 docs/dev-rules/plugin-security-and-authoring.md。
  • 存量插件兼容(红线,优先级等同凭证与安全):改了宿主侧批准状态记录(receipt 一类) 的 schema/必填字段/落盘位置、指纹或摘要编码、manifest 校验、slot 形态、技能快照与 链接命名、安装根/状态根路径、.cindy 包格式、管子协议或内置插件 id 时,必须按用户 升级后什么都不做来判:已装、已批准、已启用的插件是否照旧可用。新增必填字段或 新校验只有拒绝分支、没有从旧版 授权事实 backfill 的迁移路径,就是 P1(不改不能合)——"老数据缺新字段"是历史状态, 不是篡改。自动迁移做不到时,必须有明确提示 + 一次性批量恢复入口(不能只留"去市场 重装")、不清掉用户已存的凭证与偏好、且新状态被旧版本读到不判损坏。还要看有没有基于 旧布局 fixture 的升级用例——只测全新安装不算覆盖,这类回归只在存量数据上出现。判据见 docs/dev-rules/plugin-security-and-authoring.md 第 5 节与 Review 清单第 5 条。这类 PR 同时 要走白名单确认门(放行人明确 Approve 才能合并),命中就在评论里点名;门本身不看 diff 大小,也不因「是 bugfix/纯技术改动」豁免。
  • UI 双模式:新增或修改的界面必须同时实现 Light 与 Dark,颜色走语义 token; 只适配一种模式的硬编码或条件补丁视为未完成。见 docs/design-rules/DESIGN.md。
  • UI 圆角分类与点击目标尺寸:普通 UI 改动同样先读 docs/design-rules/DESIGN.md §5 与 docs/design-rules/design-governance.md §13,不只在设计系统迁移 PR 中读取。先识别 已登记成员覆盖的具体可见层,再核对登记值和交互约束;不得仅凭 DOM 标签、可访问名称 或局部样式类推导 pill。未决分类、证据缺口、违反登记值分别报告;已登记的待迁移差异 按 design-inventory.md 对应条目处理,不用相反的改形建议替代缺失裁决。 用量图表的密集日期目标按 §5 的 Hit-target sizing 条款核对 Equivalent 路径;同页等价 日期选择控件已实际交付并满足 Equivalent 时,不得仅因单个目标小于 24×24px 就要求逐柱 撑宽或横向滚动。控件未交付时按 §5 与台账的登记过渡记录处理——报告欠付的等价控件, 不以 24px 撑宽或横向滚动等改形建议替代,也不把 data mark 登记当成尺寸通用豁免。 规范修订 PR 应按已授权的新裁决核对正文、摘要与登记的一致性,不把正在被取代的旧 措辞当作保留它的理由;新条款自身的实际矛盾、遗漏与越界仍须报告。
  • 跨平台:macOS 与 Windows 的路径、进程、文件系统差异。
  • 测试:是否通过 skip、删除或弱化断言制造通过。

5. 评审环境的已知限制

  • 只 checkout 了 PR 的 merge ref 与最近 100 条历史,更早的 git log/git blame 会不完整。
  • 依赖未安装,无法运行测试或构建;结论只能来自静态阅读。

拿不准的地方,宁可不报,也不要用推测填空——上报前先把相关源文件读到能确认为止。

6. 评论语言

用中文写评论,与仓库文档保持一致;PR 正文为英文时用英文。