PR 评审反馈闭环

场景

gitcode-cli 项目的外部贡献者通过 fork 提交 feature PR。评审者按 spec/workflows/review-workflow.md 完成首轮结构化评审,给出 P1(should-fix)与 P2(nits)分级意见;作者按清单修复后推送新 commit;评审者拉取 fork 分支本地复核(diff 核对 + go build/go test/go test -race),确认全部命中后放行合并。

本案例以 PR #440(feat(actions): 新增 gc actions yaml validate 命令)为实例,完整记录"评审 → 修复 → 复核 → 放行"闭环,并覆盖两个高频踩点:

  1. P0 误判撤回:首轮把"真实命令验证用了个人仓库而非 infra-test/*"标为 P0 阻塞;经维护者澄清规则边界(外部贡献者无 infra-test 权限时,用其本人授权仓库验证并记录"无 infra-test 权限"才是正确路径)后撤回 P0。教训:升级阻塞前先核实规则适用边界,不机械套用。
  2. approval 权限 403 回退 comment:gc pr review --approve 需要 GitCode 平台单独授予的"approval permission"(与 merge permission 分离)。无此权限时返回 403,按 --help 提示回退 --comment 发布 APPROVED 正文。

PR #440 背景:作者 wangwq129(fork: wangwq129/cli)为 Issue #497 实现 gc actions yaml validate 子命令,调用 GitCode Actions v8 官方校验接口,base64 编码 YAML 后 POST,输出人类可读结果或原样 JSON。变更 +767/-0 in 8 files,风险 medium。

推荐 skill

本案例可结合独立仓库 gitcode-cli/skills 中的工作流 Skill:

  • gitcode-pr-review — 工程 PR 评审、验证与结论提交(本案例主 skill)
  • gitcode-pr-apply-feedback — 作者侧:接收评审意见、落地修复、推送、回复(本案例作者侧流程)
  • 可辅助:gitcode-pr — pr merge、pr view 等底层操作

注意:评审边界以 spec/workflows/review-workflow.md 为准,Skill 提供执行方法不替代正式规范。

适用人群

  • 评审他人 PR 的单评审者(非多 Agent 团队场景)
  • 项目维护者(理解反馈闭环如何高效收口)
  • 外部贡献者(理解评审者如何复核自己的修复)
  • AI 评审代理(学习 P1/P2 分级与复核命令链)

可直接执行的 Prompt

请使用 gitcode-pr-review skill,对 gitcode-cli/cli 的 PR #<N> 做评审反馈闭环。

要求:
1. 先 `gitcode pr view <N> -R gitcode-cli/cli --comments --json` 通读评审历史,
   找出上一轮已给出的 P1/P2 意见清单与作者"已修复"的回复。
2. `gitcode pr diff <N> -R gitcode-cli/cli` 核对 diff,逐项确认每条 P1/P2 是否命中。
3. 拉取 fork 分支本地复核(fork PR 不能用 origin 远程):
   git fetch git@gitcode.com:<author>/cli.git <branch>
   git checkout FETCH_HEAD
   go build ./...
   go test ./<changed-pkgs>/...
   go test -race ./<changed-pkgs>/...
4. 对涉及共享能力复用、安全防御的修复,顺带 grep 源码确认底层函数确有对应行为
   (如评审要求"复用 cmdutil 并恢复 secret 扫描",应核实 cmdutil.ReadTextFile /
   ReadTextFromFlag 内部确实调用 ScanContentForSecrets)。
5. 全部命中且本地验证通过 → 撰写 APPROVED 复核意见(含逐项命中表 + 本地验证证据)。
6. 先 `gitcode pr review <N> -R gitcode-cli/cli --approve --comment-file <approval.md>`;
   若返回 403(无 approval permission),回退
   `gitcode pr review <N> -R gitcode-cli/cli --comment-file <approval.md>`。
7. 放行合并:`gitcode pr merge <N> -R gitcode-cli/cli --yes --json`,
   再 `gitcode pr view <N> --json` 确认 state=merged。
8. 若首轮意见有误判(规则边界理解偏差),必须在 PR 评论中显式撤回并说明正确边界,
   不得静默放弃。

注意:
- 评审与作者须为不同执行上下文。
- 复核必须基于最新 head sha,不能用过期 diff。
- 裸 `#NNN` 在 PR body 会触发 GitCode 自动关闭,评论中可用;但自检/描述文本引用其他
  issue 时用文字描述(如"后续 run view 交付")。

预期产出

  • 逐项命中表:P1/P2 每条意见 → 修复位置 → 复核证据(diff 片段 / 测试函数 / 源码行号)
  • 本地验证证据:build / test / race 命令与结果
  • 一份 APPROVED 复核评论,发布到 PR
  • 合并结果(merged: true、merged_at)
  • 若首轮有误判,附带撤回说明

价值

  • 闭环而非单点:把"评审→修复→复核→放行"串成可复用管线,覆盖评审反馈最常见却最易遗漏的复核环节。
  • 分级清晰:P1 should-fix + P2 nits 模型,让作者批量修复、评审者批量复核,避免逐条往返。
  • 外部贡献者路径:明确外部贡献者无 infra-test 权限时的验证替代路径与"误判撤回"范式。
  • 权限踩点前置:把 approval permission 403 → comment 回退写进流程,避免评审者在合并门口卡住。
  • fork PR 复核命令链:固化 git fetch <fork-url> <branch> + go build/test/race 的复核命令,可直接套用。

复用方式

替换清单

占位符 案例值 替换为
仓库 gitcode-cli/cli 目标仓库
PR 编号 #440 待复核 PR
PR 作者 wangwq129(fork: wangwq129/cli) PR 提交者及其 fork
分支 feature/issue-497 PR head 分支
意见清单 P1 readYAML 复用 cmdutil / P2 六项 nits 上一轮评审给出的意见
改动包 ./pkg/cmd/actions/yaml/... ./api/... 该 PR 实际变更的包

适用场景

跨平台提醒

  • 复核用 git fetch git@gitcode.com:<author>/cli.git <branch>;fork PR 在 origin 远程找不到 head 分支。
  • Windows PowerShell 下 go test 输出中文可能乱码,可重定向到文件再读。
  • gc pr review --approve 与 gc pr merge 是两套独立权限;有 merge 权限不代表能 approve。

前置条件

  • gitcode auth status 已登录且有仓库读权限
  • 本地有 Go 工具链(复核 Go PR 必需)
  • 已读 spec/workflows/review-workflow.md 理解评审角色与分级
  • PR 已经过作者自检与上一轮评审

本次真实执行记录

执行信息

评审时间线(前序轮次)

时间 动作 要点
08-06 17:12 Round-1 评审 CHANGES-REQUESTED;P0(infra-test 偏差)+ P1(readYAML 复用 cmdutil + secret 扫描)+ P2(6 项 nits)
08-06 17:18 P0 撤回 维护者澄清:外部贡献者无 infra-test 权限时用本人授权仓库验证是正确路径;撤回 P0
08-06 17:28 修复清单固化 P1 should-fix(附可直接套用的 readYAML 代码片段)+ P2 nits 六项
08-06 19:51 作者回复已修复 commit b46b18b:P1+P2 全部落实 + 额外 secret 扫描拦截用例
08-06 20:03 作者修正空文件用例 commit 0e69a38:按真实 API 400 行为断言 ExitUsage(2)
08-07 10:36 评审者复核 + 放行 本文阶段

阶段 1:通读评审历史

gc pr view 440 -R gitcode-cli/cli --comments --json

确认上一轮已给 P1(readYAML 复用 cmdutil)+ P2 六项,作者在 commit b46b18b/0e69a38 称已全部落实。

阶段 2:核对 diff

gc pr diff 440 -R gitcode-cli/cli

逐项核对命中情况:

意见 修复位置 复核证据
P1 readYAML 复用 cmdutil I/O validate.go readYAML 改为 cmdutil.ReadTextFromFlag(stdin, "--file") / cmdutil.ReadTextFile(file),不再手写 os.ReadFile/io.ReadAll
P1 恢复 secret 扫描 同上 见阶段 4:text_file.go:73,104 两函数均调 ScanContentForSecrets,后者含 PS lossy-stdin 守卫(L101)
P2.1 参数名 io→ios printResult 签名 printResult(ios *iostreams.IOStreams, ...)
P2.2 API 注释改写 api/queries_actions.go "Authentication uses the standard Bearer header; no token is placed in the query string."
P2.3 MissingFile 退出码断言 validate_test.go ExitCode(err) == ExitError
P2.4 退出码表格化 TestValidateRunAPIError 401/403→ExitAuth(4)、404→ExitNotFound(3)、409→ExitConflict(5)
P2.5 0 字节空文件用例 TestValidateRunEmptyFile 按真实 API 400 断言 ExitUsage(2)
P2.6 README 示例对齐 README.md:355 gitcode actions yaml validate
额外 secret 拦截用例 TestValidateRunRejectsContentWithSecret YAML 含当前 GC_TOKEN 时拒绝请求、API 不被调用、exit 1

阶段 3:拉取 fork 分支本地复核

# fork PR 不能用 origin 远程,需显式 fetch fork
git fetch git@gitcode.com:wangwq129/cli.git feature/issue-497
# → FETCH_HEAD = 0e69a3893e8330d29bb1c16971a4703b8dd54e9b
git checkout FETCH_HEAD
# HEAD is now at 0e69a38 test: align empty-file boundary with real API 400 behavior

go build ./...
# (no output) ✅

go test ./pkg/cmd/actions/yaml/... ./api/...
# ok  gitcode.com/gitcode-cli/cli/pkg/cmd/actions/yaml/validate  0.006s
# ok  gitcode.com/gitcode-cli/cli/api  1.469s ✅

go test -race ./pkg/cmd/actions/yaml/validate/...
# ok  gitcode.com/gitcode-cli/cli/pkg/cmd/actions/yaml/validate  1.015s ✅

阶段 4:核实底层共享能力(P1 关键依据)

P1 的核心论据是"复用 cmdutil 可零成本恢复 secret 扫描 + PS stdin 守卫"。复核需确认底层函数确有此行为,而非仅看 readYAML 调用点:

# 确认两个 cmdutil 函数内部调用 ScanContentForSecrets
rg "func (ReadTextFile|ReadTextFromFlag)" pkg/cmdutil/
# pkg/cmdutil/text_file.go:67  ReadTextFile
# pkg/cmdutil/text_file.go:95  ReadTextFromFlag

读 pkg/cmdutil/text_file.go:

  • ReadTextFile(L67-77):os.ReadFile → DecodeUserText → ScanContentForSecrets(text) ✅
  • ReadTextFromFlag(L95-108):io.ReadAll → DecodeUserText → isLikelyLossyPowerShellStdin 守卫(L101)→ ScanContentForSecrets(L104) ✅

P1 的两项防御(secret 扫描 + PS stdin 守卫)均生效,且错误保留 %w 包装,errors.Is(err, cmdutil.ErrSecretDetected) 仍可识别。

阶段 5:撰写并发布 APPROVED 复核意见

# 先尝试 --approve
gc pr review 440 -R gitcode-cli/cli --approve --comment-file /tmp/pr440-approval.md
# Error: failed to approve PR: HTTP 403: 403 Forbidden
#        - You don't have the authority to approval this merge request.

403:--approve 需要 GitCode 平台单独授予的 approval permission(与 merge permission 分离)。按 gc pr review --help 提示回退 --comment:

gc pr review 440 -R gitcode-cli/cli --comment-file /tmp/pr440-approval.md
# ✓ Commented on PR #440

评论正文含:逐项命中表(P1 两项 + P2 六项 + 额外 secret 用例)+ 本地验证三命令结果 + "无阻塞项,准予合并"结论。

阶段 6:合并放行 + 验证

gc pr merge 440 -R gitcode-cli/cli --yes --json
# { "number": 440, "merged": true, ... }

gc pr view 440 -R gitcode-cli/cli --json | \
  python3 -c "import json,sys;d=json.load(sys.stdin);pr=d.get('pull_request',d);print('state:',pr['state']);print('merged:',pr['merged']);print('merged_at:',pr.get('merged_at'))"
# state: merged
# merged: True
# merged_at: 2026-08-07T10:36:08+08:00

PR body 含 Closes #497,合并后 Issue #497 由 GitCode 自动关闭。

端到端流程

PR #440 (wangwq129 fork, feature/issue-497)
  → 通读评审历史(P1+P2 清单)
  → gc pr diff 逐项核对命中
  → git fetch <fork-url> feature/issue-497      ← fork PR 复核关键
  → git checkout FETCH_HEAD (0e69a38)
  → go build / go test / go test -race          ← 本地等价门禁
  → grep 核实 cmdutil 底层 ScanContentForSecrets 行为  ← P1 依据
  → 撰写 APPROVED 复核意见
  → gc pr review --approve → 403                ← approval 权限踩点
  → 回退 gc pr review --comment-file           ← 按 --help 提示
  → gc pr merge --yes --json                    ← 放行
  → gc pr view 确认 state=merged
  → Issue #497 自动关闭(PR body Closes #497)

关键经验

  1. 复核必须基于最新 head sha:作者可能多轮推送(9be35e6 → b46b18b → 0e69a38),过时 diff 会漏掉修正。git fetch 后用 git rev-parse FETCH_HEAD 与 PR 页面 sha 比对。
  2. fork PR 复核命令链固定:git fetch git@gitcode.com:<author>/<repo>.git <branch> → git checkout FETCH_HEAD。origin 远程找不到 fork 分支。
  3. 底层核实胜过表面调用:P1 论据是"复用 cmdutil 可恢复 secret 扫描",复核不能只看 readYAML 调了 ReadTextFile,要 grep 确认 ReadTextFile 内部确实调 ScanContentForSecrets,否则修复可能"形似而神不至"。
  4. approval 与 merge 是两套权限:gc pr review --approve 需 GitCode 平台单独授予 approval permission;403 时按 --help 回退 --comment 发布 APPROVED 正文,不影响放行。
  5. P0 升级前先核实规则边界:首轮把"个人仓库验证"误判为违反 infra-test 硬约束;实际规则边界是"项目内部开发须用 infra-test,外部贡献者无权限时用本人授权仓库并记录"是正确路径。升级阻塞前先确认规则适用对象,不机械套用。
  6. 误判必须显式撤回:撤回 P0 时在 PR 评论中明确写出"撤回"与正确边界,不静默放弃,让作者与后续读者都能追溯。
  7. P1 should-fix + P2 nits 分级让闭环高效:作者一次批量修复,评审者一次批量复核,避免逐条往返;P2 可选但作者若顺带修,复核时一并验证。
  8. 合并后验证状态:gc pr merge 返回 merged: true 后,再 gc pr view 看 state=merged + merged_at 双确认,PR body Closes #497 触发 Issue 自动关闭。

相关案例

  • 前置:评审已有 Tag 发布能力 PR — 单轮结构化评审的基础流程,本案例是其反馈闭环延伸
  • 对照:Team Agent 多角色并行评审 — 多 Agent 并行评审 + 反馈闭环,本案例是单评审者轻量版
  • 关联:PR 合并策略与清理 — pr merge 策略选择与分支清理
  • 参考:spec/workflows/review-workflow.md — 评审角色与分级正式规范
  • 参考:spec/workflows/pr-workflow.md — PR 状态机与 Closes #XXX 自动关闭规则