fix(canary): separate validation from merge authority - #4853
Conversation
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
545d555 to
4245284
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
动机
本次按 exact head 42452843182b672d6bf35d420900e2aa8fb79550 审阅 #4853。它修复的是一个真实的 authority 语义错误:loopx canary premerge 只拥有验证执行与证据汇总,但旧版在 status=passed 且没有 manual hold 时会给出 self_merge_allowed: true。这会把“本地/CI 验证通过”错误提升成“仓库政策允许作者自合并”,而 #4835 的 post-merge audit 已给出了实际误用证据。
本 PR 的交付边界完整且克制:保留 merge_gate_passed 表示验证结论,新增 self_merge_validation_passed 让 validation-only 语义可机读,把旧 self_merge_allowed 永久设为 fail-closed,并用 self_merge_authority 明确指向 repository policy 与 exact-head merge readiness。它没有在 canary 内复制一套 GitHub 权限或仓库策略引擎。
改动思路
入口仍是 build_premerge_validation_gate,权威输入仍是 direct/catalog/risk/boundary checks、manual holds 与可选的 change-quality receipt;_gate_status 只负责形成验证投影。真正的合并授权继续属于仓库政策与 exact-head readiness,两者没有被一个绿色 canary 隐式替代。
正向路径中,全部验证通过时 merge_gate_passed=true、self_merge_validation_passed=true,但 self_merge_allowed=false 且 self_merge_authority.granted=false。负向路径中,若后续 change-quality receipt 强制失败,apply_change_quality_verification 会同时清掉两个 validation-success 信号,authority 仍保持拒绝。这样既避免旧字段继续误授权,也没有产生第二份持久状态或重复 authority owner。
具体改动
关键代码讲解
loopx/canary/premerge.py:578的_gate_status保留原 status 计算,只把 validation 与 authority 拆开;任何 status 都不会让 canary 自己授予合并权限。loopx/canary/premerge.py:707的apply_change_quality_verification在 enforced receipt failure 时新增清理self_merge_validation_passed,防止“先绿后红”后残留陈旧成功信号。loopx/canary/premerge.py:1096的 markdown renderer 把 validation 与 authority reason 分行展示,机器字段与人工报告保持一致。examples/canary/premerge-validation-gate-smoke.py:313新增 durable regression:直接构造绿色 gate,断言 validation 通过但 authority 拒绝;随后注入无效 quality receipt,再断言 validation 也变为 false。
仓库全量搜索没有发现 self_merge_allowed 的生产读取方;现有引用只在 canary 与测试/示例内。tests/capabilities/test_change_quality.py 中保留的 synthetic self_merge_allowed: true 只是注册表解析 fixture,没有消费该字段,也没有形成第二个 authority owner。
对主干的风险
最强反例是旧 consumer 把 self_merge_allowed 当作许可:升级后它会看到 false 并停止,而不是继续误合并。这是有意的 fail-closed 兼容变化,PR body、payload note、JSON/markdown 字段与 regression smoke 都明确披露;若 consumer 需要继续工作,正确修复是接入 repository policy 与 exact-head readiness,而不是恢复 permissive boolean。
我在该 exact head 实际执行并读回:
- changed Python
py_compile:通过; examples/canary/premerge-validation-gate-smoke.py:通过;tests/capabilities/test_change_quality.py与tests/test_public_package_lock_boundary.py:24 passed in 10.93s;- focused Ruff 与
git diff --check:通过; - CLI quick preview:读回
self_merge_allowed=false、self_merge_validation_passed=false(preview-only)以及self_merge_authority.granted=false,next gate 文本正确。
语义与 CI 对齐
这里扩展的是既有 canary gate vocabulary,而不是复用一个含糊 boolean:validation 是机器验证结论,authority 是明确拒绝并委托给外部强制门禁。没有 substring/prose 分类,没有把 mandatory gate 称为 guidance,也没有产品特定 wording。
复审时 GitHub broad rollup 有一个 test shard 失败且部分 job 尚未结束;当前 PR-review capability 的 wait_for_ci=false,所以我没有轮询或把 CI 当作本结论的证据,也不会用本地 focused pass 掩盖仓库检查。该 PR 仍须通过仓库 required checks,并由独立 maintainer 执行合并授权。
我的整体评价
APPROVE。 这是对正确 owner 的最小完整修复:验证仍由 canary 决定,合并权限明确不由 canary 决定;旧字段安全失败,新字段可观测,负向 quality-receipt 路径也不会留下陈旧成功状态。未来向检查不建议在本 PR 内再加入 merge-policy evaluator;那会把两个 authority boundary 重新混在一起。
本批准仅绑定 exact head 42452843182b672d6bf35d420900e2aa8fb79550,不授予作者自合并,也不替代 required checks 或独立 maintainer merge。
English verdict: APPROVE - Exact head 4245284 cleanly separates validation evidence from repository merge authority, keeps the legacy field fail-closed, clears stale validation success on enforced quality failure, and passes the focused runtime, smoke, static, and output-readback checks; repository checks and independent-maintainer merge authority remain separate gates.
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
动机
#4835 的事后审计没有发现其业务逻辑的新阻断缺陷,但确认了一个控制面流程缺陷:基线 62d18677c689325066bc406638d15319582f7a1a 的 _gate_status 会在本地验证通过时直接返回 self_merge_allowed: true,而 canary 并不持有仓库策略、审阅者身份或最终 CI 的合并授权。这个错误信号已被用于一次不符合独立维护者门槛的 loopx/** 合并。
本 PR 的完整可观察结果是:验证通过仍会报告 merge_gate_passed=true 和 self_merge_validation_passed=true,但旧的授权形字段始终 fail-closed,结构化 self_merge_authority 明确把下一道门槛交还给 repository policy 与 exact-head merge readiness。
改动思路
没有新建 policy engine,也没有在 canary 中复制各仓库规则。改动复用现有 _gate_status 作为验证聚合 owner、apply_change_quality_verification 作为组合失败 owner、Markdown renderer 作为人工回读面,并在现有 premerge smoke 中验证同一条公共契约。
正向路径是“全部 checks 通过 → 验证信号为真 → 授权仍为假”;负向路径是“初始 gate 通过 → 强制 change-quality receipt 失败 → merge 与 validation 两个信号都被清零”。这覆盖了最容易遗漏的后置组合失败。
具体改动
关键代码讲解
loopx/canary/premerge.py:578_gate_status:保留原有状态归约,新增 validation-only 信号;self_merge_allowed与self_merge_authority.granted均固定为 false,避免本地证据越权成为仓库授权。loopx/canary/premerge.py:707apply_change_quality_verification:强制质量凭证失败时同步清除self_merge_validation_passed,不会留下先绿后红的陈旧成功状态。loopx/canary/premerge.py:1096render_premerge_validation_gate_markdown:把 validation 与 authority 分开显示,并输出repository_policy_required,人读结果与 JSON 语义一致。examples/canary/premerge-validation-gate-smoke.py:313assert_passed_validation_does_not_grant_self_merge_authority:基线会在绿色 gate 的授权断言处失败;精确 head 同时通过绿色验证与强制质量失败两条路径。
对主干的风险
最强回归风险是外部消费者此前把 self_merge_allowed=true 当作兼容信号。这里有意选择 fail-closed:字段仍保留但不再授予权限,消费者应改读 self_merge_validation_passed,再独立应用仓库策略。没有持久化迁移、CLI 参数、调度、配额或副作用变化;回滚是一份两文件 revert。
验证:focused smoke 通过;tests/capabilities/test_change_quality.py 与 tests/test_public_package_lock_boundary.py 共 24 passed;标准 diff 风险 gate 17/17 通过,0 failures / 0 warnings / 0 manual holds;py_compile、git diff --check、DCO、Dependency Review 与 Release Artifacts 通过。
阻断项是仓库必需的 Python Tests:四个 test shard 均失败。11 个失败都可在本 PR 基线 62d18677c689325066bc406638d15319582f7a1a 的 main run 35573407345 与本地 focused replay 中复现,并不来自本 PR 的两文件 diff。审阅期间 main 又前进到 99729430348007b976b1bbf3bc9613f21665448a,本 PR 现为 BEHIND,也必须更新基线后重跑:
tests/control_plane/test_settled_replay_construction.py:51的 8 个参数化 case 仍读取已删除的 legacyprotocol_action_packet;tests/control_plane/test_blocked_transition_notice.py:176,199的 2 个 case 暴露 commit9ee5c6fbc在处理 explicit-id 时意外删除了 typed notice 的生产投影;tests/control_plane/test_selection_replan_reentry.py:97的 fixture 执行新增 vision 路径时仍传入不存在的占位文件。
最小修复不是在本 PR 中删断言或吞掉错误,而是用独立的基线修复恢复 typed notice、更新 legacy packet 断言,并给 settlement fixture 提供真实 vision 输入;该修复合入后,把 #4853 更新到修复后的 main 并完整重跑 exact-head CI。
语义与 CI 对齐
受影响契约是 loopx_premerge_validation_gate_v0。本 PR 复用既有验证生命周期,不创建第二个授权 owner;旧字段的 true→false 是已披露并有反例 smoke 的刻意行为变化。没有 opt-in/default-off 声明,相关隔离项不适用。authority、typed-state、domain-neutrality、behavior-disclosure、guidance-vs-obligation lenses 均已核验。
我的整体评价
本 PR diff 自身没有 blocking code finding;精确 head 42452843182b672d6bf35d420900e2aa8fb79550 用最小、可逆的改动修复了 #4835 暴露的真实流程缺陷,并把未来合并决定归还给正确 owner。但仓库必需 CI 未满足,因此当前结论必须是 REQUEST_CHANGES / hold,而不是 APPROVE。
基线修复与绿色 exact-head 重跑完成后,剩余风险仅是旧消费者的 fail-closed 迁移;这是比继续错误授权更安全的兼容选择。由于本 PR 自身改变 loopx/** 控制面行为,作者不会自合并,最终合并仍必须由独立维护者执行。
English verdict: REQUEST_CHANGES - exact head 4245284; the canary diff has no identified code defect, but required Python CI is red on reproduced current-main regressions, so approval and merge must wait for an independently reviewed baseline repair and a green exact-head rerun.
Summary
self_merge_allowedpremerge field fail-closed because canary validation does not own repository merge authorityself_merge_validation_passedseparately from a structuredself_merge_authorityreadbackMotivation
The post-merge audit of #4835 found that its exact-head review cited
loopx canary premergereturningself_merge_allowed: truewhile the same review acknowledged that the control-plane authority change required an independent maintainer. The canary had collapsed two different decisions: whether the selected validation passed, and whether repository policy authorized an author-owned merge.This change repairs the owning signal instead of adding another instruction.
merge_gate_passedcontinues to report validation;self_merge_validation_passedmakes the validation-only interpretation explicit; andself_merge_allowedcan no longer be used as repository authority.Validation
uv run --extra test python -m py_compile loopx/canary/premerge.py examples/canary/premerge-validation-gate-smoke.pyuv run --extra test python examples/canary/premerge-validation-gate-smoke.pyuv run --extra test pytest -q tests/capabilities/test_change_quality.py tests/test_public_package_lock_boundary.py— 24 passeduv run --extra test loopx --format json canary premerge --from-git-diff --git-diff-base origin/main --tier standard --timeout-seconds 180 --no-progress— 17/17 selected checks passed, public-boundary scan clean, zero failures or manual holdsgit diff --checkThe first standard premerge run exposed a missing local Node parser dependency in
semantic-vocabulary-drift-smoke.py. Afternpm ci --ignore-scripts, the complete same validation set passed; no failed check was waived.Risk and merge authority
This is intentionally fail-closed for callers that treated the historical field as permission. Such callers must use the validation result as evidence and then apply repository policy plus exact-head merge readiness. No repository policy, GitHub permission, or admin-bypass authority is granted here.
Because this PR changes
loopx/**canary behavior, repository policy requires an independent maintainer to merge it. The author will not self-merge this repair.