fix(review): clarify closeout authority and dismissal readback - #5565
Conversation
Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent · gpt-6.1-sol · OpenAI · runtime_reported · xhigh
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Exact head: aafe6bdfa2e2a2543219f6cf674fd1620a7d0365 · immutable baseline: 69ad89c7fe214e3fb67d1fe894b65b17e6040461
动机
评审者在批准修复后的代码时,需要协调仍生效的旧阻塞意见,让维护者看到可信的最终状态。
此前只读计划显示“不授予撤回权限”,评审者可能误以为已有操作授权失效,从而在发现旧问题已解决后仍停止协调;现在契约要求另查当前授权和 GitHub 权限,并实际核验撤回后的状态。
当前 CLI 已明确这一区别,同时保留原有只读权限标记和全部阻塞意见;HTTP 成功但 review 状态未改变,会被明确记录为失败。
本次不授予新权限、不自动撤回 review、不抹去未解决意见,也不保证 GitHub 对已合并 PR 的撤回请求一定生效。
改动思路
继续复用原有能力 owner:GitHub 适配器读取完整历史,类型化 TS 规则选择每位 reviewer 的有效意见,Python 在结果上附加原生执行步骤,host 依独立授权实际协调。最小修复是修改同一 owner 的三条步骤,避免另做权限分类器或自动撤回执行器。正向流程仍逐项核验全部 finding 和 inline comment、实际权限、当前 head 与 approval,然后要求目标状态读回;它没有从批准或 commit 年龄推导权限。
具体改动
规格依据:loopx/capabilities/pr_review_queue/README.md,固定于 69ad89c7fe214e3fb67d1fe894b65b17e6040461。该文档的 post-approval closeout 段没有原生条款编号,以下名称仅定位既有要求:finding-resolution 要求完整独立核验且年龄不构成证明;owner-and-github-authority 要求 owner 和实际 GitHub 权限、且计划只读;strict-readback 要求目标 DISMISSED、approval/head 保持并报告残余 blocker。这三项均 implemented,没有新增权限或自动执行。全量 diff 只有一个文件 +3/-3,无私有材料、生成物或额外逻辑。
关键代码讲解
approval_closeout_contract 明确 dismissal_authorized=false 和 merge_authorized=false 表示计划不授予权限,不能拿它代替当前操作人的独立授权判断。GitHub 示例补充官方文档给出的 event=DISMISS;这不证明漏字段是平台无状态变化的原因。最后一步明确 HTTP 成功不算撤回收据,目标必须读回 DISMISSED,否则报告失败,不盲目重试。plan_approval_closeout 是未改动的相关调用符号:仍调用 TS effect runtime、校验 schema 后附加同一契约,没有外部写入。它保证更新步骤进入 compact CLI,队列 builder 也加载同一契约。
对主干的风险
最强风险是把别人 approval 或旧 head 当作已解决证据,或者 HTTP200 就声称撤回完成。原 TS owner 和 authority flags 均保持;现有负例覆盖另一人批准/普通 comment 不清除 blocker、旧与同 head blocker、history 重排、缺失 source、变更 head、伪造 conclusion 和未知 state。87 项相关测试通过,Ruff、diff、公开边界扫描及原生 premerge 的 5 项直接检查、2 项选择 canary 通过。初次 semantic 校验缺少 npm 开发依赖;补齐后完整选择集通过,未删断言或放宽预算。没有查询或等待 CI。
真实路径使用同一 GitHub PR 和完整 review history,分别运行固定 baseline 与当前 head 的公开 CLI:整个 typed result 一致,原有两份 blocker、approval、hold、raw decision 和 no-write flags 都保留,只有声明过的三条指令不同。独立 guide oracle 在旧版本失败、当前版本通过。实际已合并 PR 的 REST/GraphQL 撤回请求返回成功但目标仍为 CHANGES_REQUESTED;这个负例保留为失败,不能用模拟 DISMISSED 证明它成功。付费 live model compliance 未测。本 PR 没有新 opt-in 能力、默认开启、设置或 UI,也没有 prose 分类器、领域专属通用义务。
语义与 CI 对齐
复用既有 DISMISSED、typed hold/history vocabulary 和执行契约。先运行当前 diff advisory,再通过全树 semantic canary。机器只读边界与操作人义务明确区分;新增解释不构成合并或撤回权限。
我的整体评价
没有发现阻断性代码问题。交付判定 goal_achieved 仅指这次完整的原生指令修复;不把已合并 PR 上的 GitHub 无状态变化称为业务关闭。long_horizon improved:减少遗漏有权协调的任务和反复空重试;user_experience improved:清楚说明所需权限与真正成功条件。成本与问题匹配,未来重构检查已选择复用现有 guide 和类型化 owner,额外 executor 或模块抽取没有本次价值。公开源代码与真实 CLI 对照支持 APPROVE。剩余风险是模型是否遵循指令、GitHub 当前 post-merge 状态无法撤回;合并必须另验精确 head readiness 和 owner 授权。
English verdict: APPROVE — the existing read-only closeout guide now distinguishes independent operator authority from plan no-grant flags and requires a DISMISSED postcondition. 87 tests, native premerge and immutable base/head real CLI comparison passed; no new write authority, automatic dismissal or platform recovery is claimed.
The approval closeout plan is read-only and reports
dismissal_authorized=false. That flag describes the plan’s authority, but could be mistaken for a denial of authority independently granted to the operator, stopping verified review reconciliation prematurely.Clarify the existing capability-owned procedure: resolve operator authorization and GitHub permission separately, preserve unresolved findings, use the documented
event=DISMISSrequest, and require aDISMISSEDreadback. An HTTP success with unchanged review state is a failed postcondition. This changes execution guidance only; the typed read model, authority admission, and GitHub mutation implementation stay unchanged.Validation: 87 related review/closeout/queue tests; Ruff; semantic advisory; native premerge (5 direct checks and both selected canaries); public-boundary scan; real CLI closeout readback with unchanged authority flags and remaining blockers. Initial semantic validation lacked repository npm dependencies; after
npm ci --ignore-scripts, the selected validation passed. No CI was fetched or awaited. The change does not claim that a post-merge GitHub dismissal returning an unchanged state succeeded.Placement/future-facing pass: reuse
pr_review_queue’s existing GitHub read adapter and procedure contract; no second policy owner or dismissal executor is needed.