fix(pr-review): reconcile obsolete blockers after exact-head approval - #5425
Conversation
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
审查 head:e0f81e6ccd5b06fb459b5a9e85a24c25c256003b;按 pull-request-review capability policy revision 13 执行。未发现阻塞问题。
动机
PR #5343 已证明:自己的有效批准不会覆盖另一账号仍生效的旧阻塞评审。单纯多发一次 APPROVE 不能完成用户想要的收尾,也不应把旧 commit、另一个账号的批准或绿色 CI 当成问题已修复的证据。本改动补齐 capability 的批准后收尾流程;源代码提案完成且可独立验证,实际安装行为仍等待维护者合并与发布。
改动思路
保留 GitHub 评审历史作为唯一事实源,Python 只负责读取完整文件、分页评审和调用既有批准校验,TypeScript 根据每位 reviewer 最新的决定性意见生成只读结果。没有增加重复同步的 Todo 状态、权限授予或自动撤销执行器。成功路径是读回批准、定位实际阻塞评审、逐条验证正文及 inline findings、检查 owner 授权和 GitHub 权限、临近操作再次确认精确 head,再走原生 dismissal 并读回。未解决的意见必须保留;收尾失败也不反向撤销已经得到证据支持的 APPROVE。相比只改 skill 提醒,这个有界读模型能确定性处理分页和历史;相比自动清除所有红评审,它保留必要判断与权限边界。
具体改动
关键代码讲解
handle_pr_review_command 在 loopx/cli_commands/pr_review.py:267 增加独立只读模式,混用扫描、fixture、result 或 readiness 参数会在远端读取之前拒绝,旧的直接构造 Namespace 调用用 getattr 保持兼容。read_github_approval_closeout 在 approval_closeout.py:44 复用 exact-head 规范化、文件完整性恢复和 _review_conclusion,只将当前账号的评审交给现有批准校验,再把完整跨账号历史交给 TS;正文只用于临时校验,不进入结果。planPrReviewApprovalCloseout 在 TS 文件第 18 行校验完整来源、批准和两次 head,使用 reviewer-keyed Map:COMMENTED/PENDING 不清除阻塞意见,另一账号的批准也不会清除它,最新 DISMISSED 不会复活旧历史。aggregate 与完整历史冲突时返回 hold,并保留原始 null,而不是虚构 APPROVED。
其余改动是 effect-runtime 注册、review policy 12→13 和 capability-owned closeout 组合点;README 与打包 managed skill 明示新收尾义务,skill 保持 180 行。新增测试覆盖历史、分页、同 head 评审、畸形输入、缺少批准、head 变化和禁止副作用;registry-I/O manifest 仅更新一个已有调用点行号,没有新增 registry 读取。
对主干的风险
主要风险是把“候选旧评审”错当成“已解决评审”并删掉有效反对意见。这里三个 effect/authority 标志始终为 false,候选只进入验证流程;真正撤销前必须读全 review 与 inline comments,逐项映射到当前代码和决定性验证,再由 native GitHub 权限约束操作。没有撤销 executor,因此没有对活跃 PR 做写入探针;真实 GitHub 入口使用 PR #5343 重复只读验证 clear、批准保留、无 CI 请求和无重复副作用。全量 Python 与原生 dismissal 权限没有被冒充为已验证。
语义与 CI 对齐
GitHub review states 是外部既有词汇,closeout 三种状态属于本地只读模型,不代表 merge readiness。普通 review packet 在不可变 base/head 上使用同一 public fixture 实跑,除声明的 policy/closeout 指令及生成时间、历史 fixture 的时钟年龄外逐字段一致,ranking/admission 未被归一化;独立批准后 routing oracle 在 base 缺失、head 通过。当前 Goal 设置不读 CI,评审据本地验证:253 项 Python 通过、33 项条件跳过;TS 3,642 项通过、31 项条件跳过,文档-only rebase 后产品源码不变;最终 typecheck、semantic/CLI budget/public-private 扫描和 premerge 的 19 个选中检查加 5 个直接检查通过,精确 diff CQA 有效。未提高预算或把无关红 CI 归罪于本 PR。
我的整体评价
这是现有 review owner 中一个完整、可回滚的收尾增量。长程执行改善在于重复调用从真实评审历史重新推导,不重复发布批准或维护第二份事实;用户体验改善在于给出真实阻塞 reviewer、目标和明确恢复条件。已做有界前瞻重构:隔离小 TS 读模型、复用原批准与 transport owner;不需要更大迁移或另一套 review mutation framework。默认行为新增批准后必做步骤,文档明确披露,并非 default-off 隐藏指令。残余边界是每次实际旧 finding 的人工证据判断及原生权限,而不是此 read-only CLI 自动认证。结论为 APPROVE;作者账号使用 COMMENTED fallback,既不是 GitHub 正式自批准,也不授权合并,维护者仍需独立评审合并。
English verdict: APPROVE - e0f81e6. No blocking finding; capability-owned post-approval reconciliation preserves unresolved reviews and explicit authority. Exact-head local premerge, focused Python, TS, base/head public-entrypoint characterization and real GitHub read-only validation passed; no live dismissal mutation or self-merge.
Goal And Delivered Outcome
main. Complete within the implementation/proposal scope; integration and installed behavior remain pending maintainer review and merge.Scope And Continuation
pr-review --check-approval-closeout NUMBER@HEAD_OID: require the existing capability-qualified exact-head approval, read all paginated reviews, select each reviewer's latest decisive opinion, and check the head again. Returnclear,verification_required, orhold; preserve GitHub's raw aggregate decision, includingnull.pull-request-review; TypeScript owns the pure reconciliation read model, while Python adapts GitHub transport and reuses the existing approval validator. No new capability, settings owner, provider distribution, or parallel Python decision owner.Validation
e0f81e6ccd5b06fb459b5a9e85a24c25c256003b, based on8fdc1616fc84ef158775e322cf3660cfe18a5be0.finished.synthetic,public_fixture.unit,regression_paritypassedunit,integrationpassednpm run -s test:control-plane: 3,642 passed, 31 conditional skips. Run before the documentation-only base rebase; changed product sources are identical. Focused Python, typecheck, and native premerge were rerun on the final head.staticpassednpm run -s typecheck:control-plane; Ruff on changed Python and the repository's declared lint surfaces; Mypy on the 19 repository-declared files (not a claim of full-tree Mypy coverage);git diff --check.real_entrypoint,real_backendpassed3a06669967e691780610d8f68c1ea7e048066cc8: repeated read-only closeout returnedclear, retained the existing approval, and performed no dismissal or CI read. No mutation probe was run against a live PR.integrationpassedpr-review-command-smoke.py, full semantic vocabulary/registry-I/O census, and CLI output budget smoke passed. The development-time semantic advisory was also run; GitHub's external review vocabulary and the local closeout read model have explicit owners. No budget was increased.integration,staticpassedcanary premerge --from-git-diff --git-diff-base 8fdc1616fc84ef158775e322cf3660cfe18a5be0: all 19 selected checks and 5 direct checks passed; zero manual holds, runtime failures, or quality-receipt failures. Public/private scan clean for all nine changed paths. Validation passing grants no self-merge authority.state=valid;scope_fingerprint=4e437c2da28881e12a59f797296d958231e458926315af5a0a54249bca8b9649;base_ref=base_commit=8fdc1616fc84ef158775e322cf3660cfe18a5be0;head_commit=e0f81e6ccd5b06fb459b5a9e85a24c25c256003b;receipt_id=cqr_4e437c2da28881e12a59;requalification_required=false;previous_receipt_id=null;previous_scope_fingerprint=null;previous_base_commit=null.Frontend / Visual Evidence
none. The new mode is an explicit CLI readback and post-publication host contract, not a configuration change. Existing settings/capability editors need no companion control. No dashboard, Lark, opening navigation, or public first viewport changes.none.Type of Change
LoopX Area
Technical Direction
Boundary Checklist
none.Signed-off-bytrailers.